Fix assertion failure in logical decoding.
Logical decoding set SnapshotData's regd_count field to avoid the snapshot manager from prematurely freeing snapshots that are generated by the decoding system. That was always an abuse of the field, as it was never supposed to be used outside the snapshot manager. Commit 94028691 made snapshot manager's tracking of the snapshots smarter, and that scheme fell apart. The snapshot manager got confused and hit the assertion, when a snapshot that was marked with regd_count==1 was not found in the heap, where the snapshot manager tracks registered the snapshots. To fix, don't abuse the regd_count field like that. Logical decoding still abuses the active_count field for similar purposes, but that's currently harmless. The assertion failure was first reported by Michael Paquier
This commit is contained in:
parent
90898af30b
commit
e2999abcd1
@ -1188,8 +1188,8 @@ ReorderBufferCopySnap(ReorderBuffer *rb, Snapshot orig_snap,
|
|||||||
memcpy(snap, orig_snap, sizeof(SnapshotData));
|
memcpy(snap, orig_snap, sizeof(SnapshotData));
|
||||||
|
|
||||||
snap->copied = true;
|
snap->copied = true;
|
||||||
snap->active_count = 0;
|
snap->active_count = 1; /* mark as active so nobody frees it */
|
||||||
snap->regd_count = 1;
|
snap->regd_count = 0;
|
||||||
snap->xip = (TransactionId *) (snap + 1);
|
snap->xip = (TransactionId *) (snap + 1);
|
||||||
|
|
||||||
memcpy(snap->xip, orig_snap->xip, sizeof(TransactionId) * snap->xcnt);
|
memcpy(snap->xip, orig_snap->xip, sizeof(TransactionId) * snap->xcnt);
|
||||||
|
@ -348,7 +348,7 @@ SnapBuildFreeSnapshot(Snapshot snap)
|
|||||||
Assert(snap->curcid == FirstCommandId);
|
Assert(snap->curcid == FirstCommandId);
|
||||||
Assert(!snap->suboverflowed);
|
Assert(!snap->suboverflowed);
|
||||||
Assert(!snap->takenDuringRecovery);
|
Assert(!snap->takenDuringRecovery);
|
||||||
Assert(snap->regd_count == 1);
|
Assert(snap->regd_count == 0);
|
||||||
|
|
||||||
/* slightly more likely, so it's checked even without c-asserts */
|
/* slightly more likely, so it's checked even without c-asserts */
|
||||||
if (snap->copied)
|
if (snap->copied)
|
||||||
@ -407,16 +407,16 @@ SnapBuildSnapDecRefcount(Snapshot snap)
|
|||||||
Assert(!snap->suboverflowed);
|
Assert(!snap->suboverflowed);
|
||||||
Assert(!snap->takenDuringRecovery);
|
Assert(!snap->takenDuringRecovery);
|
||||||
|
|
||||||
Assert(snap->regd_count == 1);
|
Assert(snap->regd_count == 0);
|
||||||
|
|
||||||
Assert(snap->active_count);
|
Assert(snap->active_count > 0);
|
||||||
|
|
||||||
/* slightly more likely, so it's checked even without casserts */
|
/* slightly more likely, so it's checked even without casserts */
|
||||||
if (snap->copied)
|
if (snap->copied)
|
||||||
elog(ERROR, "cannot free a copied snapshot");
|
elog(ERROR, "cannot free a copied snapshot");
|
||||||
|
|
||||||
snap->active_count--;
|
snap->active_count--;
|
||||||
if (!snap->active_count)
|
if (snap->active_count == 0)
|
||||||
SnapBuildFreeSnapshot(snap);
|
SnapBuildFreeSnapshot(snap);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -495,7 +495,7 @@ SnapBuildBuildSnapshot(SnapBuild *builder, TransactionId xid)
|
|||||||
snapshot->copied = false;
|
snapshot->copied = false;
|
||||||
snapshot->curcid = FirstCommandId;
|
snapshot->curcid = FirstCommandId;
|
||||||
snapshot->active_count = 0;
|
snapshot->active_count = 0;
|
||||||
snapshot->regd_count = 1; /* mark as registered so nobody frees it */
|
snapshot->regd_count = 0;
|
||||||
|
|
||||||
return snapshot;
|
return snapshot;
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user