Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)
Hi,
Per Coverity.
If xid is a subtransaction, the setup of base snapshot on the top-level
transaction,
can be not optional, otherwise a Dereference null return value
(NULL_RETURNS) can be raised.
Patch suggestion to fix this.
diff --git a/src/backend/replication/logical/reorderbuffer.c
b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..3c6a81f716 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb,
TransactionId xid,
*/
txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true);
if (rbtxn_is_known_subxact(txn))
- txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
- NULL, InvalidXLogRecPtr, false);
+ txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true,
+ NULL, InvalidXLogRecPtr, true);
Assert(txn->base_snapshot == NULL);txn->base_snapshot = snap;
regards,
Ranier Vilela
Attachments:
reorderbuffer.patchapplication/octet-stream; name=reorderbuffer.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..3c6a81f716 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
*/
txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true);
if (rbtxn_is_known_subxact(txn))
- txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
- NULL, InvalidXLogRecPtr, false);
+ txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true,
+ NULL, InvalidXLogRecPtr, true);
Assert(txn->base_snapshot == NULL);
txn->base_snapshot = snap;
At Wed, 10 Feb 2021 20:12:38 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in
Hi,
Per Coverity.
If xid is a subtransaction, the setup of base snapshot on the top-level
transaction,
can be not optional, otherwise a Dereference null return value
(NULL_RETURNS) can be raised.Patch suggestion to fix this.
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 5a62ab8bbc..3c6a81f716 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid, */ txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true); if (rbtxn_is_known_subxact(txn)) - txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false, - NULL, InvalidXLogRecPtr, false); + txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true, + NULL, InvalidXLogRecPtr, true); Assert(txn->base_snapshot == NULL);
If the return from the first call is a subtransaction, the second call
always obtain the top transaction. If the top transaction actualy did
not exist, it's rather the correct behavior to cause SEGV, than
creating a bogus rbtxn. THus it is wrong to set create=true and
create_as_top=true. We could change the assertion like Assert (txn &&
txn->base_snapshot) to make things clearer.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Em sex., 12 de fev. de 2021 às 03:56, Kyotaro Horiguchi <
horikyota.ntt@gmail.com> escreveu:
At Wed, 10 Feb 2021 20:12:38 -0300, Ranier Vilela <ranier.vf@gmail.com>
wrote inHi,
Per Coverity.
If xid is a subtransaction, the setup of base snapshot on the top-level
transaction,
can be not optional, otherwise a Dereference null return value
(NULL_RETURNS) can be raised.Patch suggestion to fix this.
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 5a62ab8bbc..3c6a81f716 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid, */ txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true); if (rbtxn_is_known_subxact(txn)) - txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false, - NULL, InvalidXLogRecPtr, false); + txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true, + NULL, InvalidXLogRecPtr, true); Assert(txn->base_snapshot == NULL);If the return from the first call is a subtransaction, the second call
always obtain the top transaction. If the top transaction actualy did
not exist, it's rather the correct behavior to cause SEGV, than
creating a bogus rbtxn. THus it is wrong to set create=true and
create_as_top=true. We could change the assertion like Assert (txn &&
txn->base_snapshot) to make things clearer.
It does not make sense to generate a SEGV on purpose and
assertion would not solve why this happens in a production environment.
Better to report an error if the second call fails.
What do you suggest as a description of the error?
regards,
Ranier Vilela
On Fri, Feb 12, 2021 at 03:56:02PM +0900, Kyotaro Horiguchi wrote:
If the return from the first call is a subtransaction, the second call
always obtain the top transaction. If the top transaction actualy did
not exist, it's rather the correct behavior to cause SEGV, than
creating a bogus rbtxn. THus it is wrong to set create=true and
create_as_top=true. We could change the assertion like Assert (txn &&
txn->base_snapshot) to make things clearer.
I don't see much the point to change this code. The result would be
the same: a PANIC at this location.
--
Michael
Hi,
How about the following patch ?
ReorderBufferSetBaseSnapshot() can return a bool to indicate whether the
base snapshot is set up.
For the call by SnapBuildCommitTxn(), it seems xid is top transaction. So
the return value doesn't need to be checked.
Cheers
On Fri, Feb 12, 2021 at 6:40 PM Michael Paquier <michael@paquier.xyz> wrote:
Show quoted text
On Fri, Feb 12, 2021 at 03:56:02PM +0900, Kyotaro Horiguchi wrote:
If the return from the first call is a subtransaction, the second call
always obtain the top transaction. If the top transaction actualy did
not exist, it's rather the correct behavior to cause SEGV, than
creating a bogus rbtxn. THus it is wrong to set create=true and
create_as_top=true. We could change the assertion like Assert (txn &&
txn->base_snapshot) to make things clearer.I don't see much the point to change this code. The result would be
the same: a PANIC at this location.
--
Michael
Attachments:
reorder-buffer-base-snapshot.patchapplication/octet-stream; name=reorder-buffer-base-snapshot.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..e3a7c038ca 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2977,8 +2977,9 @@ ReorderBufferAddSnapshot(ReorderBuffer *rb, TransactionId xid,
*
* If we know that xid is a subtransaction, set the base snapshot on the
* top-level transaction instead.
+ * Returns true if base snapshot is set up. Returns false otherwise.
*/
-void
+bool
ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
XLogRecPtr lsn, Snapshot snap)
{
@@ -2995,6 +2996,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
if (rbtxn_is_known_subxact(txn))
txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
NULL, InvalidXLogRecPtr, false);
+ if (txn == NULL)
+ return false;
Assert(txn->base_snapshot == NULL);
txn->base_snapshot = snap;
@@ -3002,6 +3005,7 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
dlist_push_tail(&rb->txns_by_base_snapshot_lsn, &txn->base_snapshot_node);
AssertTXNLsnOrder(rb);
+ return true;
}
/*
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e903e561af..d4380fe0f6 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -750,8 +750,9 @@ SnapBuildProcessChange(SnapBuild *builder, TransactionId xid, XLogRecPtr lsn)
* out to.
*/
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
- builder->snapshot);
+ if (!ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ builder->snapshot))
+ return false;
}
return true;
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bab31bf7af..926d1dc049 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -654,7 +654,7 @@ void ReorderBufferAbortOld(ReorderBuffer *, TransactionId xid);
void ReorderBufferForget(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
void ReorderBufferInvalidate(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
-void ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
+bool ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddNewCommandId(ReorderBuffer *, TransactionId, XLogRecPtr lsn,
CommandId cid);
Em sáb., 13 de fev. de 2021 às 01:07, Zhihong Yu <zyu@yugabyte.com>
escreveu:
Hi,
How about the following patch ?ReorderBufferSetBaseSnapshot() can return a bool to indicate whether the
base snapshot is set up.For the call by SnapBuildCommitTxn(), it seems xid is top transaction. So
the return value doesn't need to be checked.
IMO anything else is better than PANIC.
Anyway, if all fails, reporting an error can contribute to checking where.
Attached a patch suggestion v2.
1. SnapBuildProcessChange returns a result of ReorderBufferSetBaseSnapshot,
so the caller can act accordingly.
2. SnapBuildCommitTxn can't ignore a result
from ReorderBufferSetBaseSnapshot, even if it never fails.
regards,
Ranier Vilela
Attachments:
reorderbuffer.patchapplication/octet-stream; name=reorderbuffer.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..324f3f434f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2978,7 +2978,7 @@ ReorderBufferAddSnapshot(ReorderBuffer *rb, TransactionId xid,
* If we know that xid is a subtransaction, set the base snapshot on the
* top-level transaction instead.
*/
-void
+bool
ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
XLogRecPtr lsn, Snapshot snap)
{
@@ -2995,6 +2995,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
if (rbtxn_is_known_subxact(txn))
txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
NULL, InvalidXLogRecPtr, false);
+ if (txn == NULL)
+ return false;
Assert(txn->base_snapshot == NULL);
txn->base_snapshot = snap;
@@ -3002,6 +3004,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
dlist_push_tail(&rb->txns_by_base_snapshot_lsn, &txn->base_snapshot_node);
AssertTXNLsnOrder(rb);
+
+ return true;
}
/*
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e903e561af..f9d7bfa3ee 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -750,7 +750,7 @@ SnapBuildProcessChange(SnapBuild *builder, TransactionId xid, XLogRecPtr lsn)
* out to.
*/
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ return ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
builder->snapshot);
}
@@ -1071,8 +1071,13 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
if (!ReorderBufferXidHasBaseSnapshot(builder->reorder, xid))
{
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
- builder->snapshot);
+ if (!ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ builder->snapshot))
+ ereport(FATAL,
+ (errmsg("BaseSnapshot cant't be setup at point %X/%X",
+ (uint32) (lsn >> 32), (uint32) lsn),
+ errdetail("Top transaction is running.")));
+
}
/* refcount of the snapshot builder for the new snapshot */
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bab31bf7af..926d1dc049 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -654,7 +654,7 @@ void ReorderBufferAbortOld(ReorderBuffer *, TransactionId xid);
void ReorderBufferForget(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
void ReorderBufferInvalidate(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
-void ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
+bool ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddNewCommandId(ReorderBuffer *, TransactionId, XLogRecPtr lsn,
CommandId cid);Em sáb., 13 de fev. de 2021 às 17:35, Ranier Vilela <ranier.vf@gmail.com>
escreveu:
Em sáb., 13 de fev. de 2021 às 01:07, Zhihong Yu <zyu@yugabyte.com>
escreveu:Hi,
How about the following patch ?ReorderBufferSetBaseSnapshot() can return a bool to indicate whether the
base snapshot is set up.For the call by SnapBuildCommitTxn(), it seems xid is top transaction. So
the return value doesn't need to be checked.IMO anything else is better than PANIC.
Anyway, if all fails, reporting an error can contribute to checking where.Attached a patch suggestion v2.
Sorry, I forgot to mention, it is based on a patch from Zhihong Yu.
regards,
Ranier Vilela
Hi,
+ (errmsg("BaseSnapshot cant't be setup at point %X/%X",
+ (uint32) (lsn >> 32), (uint32) lsn),
+ errdetail("Top transaction is running.")));Did you mean this errdetail:
Top transaction is not running.
Cheers
On Sat, Feb 13, 2021 at 12:34 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
Show quoted text
Em sáb., 13 de fev. de 2021 às 01:07, Zhihong Yu <zyu@yugabyte.com>
escreveu:Hi,
How about the following patch ?ReorderBufferSetBaseSnapshot() can return a bool to indicate whether the
base snapshot is set up.For the call by SnapBuildCommitTxn(), it seems xid is top transaction. So
the return value doesn't need to be checked.IMO anything else is better than PANIC.
Anyway, if all fails, reporting an error can contribute to checking where.Attached a patch suggestion v2.
1. SnapBuildProcessChange returns a result of
ReorderBufferSetBaseSnapshot, so the caller can act accordingly.
2. SnapBuildCommitTxn can't ignore a result
from ReorderBufferSetBaseSnapshot, even if it never fails.regards,
Ranier Vilela
Em sáb., 13 de fev. de 2021 às 17:48, Zhihong Yu <zyu@yugabyte.com>
escreveu:
Hi, + (errmsg("BaseSnapshot cant't be setup at point %X/%X", + (uint32) (lsn >> 32), (uint32) lsn), + errdetail("Top transaction is running.")));Did you mean this errdetail:
Top transaction is not running.
Done.
Thanks Zhihong.
v3 based on your patch, attached.
regards,
Ranier Vilela
Attachments:
reorderbuffer_v3.patchapplication/octet-stream; name=reorderbuffer_v3.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..324f3f434f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2978,7 +2978,7 @@ ReorderBufferAddSnapshot(ReorderBuffer *rb, TransactionId xid,
* If we know that xid is a subtransaction, set the base snapshot on the
* top-level transaction instead.
*/
-void
+bool
ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
XLogRecPtr lsn, Snapshot snap)
{
@@ -2995,6 +2995,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
if (rbtxn_is_known_subxact(txn))
txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
NULL, InvalidXLogRecPtr, false);
+ if (txn == NULL)
+ return false;
Assert(txn->base_snapshot == NULL);
txn->base_snapshot = snap;
@@ -3002,6 +3004,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
dlist_push_tail(&rb->txns_by_base_snapshot_lsn, &txn->base_snapshot_node);
AssertTXNLsnOrder(rb);
+
+ return true;
}
/*
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e903e561af..f9d7bfa3ee 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -750,7 +750,7 @@ SnapBuildProcessChange(SnapBuild *builder, TransactionId xid, XLogRecPtr lsn)
* out to.
*/
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ return ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
builder->snapshot);
}
@@ -1071,8 +1071,13 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
if (!ReorderBufferXidHasBaseSnapshot(builder->reorder, xid))
{
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
- builder->snapshot);
+ if (!ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ builder->snapshot))
+ ereport(FATAL,
+ (errmsg("BaseSnapshot cant't be setup at point %X/%X",
+ (uint32) (lsn >> 32), (uint32) lsn),
+ errdetail("Top transaction is not running.")));
+
}
/* refcount of the snapshot builder for the new snapshot */
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bab31bf7af..926d1dc049 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -654,7 +654,7 @@ void ReorderBufferAbortOld(ReorderBuffer *, TransactionId xid);
void ReorderBufferForget(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
void ReorderBufferInvalidate(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
-void ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
+bool ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddNewCommandId(ReorderBuffer *, TransactionId, XLogRecPtr lsn,
CommandId cid);Hi,
Patch v4 corrects a small typo:
+ (errmsg("BaseSnapshot cant't be setup at point %X/%X",
Cheers
On Sat, Feb 13, 2021 at 12:58 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
Show quoted text
Em sáb., 13 de fev. de 2021 às 17:48, Zhihong Yu <zyu@yugabyte.com>
escreveu:Hi, + (errmsg("BaseSnapshot cant't be setup at point %X/%X", + (uint32) (lsn >> 32), (uint32) lsn), + errdetail("Top transaction is running.")));Did you mean this errdetail:
Top transaction is not running.
Done.
Thanks Zhihong.
v3 based on your patch, attached.regards,
Ranier Vilela
Attachments:
reorder-buffer-v4.patchapplication/octet-stream; name=reorder-buffer-v4.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5a62ab8bbc..324f3f434f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2978,7 +2978,7 @@ ReorderBufferAddSnapshot(ReorderBuffer *rb, TransactionId xid,
* If we know that xid is a subtransaction, set the base snapshot on the
* top-level transaction instead.
*/
-void
+bool
ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
XLogRecPtr lsn, Snapshot snap)
{
@@ -2995,6 +2995,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
if (rbtxn_is_known_subxact(txn))
txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
NULL, InvalidXLogRecPtr, false);
+ if (txn == NULL)
+ return false;
Assert(txn->base_snapshot == NULL);
txn->base_snapshot = snap;
@@ -3002,6 +3004,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, TransactionId xid,
dlist_push_tail(&rb->txns_by_base_snapshot_lsn, &txn->base_snapshot_node);
AssertTXNLsnOrder(rb);
+
+ return true;
}
/*
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e903e561af..f9d7bfa3ee 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -750,7 +750,7 @@ SnapBuildProcessChange(SnapBuild *builder, TransactionId xid, XLogRecPtr lsn)
* out to.
*/
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ return ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
builder->snapshot);
}
@@ -1071,8 +1071,13 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
if (!ReorderBufferXidHasBaseSnapshot(builder->reorder, xid))
{
SnapBuildSnapIncRefcount(builder->snapshot);
- ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
- builder->snapshot);
+ if (!ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn,
+ builder->snapshot))
+ ereport(FATAL,
+ (errmsg("BaseSnapshot can't be setup at point %X/%X",
+ (uint32) (lsn >> 32), (uint32) lsn),
+ errdetail("Top transaction is not running.")));
+
}
/* refcount of the snapshot builder for the new snapshot */
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bab31bf7af..926d1dc049 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -654,7 +654,7 @@ void ReorderBufferAbortOld(ReorderBuffer *, TransactionId xid);
void ReorderBufferForget(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
void ReorderBufferInvalidate(ReorderBuffer *, TransactionId, XLogRecPtr lsn);
-void ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
+bool ReorderBufferSetBaseSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddSnapshot(ReorderBuffer *, TransactionId, XLogRecPtr lsn, struct SnapshotData *snap);
void ReorderBufferAddNewCommandId(ReorderBuffer *, TransactionId, XLogRecPtr lsn,
CommandId cid);