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
