public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v22 03/10] Add tests on pg_ls_dir before changing it
15+ messages / 9 participants
[nested] [flat]
* [PATCH v22 03/10] Add tests on pg_ls_dir before changing it
@ 2020-03-17 18:16 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 15+ 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
--d6Gm4EdcadzBjdND
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v22-0004-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch"
^ permalink raw reply [nested|flat] 15+ messages in thread
* collect_corrupt_items_vacuum.patch
@ 2022-04-04 08:51 Daniel Shelepanov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Shelepanov @ 2022-04-04 08:51 UTC (permalink / raw)
To: [email protected]
Hi hackers!
I’ve been working on this [ https://www.postgresql.org/message-id/flat/cfcca574-6967-c5ab-7dc3-2c82b6723b99%40mail.ru ] bug. Finally, I’ve come up with the patch you can find attached. Basically what is does is rises a PROC_IN_VACUUM flag and resets it afterwards. I know this seems kinda crunchy and I hope you guys will give me some hints on where to continue. This [ https://www.postgresql.org/message-id/20220218175119.7hwv7ksamfjwijbx%40alap3.anarazel.de ] message contains reproduction script. Thank you very much in advance.
Kind regards,
Daniel Shelepanov
Attachments:
[text/x-diff] collect_corrupt_items_vacuum.patch (1.4K, ../../[email protected]/3-collect_corrupt_items_vacuum.patch)
download | inline diff:
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 53aaf61cb94..700ebeaa018 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -18,6 +18,7 @@
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
+#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/rel.h"
@@ -562,12 +563,19 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
Buffer vmbuffer = InvalidBuffer;
BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
TransactionId OldestXmin = InvalidTransactionId;
+ uint8 oldStatusFlags;
rel = relation_open(relid, AccessShareLock);
/* Only some relkinds have a visibility map */
check_relation_relkind(rel);
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ oldStatusFlags = MyProc->statusFlags;
+ MyProc->statusFlags |= PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
if (all_visible)
OldestXmin = GetOldestNonRemovableTransactionId(rel);
@@ -722,6 +730,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
items->count = items->next;
items->next = 0;
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags = oldStatusFlags;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
return items;
}
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-07-27 21:50 Robert Haas <[email protected]>
parent: Daniel Shelepanov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Robert Haas @ 2022-07-27 21:50 UTC (permalink / raw)
To: Daniel Shelepanov <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Mon, Apr 4, 2022 at 4:51 AM Daniel Shelepanov <[email protected]> wrote:
> I’ve been working on this [https://www.postgresql.org/message-id/flat/cfcca574-6967-c5ab-7dc3-2c82b6723b99%40mail.ru] bug. Finally, I’ve come up with the patch you can find attached. Basically what is does is rises a PROC_IN_VACUUM flag and resets it afterwards. I know this seems kinda crunchy and I hope you guys will give me some hints on where to continue. This [https://www.postgresql.org/message-id/20220218175119.7hwv7ksamfjwijbx%40alap3.anarazel.de] message contains reproduction script. Thank you very much in advance.
I noticed the CommitFest entry for this thread today and decided to
take a look. I think the general issue here can be stated in this way:
suppose a VACUUM computes an all-visible cutoff X, i.e. it thinks all
committed XIDs < X are all-visible. Then, at a later time, pg_visible
computes an all-visible cutoff Y, i.e. it thinks all committed XIDs <
Y are all-visible. If Y < X, pg_check_visible() might falsely report
corruption, because VACUUM might have marked as all-visible some page
containing tuples which pg_check_visibile() thinks aren't really
all-visible.
In reality, the oldest all-visible XID cannot move backward, but
ComputeXidHorizons() lets it move backward, because it's intended for
use by a caller who wants to mark pages all-visible, and it's only
concerned with making sure that the value is old enough to be safe.
And that's a problem for the way that pg_visibility is (mis-)using it.
To say that another way, ComputeXidHorizons() is perfectly fine with
returning a value that is older than the true answer, as long as it
never returns a value that is newer than the new answer. pg_visibility
wants the opposite. Here, a value that is newer than the true value
can't do worse than hide corruption, which is sort of OK, but a value
that's older than the true value can report corruption where none
exists, which is very bad.
I have a feeling, therefore, that this isn't really a complete fix. I
think it might address one way for the horizon reported by
ComputeXidHorizons() to move backward, but not all the ways.
Unfortunately, I am out of time for today to study this... but will
try to find more time on another day.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-07-27 21:55 Tom Lane <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2022-07-27 21:55 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Daniel Shelepanov <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Robert Haas <[email protected]> writes:
> In reality, the oldest all-visible XID cannot move backward, but
> ComputeXidHorizons() lets it move backward, because it's intended for
> use by a caller who wants to mark pages all-visible, and it's only
> concerned with making sure that the value is old enough to be safe.
Right.
> And that's a problem for the way that pg_visibility is (mis-)using it.
> To say that another way, ComputeXidHorizons() is perfectly fine with
> returning a value that is older than the true answer, as long as it
> never returns a value that is newer than the new answer. pg_visibility
> wants the opposite. Here, a value that is newer than the true value
> can't do worse than hide corruption, which is sort of OK, but a value
> that's older than the true value can report corruption where none
> exists, which is very bad.
Maybe we need a different function for pg_visibility to call?
If we want ComputeXidHorizons to serve both these purposes, then it
has to always deliver exactly the right answer, which seems like
a definition that will be hard and expensive to achieve.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-07-28 01:47 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Robert Haas @ 2022-07-28 01:47 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Daniel Shelepanov <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Wed, Jul 27, 2022 at 5:56 PM Tom Lane <[email protected]> wrote:
> Maybe we need a different function for pg_visibility to call?
> If we want ComputeXidHorizons to serve both these purposes, then it
> has to always deliver exactly the right answer, which seems like
> a definition that will be hard and expensive to achieve.
Yeah, I was thinking along similar lines.
I'm also kind of wondering why these calculations use
latestCompletedXid. Is that something we do solely to reduce locking?
The XIDs of running transactions matter, and their snapshots matter,
and the XIDs that could start running in the future matter, but I
don't know why it matters what the latest completed XID is.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-10-12 05:14 Michael Paquier <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2022-10-12 05:14 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Daniel Shelepanov <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Wed, Jul 27, 2022 at 09:47:19PM -0400, Robert Haas wrote:
> On Wed, Jul 27, 2022 at 5:56 PM Tom Lane <[email protected]> wrote:
> > Maybe we need a different function for pg_visibility to call?
> > If we want ComputeXidHorizons to serve both these purposes, then it
> > has to always deliver exactly the right answer, which seems like
> > a definition that will be hard and expensive to achieve.
>
> Yeah, I was thinking along similar lines.
>
> I'm also kind of wondering why these calculations use
> latestCompletedXid. Is that something we do solely to reduce locking?
> The XIDs of running transactions matter, and their snapshots matter,
> and the XIDs that could start running in the future matter, but I
> don't know why it matters what the latest completed XID is.
Daniel, it seems to me that this thread is waiting for some input from
you, based on the remarks of Tom and Robert. Are you planning to do
so? This is marked as a bug fix, so I have moved this item to the
next CF for now.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-11-07 13:30 Nikita Malakhov <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Nikita Malakhov @ 2022-11-07 13:30 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; Daniel Shelepanov <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi hackers!
Daniel is busy with other tasks. I've found this topic and this problem
seems to be actual
or v15 too.
Please correct me if I am wrong. I've checked another discussion related to
pg_visibility [1].
According to discussion: if using latest completed xid is not right for
checking visibility, than
it should be the least running transaction xid? So it must be another
function to be used for
these calculations, not the GetOldestNonRemovableTransactionId that uses
the ComputeXidHorizons.
[1]
https://www.postgresql.org/message-id/flat/c0610352-8433-ab4b-986d-0e803c628efe%40postgrespro.ru
On Wed, Oct 12, 2022 at 8:15 AM Michael Paquier <[email protected]> wrote:
> On Wed, Jul 27, 2022 at 09:47:19PM -0400, Robert Haas wrote:
> > On Wed, Jul 27, 2022 at 5:56 PM Tom Lane <[email protected]> wrote:
> > > Maybe we need a different function for pg_visibility to call?
> > > If we want ComputeXidHorizons to serve both these purposes, then it
> > > has to always deliver exactly the right answer, which seems like
> > > a definition that will be hard and expensive to achieve.
> >
> > Yeah, I was thinking along similar lines.
> >
> > I'm also kind of wondering why these calculations use
> > latestCompletedXid. Is that something we do solely to reduce locking?
> > The XIDs of running transactions matter, and their snapshots matter,
> > and the XIDs that could start running in the future matter, but I
> > don't know why it matters what the latest completed XID is.
>
> Daniel, it seems to me that this thread is waiting for some input from
> you, based on the remarks of Tom and Robert. Are you planning to do
> so? This is marked as a bug fix, so I have moved this item to the
> next CF for now.
> --
> Michael
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2022-12-14 18:56 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Nikita Malakhov @ 2022-12-14 18:56 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; Daniel Shelepanov <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi hackers!
Just to bump this thread, because the problem seems to be still actual:
Please correct me if I am wrong. I've checked another discussion related to
pg_visibility [1].
According to discussion: if using latest completed xid is not right for
checking visibility, than
it should be the least running transaction xid? So it must be another
function to be used for
these calculations, not the GetOldestNonRemovableTransactionId that uses
the ComputeXidHorizons.
[1]
https://www.postgresql.org/message-id/flat/c0610352-8433-ab4b-986d-0e803c628efe%40postgrespro.ru
On Wed, Oct 12, 2022 at 8:15 AM Michael Paquier <[email protected]> wrote:
>
>> On Wed, Jul 27, 2022 at 09:47:19PM -0400, Robert Haas wrote:
>> > On Wed, Jul 27, 2022 at 5:56 PM Tom Lane <[email protected]> wrote:
>> > > Maybe we need a different function for pg_visibility to call?
>> > > If we want ComputeXidHorizons to serve both these purposes, then it
>> > > has to always deliver exactly the right answer, which seems like
>> > > a definition that will be hard and expensive to achieve.
>> >
>> > Yeah, I was thinking along similar lines.
>> >
>> > I'm also kind of wondering why these calculations use
>> > latestCompletedXid. Is that something we do solely to reduce locking?
>> > The XIDs of running transactions matter, and their snapshots matter,
>> > and the XIDs that could start running in the future matter, but I
>> > don't know why it matters what the latest completed XID is.
>>
>> Daniel, it seems to me that this thread is waiting for some input from
>> you, based on the remarks of Tom and Robert. Are you planning to do
>> so? This is marked as a bug fix, so I have moved this item to the
>> next CF for now.
>> --
>> Michael
>>
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-07-04 07:21 Daniel Gustafsson <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Gustafsson @ 2023-07-04 07:21 UTC (permalink / raw)
To: Daniel Shelepanov <[email protected]>; +Cc: Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
This patch has been waiting on the author for about a year now, so I will close
it as Returned with Feedback. Plesae feel free to resubmit to a future CF when
there is renewed interest in working on this.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-11-06 09:30 Alexander Korotkov <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Korotkov @ 2023-11-06 09:30 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi!
On Tue, Jul 4, 2023 at 10:21 AM Daniel Gustafsson <[email protected]> wrote:
> This patch has been waiting on the author for about a year now, so I will close
> it as Returned with Feedback. Plesae feel free to resubmit to a future CF when
> there is renewed interest in working on this.
I'd like to revive this thread. While the patch proposed definitely
makes things better. But as pointed out by Robert and Tom, it didn't
allow to avoid all false reports. The reason is that the way we
currently calculate the oldest xmin, it could move backwards (see
comments to ComputeXidHorizons()). The attached patch implements own
function to calculate strict oldest xmin, which should be always
greater or equal to any xid horizon calculated before. I have to do
the following changes in comparison to what ComputeXidHorizons() do.
1. Ignore processes xmin's, because they take into account connection
to other databases which were ignored before.
2. Ignore KnownAssignedXids, because they are not database-aware.
While primary could compute its horizons database-aware.
3. Ignore walsender xmin, because it could go backward if some
replication connections don't use replication slots.
Surely these would significantly sacrifice accuracy. But we have to do
so in order to avoid reporting false errors.
Any thoughts?
------
Regards,
Alexander Korotkov
Attachments:
[application/octet-stream] 0001-Fix-false-reports-in-pg_visibility-v2.patch (4.7K, ../../CAPpHfdu6c=dQHq4r_xXbcAu0T4P28wv6x7QTwBZa7idv6nZsrA@mail.gmail.com/2-0001-Fix-false-reports-in-pg_visibility-v2.patch)
download | inline diff:
From 7723ba395dbf15a0badd6cdafda2752c19356baa Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 6 Nov 2023 11:07:35 +0200
Subject: [PATCH] Fix false reports in pg_visibility
---
contrib/pg_visibility/pg_visibility.c | 75 ++++++++++++++++++++++++---
1 file changed, 69 insertions(+), 6 deletions(-)
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 2a4acfd1eee..036a9fd7681 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -19,6 +19,7 @@
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
+#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/rel.h"
@@ -532,6 +533,58 @@ collect_visibility_data(Oid relid, bool include_pd)
return info;
}
+/*
+ * The "strict" version of GetOldestNonRemovableTransactionId(). The
+ * pg_visiblity check can tolerale false positives (don't report some of the
+ * errors), but can't toletate false negatives (report false errors).
+ * This makes us fundamentally unhappy with ComputeXidHorizons(). Normally,
+ * horozons moves forwards, but there are cases when it could move backwards
+ * (see comment for ComputeXidHorizons()).
+ *
+ * This is why we have to implement our own function for xid horizon, which
+ * would be guaranteed to be newer or equal to any xid horizon computed before.
+ * We have to do the following to achieve this.
+ *
+ * 1. Ignore processes xmin's, because they take into account connection to
+ * other databases which were ignored before.
+ * 2. Ignore KnownAssignedXids, because they are not database-aware. While
+ * primary could compute its horizons database-aware.
+ * 3. Ignore walsender xmin, because it could go backward if some replication
+ * connections don't use replication slots.
+ *
+ * Surely these would significantly sacrifice accuracy. But we have to do so
+ * in order to avoid reporting false errors.
+ */
+static TransactionId
+get_strict_xid_horizon(void)
+{
+ TransactionId xmin = InvalidTransactionId;
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+ for (int index = 0; index < ProcGlobal->allProcCount; index++)
+ {
+ TransactionId xid = (uint32) (*((volatile uint32 *) &ProcGlobal->xids[index]));
+ PGPROC *proc = &ProcGlobal->allProcs[index];
+ uint8 statusFlags;
+
+ if (likely(xid == InvalidTransactionId))
+ continue;
+
+ statusFlags = ProcGlobal->statusFlags[index];
+ if (statusFlags & (PROC_IN_LOGICAL_DECODING | PROC_IN_VACUUM))
+ continue;
+
+ if (proc->databaseId == MyDatabaseId ||
+ (statusFlags & PROC_AFFECTS_ALL_HORIZONS))
+ {
+ xmin = TransactionIdOlder(xmin, xid);
+ }
+ }
+ LWLockRelease(ProcArrayLock);
+
+ return xmin;
+}
+
/*
* Returns a list of items whose visibility map information does not match
* the status of the tuples on the page.
@@ -562,8 +615,14 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
/* Only some relkinds have a visibility map */
check_relation_relkind(rel);
+ /* Mark ourselves as PROC_IN_VACUUM to exclude our influence on xmin's */
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags |= PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
if (all_visible)
- OldestXmin = GetOldestNonRemovableTransactionId(rel);
+ OldestXmin = get_strict_xid_horizon();
nblocks = RelationGetNumberOfBlocks(rel);
@@ -670,12 +729,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
* From a concurrency point of view, it sort of sucks to
* retake ProcArrayLock here while we're holding the buffer
* exclusively locked, but it should be safe against
- * deadlocks, because surely
- * GetOldestNonRemovableTransactionId() should never take a
- * buffer lock. And this shouldn't happen often, so it's worth
- * being careful so as to avoid false positives.
+ * deadlocks, because surely get_strict_xid_horizon() should
+ * never take a buffer lock. And this shouldn't happen often,
+ * so it's worth being careful so as to avoid false positives.
*/
- RecomputedOldestXmin = GetOldestNonRemovableTransactionId(rel);
+ RecomputedOldestXmin = get_strict_xid_horizon();
if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin))
record_corrupt_item(items, &tuple.t_self);
@@ -715,6 +773,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
items->count = items->next;
items->next = 0;
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags &= ~PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
return items;
}
--
2.39.3 (Apple Git-145)
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-11-07 11:00 Alexander Lakhin <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Lakhin @ 2023-11-07 11:00 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Daniel Gustafsson <[email protected]>; +Cc: Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi Alexander,
06.11.2023 12:30, Alexander Korotkov wrote:
> Surely these would significantly sacrifice accuracy. But we have to do
> so in order to avoid reporting false errors.
>
I've reduced the dirty reproducer Daniel Shelepanov posted initially
to the following:
numdbs=10
for ((d=1;d<=$numdbs;d++)); do
createdb db$d
psql db$d -c "create extension pg_visibility"
done
for ((i=1;i<=300;i++)); do
echo "iteration $i"
for ((d=1;d<=$numdbs;d++)); do
(
echo "
create table vacuum_test as select 42 i;
vacuum (disable_page_skipping) vacuum_test;
select * from pg_check_visible('vacuum_test');
" | psql db$d -a -q >psql-$d.log 2>&1
) &
done
wait
res=0
for ((d=1;d<=$numdbs;d++)); do
grep -q '0 rows' psql-$d.log || { echo "Error condition in psql-$d.log:"; cat psql-$d.log; res=1; break; }
psql db$d -q -c "drop table vacuum_test"
done
[ $res == 0 ] || break;
done
It looks like the v2 patch doesn't fix the original issue. Maybe I miss
something, but with the patch applied, I see the failure immediately,
though without the patch several iterations are needed to get it.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-11-07 11:38 Alexander Korotkov <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Korotkov @ 2023-11-07 11:38 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi, Alexander.
On Tue, Nov 7, 2023 at 1:00 PM Alexander Lakhin <[email protected]> wrote:
> It looks like the v2 patch doesn't fix the original issue. Maybe I miss
> something, but with the patch applied, I see the failure immediately,
> though without the patch several iterations are needed to get it.
That's a bug in the patch. Thank you for cathing it. It should start
calculation from latestCompletedXid + 1, not InvalidTransactionId.
Please, check the revised patch.
------
Regards,
Alexander Korotkov
Attachments:
[application/octet-stream] 0001-Fix-false-reports-in-pg_visibility-v3.patch (4.8K, ../../CAPpHfdsVVAdaGAJgoOBAJ+6EGaCdsUi=wbgQAc3+8X_qtUr=Qg@mail.gmail.com/2-0001-Fix-false-reports-in-pg_visibility-v3.patch)
download | inline diff:
From 19ccad18b18d8ebdcfde6f4be102a02a4f6488ec Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 6 Nov 2023 11:07:35 +0200
Subject: [PATCH] Fix false reports in pg_visibility
---
contrib/pg_visibility/pg_visibility.c | 78 ++++++++++++++++++++++++---
1 file changed, 72 insertions(+), 6 deletions(-)
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 2a4acfd1eee..18721324207 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -19,6 +19,7 @@
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
+#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/rel.h"
@@ -532,6 +533,61 @@ collect_visibility_data(Oid relid, bool include_pd)
return info;
}
+/*
+ * The "strict" version of GetOldestNonRemovableTransactionId(). The
+ * pg_visiblity check can tolerale false positives (don't report some of the
+ * errors), but can't toletate false negatives (report false errors).
+ * This makes us fundamentally unhappy with ComputeXidHorizons(). Normally,
+ * horozons moves forwards, but there are cases when it could move backwards
+ * (see comment for ComputeXidHorizons()).
+ *
+ * This is why we have to implement our own function for xid horizon, which
+ * would be guaranteed to be newer or equal to any xid horizon computed before.
+ * We have to do the following to achieve this.
+ *
+ * 1. Ignore processes xmin's, because they take into account connection to
+ * other databases which were ignored before.
+ * 2. Ignore KnownAssignedXids, because they are not database-aware. While
+ * primary could compute its horizons database-aware.
+ * 3. Ignore walsender xmin, because it could go backward if some replication
+ * connections don't use replication slots.
+ *
+ * Surely these would significantly sacrifice accuracy. But we have to do so
+ * in order to avoid reporting false errors.
+ */
+static TransactionId
+get_strict_xid_horizon(void)
+{
+ TransactionId xmin;
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+ xmin = XidFromFullTransactionId(ShmemVariableCache->latestCompletedXid);
+ Assert(TransactionIdIsValid(xmin));
+ TransactionIdAdvance(xmin);
+ for (int index = 0; index < ProcGlobal->allProcCount; index++)
+ {
+ TransactionId xid = (uint32) (*((volatile uint32 *) &ProcGlobal->xids[index]));
+ PGPROC *proc = &ProcGlobal->allProcs[index];
+ uint8 statusFlags;
+
+ if (likely(xid == InvalidTransactionId))
+ continue;
+
+ statusFlags = ProcGlobal->statusFlags[index];
+ if (statusFlags & (PROC_IN_LOGICAL_DECODING | PROC_IN_VACUUM))
+ continue;
+
+ if (proc->databaseId == MyDatabaseId ||
+ (statusFlags & PROC_AFFECTS_ALL_HORIZONS))
+ {
+ xmin = TransactionIdOlder(xmin, xid);
+ }
+ }
+ LWLockRelease(ProcArrayLock);
+
+ return xmin;
+}
+
/*
* Returns a list of items whose visibility map information does not match
* the status of the tuples on the page.
@@ -562,8 +618,14 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
/* Only some relkinds have a visibility map */
check_relation_relkind(rel);
+ /* Mark ourselves as PROC_IN_VACUUM to exclude our influence on xmin's */
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags |= PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
if (all_visible)
- OldestXmin = GetOldestNonRemovableTransactionId(rel);
+ OldestXmin = get_strict_xid_horizon();
nblocks = RelationGetNumberOfBlocks(rel);
@@ -670,12 +732,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
* From a concurrency point of view, it sort of sucks to
* retake ProcArrayLock here while we're holding the buffer
* exclusively locked, but it should be safe against
- * deadlocks, because surely
- * GetOldestNonRemovableTransactionId() should never take a
- * buffer lock. And this shouldn't happen often, so it's worth
- * being careful so as to avoid false positives.
+ * deadlocks, because surely get_strict_xid_horizon() should
+ * never take a buffer lock. And this shouldn't happen often,
+ * so it's worth being careful so as to avoid false positives.
*/
- RecomputedOldestXmin = GetOldestNonRemovableTransactionId(rel);
+ RecomputedOldestXmin = get_strict_xid_horizon();
if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin))
record_corrupt_item(items, &tuple.t_self);
@@ -715,6 +776,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
items->count = items->next;
items->next = 0;
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags &= ~PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
return items;
}
--
2.39.3 (Apple Git-145)
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-11-07 13:00 Alexander Lakhin <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Lakhin @ 2023-11-07 13:00 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
07.11.2023 14:38, Alexander Korotkov wrote:
> Hi, Alexander.
>
> On Tue, Nov 7, 2023 at 1:00 PM Alexander Lakhin <[email protected]> wrote:
>> It looks like the v2 patch doesn't fix the original issue. Maybe I miss
>> something, but with the patch applied, I see the failure immediately,
>> though without the patch several iterations are needed to get it.
>
> That's a bug in the patch. Thank you for cathing it. It should start
> calculation from latestCompletedXid + 1, not InvalidTransactionId.
> Please, check the revised patch.
Thanks for looking at this!
Unfortunately, I still see the failure with the v3, but not on a first
iteration:
...
iteration 316
Error condition in psql-8.log:
create table vacuum_test as select 42 i;
vacuum (disable_page_skipping) vacuum_test;
select * from pg_check_visible('vacuum_test');
t_ctid
--------
(0,1)
(1 row)
(I've double-checked that the patch is applied and get_strict_xid_horizon()
is called.)
Best regards,
Alexander
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-12-04 00:23 Alexander Korotkov <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Korotkov @ 2023-12-04 00:23 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi, Alexander!
On Tue, Nov 7, 2023 at 3:00 PM Alexander Lakhin <[email protected]> wrote:
>
> 07.11.2023 14:38, Alexander Korotkov wrote:
> > Hi, Alexander.
> >
> > On Tue, Nov 7, 2023 at 1:00 PM Alexander Lakhin <[email protected]> wrote:
> >> It looks like the v2 patch doesn't fix the original issue. Maybe I miss
> >> something, but with the patch applied, I see the failure immediately,
> >> though without the patch several iterations are needed to get it.
> >
> > That's a bug in the patch. Thank you for cathing it. It should start
> > calculation from latestCompletedXid + 1, not InvalidTransactionId.
> > Please, check the revised patch.
>
> Thanks for looking at this!
> Unfortunately, I still see the failure with the v3, but not on a first
> iteration:
> ...
> iteration 316
> Error condition in psql-8.log:
> create table vacuum_test as select 42 i;
> vacuum (disable_page_skipping) vacuum_test;
> select * from pg_check_visible('vacuum_test');
> t_ctid
> --------
> (0,1)
> (1 row)
>
> (I've double-checked that the patch is applied and get_strict_xid_horizon()
> is called.)
I managed to reproduce this on a Linux VM. This problem should arise
because in extension I don't have access to ProcArrayStruct. So, my
code is iterating the whole PGPROC's array. I reimplemented the new
horizon calculation function in the core with usage of
ProcArrayStruct. Now it doesn't fall for me. Please, recheck.
------
Regards,
Alexander Korotkov
Attachments:
[application/octet-stream] 0001-Fix-false-reports-in-pg_visibility-v4.patch (8.0K, ../../CAPpHfdt8Rj_LtG8aWKNzeRejutrN-zt6fKQGuXYyAE7-zNVHbg@mail.gmail.com/2-0001-Fix-false-reports-in-pg_visibility-v4.patch)
download | inline diff:
From b3e2d15a0add4a55907195a05f38f1ce22857ade Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 6 Nov 2023 11:07:35 +0200
Subject: [PATCH] Fix false reports in pg_visibility
---
contrib/pg_visibility/pg_visibility.c | 23 +++++++---
src/backend/storage/ipc/procarray.c | 66 +++++++++++++++++++++++++++
src/include/storage/procarray.h | 1 +
3 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 2a4acfd1eee..78dac323e51 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -19,6 +19,7 @@
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
+#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/rel.h"
@@ -562,8 +563,14 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
/* Only some relkinds have a visibility map */
check_relation_relkind(rel);
+ /* Mark ourselves as PROC_IN_VACUUM to exclude our influence on xmin's */
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags |= PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
if (all_visible)
- OldestXmin = GetOldestNonRemovableTransactionId(rel);
+ OldestXmin = GetStrictOldestNonRemovableTransactionId();
nblocks = RelationGetNumberOfBlocks(rel);
@@ -670,12 +677,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
* From a concurrency point of view, it sort of sucks to
* retake ProcArrayLock here while we're holding the buffer
* exclusively locked, but it should be safe against
- * deadlocks, because surely
- * GetOldestNonRemovableTransactionId() should never take a
- * buffer lock. And this shouldn't happen often, so it's worth
- * being careful so as to avoid false positives.
+ * deadlocks, because surely get_strict_xid_horizon() should
+ * never take a buffer lock. And this shouldn't happen often,
+ * so it's worth being careful so as to avoid false positives.
*/
- RecomputedOldestXmin = GetOldestNonRemovableTransactionId(rel);
+ RecomputedOldestXmin = GetStrictOldestNonRemovableTransactionId();
if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin))
record_corrupt_item(items, &tuple.t_self);
@@ -715,6 +721,11 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
items->count = items->next;
items->next = 0;
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ MyProc->statusFlags &= ~PROC_IN_VACUUM;
+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
+ LWLockRelease(ProcArrayLock);
+
return items;
}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 80ab026bf56..e36a463cc66 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -237,6 +237,30 @@ typedef struct ComputeXidHorizonsResult
*/
TransactionId data_oldest_nonremovable;
+ /*
+ * The "strict" version of GetOldestNonRemovableTransactionId(). The
+ * pg_visiblity check can tolerale false positives (don't report some of the
+ * errors), but can't toletate false negatives (report false errors).
+ * This makes us fundamentally unhappy with ComputeXidHorizons(). Normally,
+ * horozons moves forwards, but there are cases when it could move backwards
+ * (see comment for ComputeXidHorizons()).
+ *
+ * This is why we have to implement our own function for xid horizon, which
+ * would be guaranteed to be newer or equal to any xid horizon computed before.
+ * We have to do the following to achieve this.
+ *
+ * 1. Ignore processes xmin's, because they take into account connection to
+ * other databases which were ignored before.
+ * 2. Ignore KnownAssignedXids, because they are not database-aware. While
+ * primary could compute its horizons database-aware.
+ * 3. Ignore walsender xmin, because it could go backward if some replication
+ * connections don't use replication slots.
+ *
+ * Surely these would significantly sacrifice accuracy. But we have to do so
+ * in order to avoid reporting false errors.
+ */
+ TransactionId data_strict_oldest_nonremovable;
+
/*
* Oldest xid for which deleted tuples need to be retained in this
* session's temporary tables.
@@ -252,6 +276,7 @@ typedef enum GlobalVisHorizonKind
VISHORIZON_SHARED,
VISHORIZON_CATALOG,
VISHORIZON_DATA,
+ VISHORIZON_DATA_STRICT,
VISHORIZON_TEMP,
} GlobalVisHorizonKind;
@@ -1743,6 +1768,7 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
h->oldest_considered_running = initial;
h->shared_oldest_nonremovable = initial;
h->data_oldest_nonremovable = initial;
+ h->data_strict_oldest_nonremovable = initial;
/*
* Only modifications made by this backend affect the horizon for
@@ -1842,6 +1868,10 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
{
h->data_oldest_nonremovable =
TransactionIdOlder(h->data_oldest_nonremovable, xmin);
+
+ if (TransactionIdIsValid(xid))
+ h->data_strict_oldest_nonremovable =
+ TransactionIdOlder(h->data_strict_oldest_nonremovable, xid);
}
}
@@ -1997,6 +2027,8 @@ GetOldestNonRemovableTransactionId(Relation rel)
return horizons.catalog_oldest_nonremovable;
case VISHORIZON_DATA:
return horizons.data_oldest_nonremovable;
+ case VISHORIZON_DATA_STRICT:
+ return horizons.data_strict_oldest_nonremovable;
case VISHORIZON_TEMP:
return horizons.temp_oldest_nonremovable;
}
@@ -2005,6 +2037,38 @@ GetOldestNonRemovableTransactionId(Relation rel)
return InvalidTransactionId;
}
+/*
+ * The "strict" version of GetOldestNonRemovableTransactionId(). The
+ * pg_visiblity check can tolerale false positives (don't report some of the
+ * errors), but can't toletate false negatives (report false errors).
+ * This makes us fundamentally unhappy with ComputeXidHorizons(). Normally,
+ * horozons moves forwards, but there are cases when it could move backwards
+ * (see comment for ComputeXidHorizons()).
+ *
+ * This is why we have to implement our own function for xid horizon, which
+ * would be guaranteed to be newer or equal to any xid horizon computed before.
+ * We have to do the following to achieve this.
+ *
+ * 1. Ignore processes xmin's, because they take into account connection to
+ * other databases which were ignored before.
+ * 2. Ignore KnownAssignedXids, because they are not database-aware. While
+ * primary could compute its horizons database-aware.
+ * 3. Ignore walsender xmin, because it could go backward if some replication
+ * connections don't use replication slots.
+ *
+ * Surely these would significantly sacrifice accuracy. But we have to do so
+ * in order to avoid reporting false errors.
+ */
+TransactionId
+GetStrictOldestNonRemovableTransactionId(void)
+{
+ ComputeXidHorizonsResult horizons;
+
+ ComputeXidHorizons(&horizons);
+
+ return horizons.data_strict_oldest_nonremovable;
+}
+
/*
* Return the oldest transaction id any currently running backend might still
* consider running. This should not be used for visibility / pruning
@@ -4028,6 +4092,8 @@ GlobalVisTestFor(Relation rel)
case VISHORIZON_TEMP:
state = &GlobalVisTempRels;
break;
+ default:
+ break;
}
Assert(FullTransactionIdIsValid(state->definitely_needed) &&
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index d8cae3ce1c5..d44c2a98b2d 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -55,6 +55,7 @@ extern RunningTransactions GetRunningTransactionData(void);
extern bool TransactionIdIsInProgress(TransactionId xid);
extern bool TransactionIdIsActive(TransactionId xid);
extern TransactionId GetOldestNonRemovableTransactionId(Relation rel);
+extern TransactionId GetStrictOldestNonRemovableTransactionId(void);
extern TransactionId GetOldestTransactionIdConsideredRunning(void);
extern TransactionId GetOldestActiveTransactionId(void);
extern TransactionId GetOldestSafeDecodingTransactionId(bool catalogOnly);
--
2.39.3 (Apple Git-145)
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: collect_corrupt_items_vacuum.patch
@ 2023-12-04 10:00 Alexander Lakhin <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Alexander Lakhin @ 2023-12-04 10:00 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Daniel Shelepanov <[email protected]>; Nikita Malakhov <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi Alexander,
04.12.2023 03:23, Alexander Korotkov wrote:
> I managed to reproduce this on a Linux VM. This problem should arise
> because in extension I don't have access to ProcArrayStruct. So, my
> code is iterating the whole PGPROC's array. I reimplemented the new
> horizon calculation function in the core with usage of
> ProcArrayStruct. Now it doesn't fall for me. Please, recheck.
Yes, v4 works for me as well (thousands of iterations passed).
Thank you!
Though the test passes even without manipulations with the PROC_IN_VACUUM
flag in pg_visibility.c (maybe the test is not good enough to show why
those manipulations are needed).
I also couldn't see where VISHORIZON_DATA_STRICT comes into play...
Best regards,
Alexander
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2023-12-04 10:00 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 18:16 [PATCH v22 03/10] Add tests on pg_ls_dir before changing it Justin Pryzby <[email protected]>
2022-04-04 08:51 collect_corrupt_items_vacuum.patch Daniel Shelepanov <[email protected]>
2022-07-27 21:50 ` Re: collect_corrupt_items_vacuum.patch Robert Haas <[email protected]>
2022-07-27 21:55 ` Re: collect_corrupt_items_vacuum.patch Tom Lane <[email protected]>
2022-07-28 01:47 ` Re: collect_corrupt_items_vacuum.patch Robert Haas <[email protected]>
2022-10-12 05:14 ` Re: collect_corrupt_items_vacuum.patch Michael Paquier <[email protected]>
2022-11-07 13:30 ` Re: collect_corrupt_items_vacuum.patch Nikita Malakhov <[email protected]>
2022-12-14 18:56 ` Re: collect_corrupt_items_vacuum.patch Nikita Malakhov <[email protected]>
2023-07-04 07:21 ` Re: collect_corrupt_items_vacuum.patch Daniel Gustafsson <[email protected]>
2023-11-06 09:30 ` Re: collect_corrupt_items_vacuum.patch Alexander Korotkov <[email protected]>
2023-11-07 11:00 ` Re: collect_corrupt_items_vacuum.patch Alexander Lakhin <[email protected]>
2023-11-07 11:38 ` Re: collect_corrupt_items_vacuum.patch Alexander Korotkov <[email protected]>
2023-11-07 13:00 ` Re: collect_corrupt_items_vacuum.patch Alexander Lakhin <[email protected]>
2023-12-04 00:23 ` Re: collect_corrupt_items_vacuum.patch Alexander Korotkov <[email protected]>
2023-12-04 10:00 ` Re: collect_corrupt_items_vacuum.patch Alexander Lakhin <[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