On Wed, Mar 17, 2021 at 11:27 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 5. I have modified the comments atop worker.c to explain the design > and some of the problems clearly. See attached. If you are fine with > this, please include it in the next version of the patch. >
I have further expanded these comments to explain the handling of
prepared transactions for multiple subscriptions on the same server
especially when the same prepared transaction operates on tables for
those subscriptions. See attached, this applies atop the patch sent by
me in the last email. I am not sure but I think it might be better to
add something on those lines in user-facing docs. What do you think?
Another comment:
+ ereport(LOG,
+ (errmsg("logical replication apply worker for subscription \"%s\" 2PC is %s.",
+ MySubscription->name,
+ MySubscription->twophase == LOGICALREP_TWOPHASE_STATE_DISABLED ? "DISABLED" :
+ MySubscription->twophase == LOGICALREP_TWOPHASE_STATE_PENDING ? "PENDING" :
+ MySubscription->twophase == LOGICALREP_TWOPHASE_STATE_ENABLED ? "ENABLED" :
+ "?")));
I don't think this is required in LOGs, maybe at some DEBUG level,
because users can check this in pg_subscription. If we keep this
message, there will be two consecutive messages like below in logs for
subscriptions that have two_pc option enabled which looks a bit odd.
LOG: logical replication apply worker for subscription "mysub" has started
LOG: logical replication apply worker for subscription "mysub" 2PC is ENABLED.
--
With Regards,
Amit Kapila.
change_two_phase_desc_2.patch
Description: Binary data
