Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 240e0db

Browse files
author
Amit Kapila
committed
Add additional checks while creating the initial decoding snapshot.
As per one of the CI reports, there is an assertion failure which indicates that we were trying to use an unenforced xmin horizon for decoding snapshots. Though, we couldn't figure out the reason for assertion failure these checks would help us in finding the reason if the problem happens again in the future. Author: Amit Kapila based on suggestions by Andres Freund Reviewd by: Andres Freund Discussion: https://postgr.es/m/CAA4eK1L8wYcyTPxNzPGkhuO52WBGoOZbT0A73Le=ZUWYAYmdfw@mail.gmail.com
1 parent a4adc31 commit 240e0db

File tree

2 files changed

+23
-11
lines changed

2 files changed

+23
-11
lines changed

src/backend/replication/logical/snapbuild.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -566,11 +566,18 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
566566
{
567567
Snapshot snap;
568568
TransactionId xid;
569+
TransactionId safeXid;
569570
TransactionId *newxip;
570571
int newxcnt = 0;
571572

572-
Assert(!FirstSnapshotSet);
573573
Assert(XactIsoLevel == XACT_REPEATABLE_READ);
574+
Assert(builder->building_full_snapshot);
575+
576+
/* don't allow older snapshots */
577+
InvalidateCatalogSnapshot(); /* about to overwrite MyProc->xmin */
578+
if (HaveRegisteredOrActiveSnapshot())
579+
elog(ERROR, "cannot build an initial slot snapshot when snapshots exist");
580+
Assert(!HistoricSnapshotActive());
574581

575582
if (builder->state != SNAPBUILD_CONSISTENT)
576583
elog(ERROR, "cannot build an initial slot snapshot before reaching a consistent state");
@@ -588,18 +595,18 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
588595
* We know that snap->xmin is alive, enforced by the logical xmin
589596
* mechanism. Due to that we can do this without locks, we're only
590597
* changing our own value.
598+
*
599+
* Building an initial snapshot is expensive and an unenforced xmin
600+
* horizon would have bad consequences, therefore always double-check that
601+
* the horizon is enforced.
591602
*/
592-
#ifdef USE_ASSERT_CHECKING
593-
{
594-
TransactionId safeXid;
603+
LWLockAcquire(ProcArrayLock, LW_SHARED);
604+
safeXid = GetOldestSafeDecodingTransactionId(false);
605+
LWLockRelease(ProcArrayLock);
595606

596-
LWLockAcquire(ProcArrayLock, LW_SHARED);
597-
safeXid = GetOldestSafeDecodingTransactionId(false);
598-
LWLockRelease(ProcArrayLock);
599-
600-
Assert(TransactionIdPrecedesOrEquals(safeXid, snap->xmin));
601-
}
602-
#endif
607+
if (TransactionIdFollows(safeXid, snap->xmin))
608+
elog(ERROR, "cannot build an initial slot snapshot as oldest safe xid %u follows snapshot's xmin %u",
609+
safeXid, snap->xmin);
603610

604611
MyProc->xmin = snap->xmin;
605612

src/backend/replication/walsender.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,11 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
10991099
/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
11001100
(errmsg("%s must be called in REPEATABLE READ isolation mode transaction",
11011101
"CREATE_REPLICATION_SLOT ... (SNAPSHOT 'use')")));
1102+
if (!XactReadOnly)
1103+
ereport(ERROR,
1104+
/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
1105+
(errmsg("%s must be called in a read only transaction",
1106+
"CREATE_REPLICATION_SLOT ... (SNAPSHOT 'use')")));
11021107

11031108
if (FirstSnapshotSet)
11041109
ereport(ERROR,

0 commit comments

Comments
 (0)