public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively.. 3+ messages / 3 participants [nested] [flat]
* [PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively.. @ 2020-03-09 03:52 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-03-09 03:52 UTC (permalink / raw) ..possibly there's a better place to put this, like maybe a doc-only example ? Need catversion bumped ? --- doc/src/sgml/func.sgml | 27 ++++++++++++++++++++ src/backend/catalog/system_views.sql | 1 + src/include/catalog/pg_proc.dat | 6 +++++ src/test/regress/expected/misc_functions.out | 14 ++++++++++ src/test/regress/sql/misc_functions.sql | 6 +++++ 5 files changed, 54 insertions(+) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index c3a6b3277f..3a2ad3eb50 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25311,6 +25311,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); EXECUTE to run the function. </entry> </row> + <row> + <entry> + <literal><function>pg_ls_dir_recurse(<parameter>dirname</parameter> <type>text</type>)</function></literal> + </entry> + <entry><type>setof text</type></entry> + <entry> + Call pg_ls_dir_metadata to recursively list the files in the specified + directory, along with each file's metadata. + Restricted to superusers by default, but other users can be granted + EXECUTE to run the function. + </entry> + </row> <row> <entry> <literal><function>pg_ls_logdir()</function></literal> @@ -25422,6 +25434,21 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); directory along with the file's metadata. </para> + <indexterm> + <primary>pg_ls_dir_recurse</primary> + </indexterm> + <para> + <function>pg_ls_dir_recurse</function> recursively lists the files + in the specified directory. + To recursively list temporary directories in all tablespaces: +<programlisting> +SELECT * FROM (SELECT DISTINCT COALESCE(NULLIF(pg_tablespace_location(b.oid),'')||suffix, 'base/pgsql_tmp') AS dir +FROM pg_tablespace b, pg_control_system() pcs, +LATERAL format('/PG_%s_%s', left(current_setting('server_version_num'), 2), pcs.catalog_version_no) AS suffix) AS dir, +LATERAL pg_ls_dir_recurse(dir) AS a; +</programlisting> + </para> + <indexterm> <primary>pg_ls_logdir</primary> </indexterm> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 1c77430f0c..215fba3b7d 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1468,6 +1468,7 @@ REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir_metadata(text,boolean,boolean) FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_dir_recurse(text) FROM public; -- -- We also set up some things as accessible to standard roles. diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index adfce45d1a..f94f403475 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10916,6 +10916,12 @@ proallargtypes => '{text,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}', proargnames => '{dirname,name,size,modification,isdir}', prosrc => 'pg_ls_dir_metadata_1arg' }, +{ oid => '5034', descr => 'list all files in a directory recursively', + proname => 'pg_ls_dir_recurse', prorows => '10000', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => 'text', + proallargtypes => '{text,text,int8,timestamptz,bool}', + proargnames => '{dirname,name,size,modification,isdir}', proargmodes => '{i,o,o,o,o}', + prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.modification, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" }, # hash partitioning constraint function { oid => '5028', descr => 'hash partition CHECK constraint', diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 64b1417fb8..4188d684f0 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -243,6 +243,20 @@ select * from pg_ls_dir_metadata('.') limit 0; ------+------+--------------+------- (0 rows) +-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix +SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal'; + name | isdir +-----------------------+------- + pg_wal | t + pg_wal/archive_status | t +(2 rows) + +-- Check that expected columns are present +SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0; + name | size | modification | isdir +------+------+--------------+------- +(0 rows) + -- -- Test adding a support function to a subject function -- diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index 372345720d..6041c4f3dc 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -76,6 +76,12 @@ select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- -- Check that expected columns are present select * from pg_ls_dir_metadata('.') limit 0; +-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix +SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal'; + +-- Check that expected columns are present +SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0; + -- -- Test adding a support function to a subject function -- -- 2.17.0 --2FkSFaIQeDFoAt0B Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0008-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Race conditions in logical decoding @ 2026-01-22 19:59 Álvaro Herrera <[email protected]> 2026-01-23 06:33 ` Re: Race conditions in logical decoding Antonin Houska <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Álvaro Herrera @ 2026-01-22 19:59 UTC (permalink / raw) To: Antonin Houska <[email protected]>; +Cc: [email protected] On 2026-Jan-20, Antonin Houska wrote: > Antonin Houska <[email protected]> wrote: > > > I'm not sure yet how to fix the problem. I tried to call XactLockTableWait() > > from SnapBuildAddCommittedTxn() (like it happens in SnapBuildWaitSnapshot()), > > but it made at least one regression test (subscription/t/010_truncate.pl) > > stuck - probably a deadlock. I can spend more time on it, but maybe someone > > can come up with a good idea sooner than me. > > Attached here is what I consider a possible fix - simply wait for the CLOG > update before building a new snapshot. > > Unfortunately I have no idea right now how to test it using the isolation > tester. With the fix, the additional waiting makes the current test > block. (And if a step is added that unblock the session, it will not reliably > catch failure to wait.) > > -- > Antonin Houska > Web: https://www.cybertec-postgresql.com > > @@ -400,6 +400,47 @@ SnapBuildBuildSnapshot(SnapBuild *builder) > snapshot->xmin = builder->xmin; > snapshot->xmax = builder->xmax; > > + /* > + * Although it's very unlikely, it's possible that a commit WAL record was > + * decoded but CLOG is not aware of the commit yet. Should the CLOG update > + * be delayed even more, visibility checks that use this snapshot could > + * work incorrectly. Therefore we check the CLOG status here. > + */ > + while (true) > + { > + bool found = false; > + > + for (int i = 0; i < builder->committed.xcnt; i++) > + { > + /* > + * XXX Is it worth remembering the XIDs that appear to be > + * committed per CLOG and skipping them in the next iteration of > + * the outer loop? Not sure it's worth the effort - a single > + * iteration is enough in most cases. > + */ > + if (unlikely(!TransactionIdDidCommit(builder->committed.xip[i]))) > + { > + found = true; > + > + /* > + * Wait a bit before going to the next iteration of the outer > + * loop. The race conditions we address here is pretty rare, > + * so we shouldn't need to wait too long. > + */ > + (void) WaitLatch(MyLatch, > + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, > + 10L, > + WAIT_EVENT_SNAPBUILD_CLOG); > + ResetLatch(MyLatch); > + > + break; > + } > + } > + > + if (!found) > + break; > + } I think this algorithm is strange -- if you do have to wait more than once for one transaction, it would lead to doing the TransactionIdDidCommit again times for _all_ transactions by starting the inner loop from scratch, which sounds really wasteful. Why not nest the for() loops the other way around? Something like this perhaps, for (int i = 0; i < builder->committed.xcnt; i++) { for (;;) { if (TransactionIdDidCommit(builder->committed.xip[i])) break; else { (void) WaitLatch(MyLatch, WL_LATCH_SET, WL_TIMEOUT, WL_EXIT_ON_PM_DEATH, 10L, WAIT_EVENT_SNAPBUILD_CLOG); ResetLatch(MyLatch); } CHECK_FOR_INTERRUPTS(); } } This way you wait repeatedly for one transaction until it is marked committed; and once it does, you don't test it again. I also wondered if it would make sense to get rid of the memcpy, given that we're doing so much work per xid anyway it won't make any visible difference (I believe), and do the copy per XID there, like if (TransactionIdDidCommit(builder->committed.xip[i])) { snapshot->xip[i] = builder->committed.xip[i]; break; } else ... -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Sallah, I said NO camels! That's FIVE camels; can't you count?" (Indiana Jones) ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Race conditions in logical decoding 2026-01-22 19:59 Re: Race conditions in logical decoding Álvaro Herrera <[email protected]> @ 2026-01-23 06:33 ` Antonin Houska <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Antonin Houska @ 2026-01-23 06:33 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] Álvaro Herrera <[email protected]> wrote: > I think this algorithm is strange -- if you do have to wait more than > once for one transaction, it would lead to doing the > TransactionIdDidCommit again times for _all_ transactions by starting > the inner loop from scratch, which sounds really wasteful. Why not nest > the for() loops the other way around? I'm quite sure I wanted to iterate through committed.xnt in the outer loop, but probably got distracted by something else and messed things up. > Something like this perhaps, > > for (int i = 0; i < builder->committed.xcnt; i++) > { > for (;;) > { > if (TransactionIdDidCommit(builder->committed.xip[i])) > break; > else > { > (void) WaitLatch(MyLatch, > WL_LATCH_SET, WL_TIMEOUT, WL_EXIT_ON_PM_DEATH, > 10L, > WAIT_EVENT_SNAPBUILD_CLOG); > ResetLatch(MyLatch); > } > CHECK_FOR_INTERRUPTS(); > } > } > > This way you wait repeatedly for one transaction until it is marked > committed; and once it does, you don't test it again. Sure, that's much beter. Thanks. -- Antonin Houska Web: https://www.cybertec-postgresql.com ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-01-23 06:33 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-09 03:52 [PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively.. Justin Pryzby <[email protected]> 2026-01-22 19:59 Re: Race conditions in logical decoding Álvaro Herrera <[email protected]> 2026-01-23 06:33 ` Re: Race conditions in logical decoding Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox