Fix comments atop ReorderBufferAddInvalidations
The comments atop seem to indicate that we always accumulate
invalidation messages in top-level transactions which is neither
required nor match with the code. This is introduced in the commit
c55040ccd0 and I have observed it while working on a fix for commit
16b1fe0037.
--
With Regards,
Amit Kapila.
Attachments:
fix_comments_rb_invalidations_1.patchapplication/octet-stream; name=fix_comments_rb_invalidations_1.patchDownload
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 22a15a482a..31f7381f2d 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -3208,16 +3208,17 @@ ReorderBufferAddNewTupleCids(ReorderBuffer *rb, TransactionId xid,
}
/*
- * Setup the invalidation of the toplevel transaction.
+ * Accumulate the invalidations for executing them later.
*
* This needs to be called for each XLOG_XACT_INVALIDATIONS message and
- * accumulates all the invalidation messages in the toplevel transaction as
- * well as in the form of change in reorder buffer. We require to record it in
- * form of the change so that we can execute only the required invalidations
- * instead of executing all the invalidations on each CommandId increment. We
- * also need to accumulate these in the toplevel transaction because in some
- * cases we skip processing the transaction (see ReorderBufferForget), we need
- * to execute all the invalidations together.
+ * accumulates all the invalidation messages in the toplevel transaction, if
+ * available, otherwise in the current transaction, as well as in the form of
+ * change in reorder buffer. We require to record it in form of the change
+ * so that we can execute only the required invalidations instead of executing
+ * all the invalidations on each CommandId increment. We also need to
+ * accumulate these in the txn buffer because in some cases where we skip
+ * processing the transaction (see ReorderBufferForget), we need to execute
+ * all the invalidations together.
*/
void
ReorderBufferAddInvalidations(ReorderBuffer *rb, TransactionId xid,
@@ -3233,8 +3234,9 @@ ReorderBufferAddInvalidations(ReorderBuffer *rb, TransactionId xid,
oldcontext = MemoryContextSwitchTo(rb->context);
/*
- * Collect all the invalidations under the top transaction so that we can
- * execute them all together. See comment atop this function
+ * Collect all the invalidations under the top transaction, if available,
+ * so that we can execute them all together. See comments atop this
+ * function.
*/
if (txn->toptxn)
txn = txn->toptxn;
Hi
On Thu, Nov 3, 2022 at 7:53 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
The comments atop seem to indicate that we always accumulate
invalidation messages in top-level transactions which is neither
required nor match with the code. This is introduced in the commit
c55040ccd0 and I have observed it while working on a fix for commit
16b1fe0037.
Thank you for the patch. It looks good to me.
I think we can backpatch it to avoid confusion in future.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On Fri, Nov 4, 2022 at 11:14 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Thu, Nov 3, 2022 at 7:53 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
The comments atop seem to indicate that we always accumulate
invalidation messages in top-level transactions which is neither
required nor match with the code. This is introduced in the commit
c55040ccd0 and I have observed it while working on a fix for commit
16b1fe0037.Thank you for the patch. It looks good to me.
I think we can backpatch it to avoid confusion in future.
Pushed.
--
With Regards,
Amit Kapila.