On Sat, Dec 20, 2025 at 02:55:15PM +0530, Amit Kapila wrote: > As of today, I can't think of a case where next time (restart after > error) we won't generate the same origin_name but I think this will > add the dependency that each time the origin name should be generated > the same.
ReplicationOriginNameForLogicalRep() would generate the origin name as pg_suboid_relid, so we would always reuse the same origin name on restart as long as the subscription is not recreated, wouldn't we? > Also, moving just repl_origin_create part (leaving other > things like origin_advance at its location) would need some > explanation in comments which is that it has some dependency on > DropSubscription and cleanup. Anyway, if we want to go with creating > origin in a separate transaction than COPY, then we should change few > comments like: "It is possible that the origin is not yet created for > tablesync worker, this can happen for the states before > SUBREL_STATE_FINISHEDCOPY." in the code. Hmm. So... Do you think that it should be OK to just create a new transaction before we open the transaction that locks MyLogicalRepWorker->relid (one that opens a BEGIN READ ONLY on the remote side)? And I guess that we would just move the replorigin_by_name(), replorigin_create() and ERROR into this new transaction? Setting up replorigin_session_setup() and replorigin_session_origin could then be delayed until we have done the replorigin_advance() in BEGIN READ ONLY transaction? By that I mean to leave the replorigin_advance() position untouched. I have studied this code quite a bit. I "think" that something among these lines would work, but I am not 100% sure if things are OK based the relstate we store in each of these phases. If you have an argument that invalidates these lines, please feel free! -- Michael
signature.asc
Description: PGP signature
