public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v17 03/10] Add tests on pg_ls_dir before changing it 3+ messages / 2 participants [nested] [flat]
* [PATCH v17 03/10] Add tests on pg_ls_dir before changing it @ 2020-03-17 18:16 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-03-17 18:16 UTC (permalink / raw) --- src/test/regress/expected/misc_functions.out | 18 ++++++++++++++++++ src/test/regress/sql/misc_functions.sql | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index d3acb98d04..2e87c548eb 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -201,6 +201,24 @@ select count(*) > 0 from t (1 row) +select * from (select pg_ls_dir('.', false, true) as name) as ls where ls.name='.'; -- include_dot_dirs=true + name +------ + . +(1 row) + +select * from (select pg_ls_dir('.', false, false) as name) as ls where ls.name='.'; -- include_dot_dirs=false + name +------ +(0 rows) + +select pg_ls_dir('does not exist', true, false); -- ok with missingok=true + pg_ls_dir +----------- +(0 rows) + +select pg_ls_dir('does not exist'); -- fails with missingok=false +ERROR: could not open directory "does not exist": No such file or directory -- -- 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 094e8f8296..f6857ad177 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -60,6 +60,11 @@ select count(*) > 0 from where spcname = 'pg_default') pts join pg_database db on pts.pts = db.oid; +select * from (select pg_ls_dir('.', false, true) as name) as ls where ls.name='.'; -- include_dot_dirs=true +select * from (select pg_ls_dir('.', false, false) as name) as ls where ls.name='.'; -- include_dot_dirs=false +select pg_ls_dir('does not exist', true, false); -- ok with missingok=true +select pg_ls_dir('does not exist'); -- fails with missingok=false + -- -- Test adding a support function to a subject function -- -- 2.17.0 --4LFBTxd4L5NLO6ly Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0004-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: create_index test fails when synchronous_commit = off @ master @ 2022-02-24 15:33 Andres Freund <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Andres Freund @ 2022-02-24 15:33 UTC (permalink / raw) To: Aleksander Alekseev <[email protected]>; David Rowley <[email protected]>; Tom Lane <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> Hi, On 2022-02-24 16:47:25 +0300, Aleksander Alekseev wrote: > - QUERY PLAN > -------------------------------------------------------- > - Index Only Scan using tenk1_thous_tenthous on tenk1 > - Index Cond: (thousand < 2) > - Filter: (tenthous = ANY ('{1001,3000}'::integer[])) > -(3 rows) > + QUERY PLAN > +-------------------------------------------------------------------------------------- > + Sort > + Sort Key: thousand > + -> Index Only Scan using tenk1_thous_tenthous on tenk1 > + Index Cond: ((thousand < 2) AND (tenthous = ANY > ('{1001,3000}'::integer[]))) > +(4 rows) Heh. We've been having a lot of fights with exactly this plan change in the AIO branch, before cc50080a82, and without synchronous_commit = off. Interestingly near-exclusively with the regression run within pg_upgrade's tests. For aio we (David did a lot of that IIRC) finally hunted it down to be due vacuum skipping pages due to inability to get a cleanup lock. If that happens enough, pg_class.relallvisible changes enough to lead to the different plan. I first was going to suggest that we should just use VACUUM FREEZE to prevent the issue. But in this instance the cause isn't cleanup locks, probably that we can't yet set hint bits due to synchronous_commit=off? But I don't *fully* understand how it leads to this. I added the SELECT relpages, reltuples, relallvisible FROM pg_class WHERE oid = 'tenk1'::regclass; just after the VACUUM ANALYZE tenk1; synchronous_commit=on + relpages | reltuples | relallvisible +----------+-----------+--------------- + 345 | 10000 | 345 +(1 row) synchronous_commit=off + relpages | reltuples | relallvisible +----------+-----------+--------------- + 345 | 10000 | 0 +(1 row) So it clearly is the explanation for the issue. Obviously we can locally work around it by adding a SET LOCAL synchronous_commit = local; to the COPY. But I'd like to fully understand what's going on. > I didn't investigate further. Do we assume that `make installcheck` suppose > to pass with a different postgresql.conf options? Depends on the option, I think... There's some where it's interesting to run tests with different options and where the effort required is reasonable. And some cases where it's not... synchronous_commit=off worked until recently, and I think we should keep it working. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: create_index test fails when synchronous_commit = off @ master @ 2022-02-24 15:46 Andres Freund <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2022-02-24 15:46 UTC (permalink / raw) To: Aleksander Alekseev <[email protected]>; David Rowley <[email protected]>; Tom Lane <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> Hi, On 2022-02-24 07:33:39 -0800, Andres Freund wrote: > I added the SELECT relpages, reltuples, relallvisible FROM pg_class WHERE oid = 'tenk1'::regclass; > just after the > VACUUM ANALYZE tenk1; > > synchronous_commit=on > + relpages | reltuples | relallvisible > +----------+-----------+--------------- > + 345 | 10000 | 345 > +(1 row) > > synchronous_commit=off > + relpages | reltuples | relallvisible > +----------+-----------+--------------- > + 345 | 10000 | 0 > +(1 row) > > So it clearly is the explanation for the issue. > > > Obviously we can locally work around it by adding a > SET LOCAL synchronous_commit = local; > to the COPY. But I'd like to fully understand what's going on. It is the hint bit sets delayed by asynchronous commit. I traced execution and we do end up not setting all visible due to reaching the !HeapTupleHeaderXminCommitted() path in lazy_scan_prune() case HEAPTUPLE_LIVE: ... /* * Is the tuple definitely visible to all transactions? * * NB: Like with per-tuple hint bits, we can't set the * PD_ALL_VISIBLE flag if the inserter committed * asynchronously. See SetHintBits for more info. Check that * the tuple is hinted xmin-committed because of that. */ if (prunestate->all_visible) { TransactionId xmin; if (!HeapTupleHeaderXminCommitted(tuple.t_data)) So it might be reasonable to use synchronous_commit=on in test_setup.sql? It's not super satisfying, but i don't immediately see what else could prevent all-visible to be set in this case? There's no dead rows, so concurrent snapshots shouldn't prevent cleanup. I guess we could alternatively try doing something like flushing pending async commits at the start of vacuum. But that probably isn't warranted. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-24 15:46 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-17 18:16 [PATCH v17 03/10] Add tests on pg_ls_dir before changing it Justin Pryzby <[email protected]> 2022-02-24 15:33 Re: create_index test fails when synchronous_commit = off @ master Andres Freund <[email protected]> 2022-02-24 15:46 ` Re: create_index test fails when synchronous_commit = off @ master Andres Freund <[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