Dear Horiguchi-san, Dilip,
Thank you for replying!
> > It seems that SnapBuildCommitTxn() is already taking care of adding
> > the top transaction to the committed transaction if any subtransaction
> > has the catalog changes, it has just missed setting the flag so I
> > think just setting the flag like this should be sufficient no?
>
> Oops! That's right.
Basically I agreed, but I was not sure the message "found top level
transaction..."
should be output or not. It may be useful even if one of sub transactions
contains the change.
How about following?
diff --git a/src/backend/replication/logical/snapbuild.c
b/src/backend/replication/logical/snapbuild.c
index bf72ad45ec..a630522907 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1086,8 +1086,17 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn,
TransactionId xid,
}
}
- /* if top-level modified catalog, it'll need a snapshot */
- if (SnapBuildXidHasCatalogChanges(builder, xid, xinfo))
+ /*
+ * if top-level or one of sub modified catalog, it'll need a snapshot.
+ *
+ * Normally the second check is not needed because the relation between
+ * top-sub transactions is tracked on the ReorderBuffer layer, and the
top
+ * transaction is marked as containing catalog changes if its children
are.
+ * But in some cases the relation may be missed, in which case only the
sub
+ * transaction may be marked as containing catalog changes.
+ */
+ if (SnapBuildXidHasCatalogChanges(builder, xid, xinfo)
+ || sub_needs_timetravel)
{
elog(DEBUG2, "found top level transaction %u, with catalog
changes",
xid);
@@ -1095,11 +1104,6 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn,
TransactionId xid,
needs_timetravel = true;
SnapBuildAddCommittedTxn(builder, xid);
}
- else if (sub_needs_timetravel)
- {
- /* track toplevel txn as well, subxact alone isn't meaningful */
- SnapBuildAddCommittedTxn(builder, xid);
- }
else if (needs_timetravel)
{
elog(DEBUG2, "forced transaction %u to do timetravel", xid);
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
