On Wed, 15 Jul 2026 at 07:01, Hayato Kuroda (Fujitsu) <[email protected]> wrote: > > Dear Vignesh, > > Thanks for updating the patch. Two comments: > > 01. > ``` > + ereport(ERROR, > + > errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > + errmsg("cannot execute %s while a sequence > synchronization worker for subscription \"%s\" is still running", > + "ALTER SUBSCRIPTION ... REFRESH > SEQUENCES", > + sub->name), > + errhint("Try again after the current > synchronization completes.")); > ``` > > Maybe a translator note is needed, per other lines.
Yes, this should be fixed > 02. > ``` > @@ -2142,6 +2145,12 @@ AlterSubscription(ParseState *pstate, > AlterSubscriptionStmt *stmt, > * retain_dead_tuples. > */ > check_pub_rdt = sub->retaindeadtuples; > + > + /* > + * If the subscription already has sequences, ensure > the new > + * publisher is new enough to synchronize them. > + */ > + check_pub_seq = HasSubscriptionSequences(subid); > ``` > > HasSubscriptionSequences() can spend long time because it searches > pg_subscription_rel > catalog linerly. Do you think we can do the deferrable approach? I imagined to > check the remote version first and then check the catalog if the version is > prior > than PG 19. Or it might be more costly to check the remote version? If we do it that way, every ALTER SUBSCRIPTION CONNECTION or SERVER would connect to the publisher, even for subscriptions that don't have any sequences. At present, we only establish a connection to the publisher when it is actually required, such as when update_failover, update_two_phase, or check_pub_rdt is true. Establishing a connection to the publisher is not a lightweight operation. It involves creating a new connection, performing authentication, and incurring network round trips before we can determine whether the publication contains any sequences. In contrast, HasSubscriptionSequences() only performs a local catalog lookup on the subscriber to determine whether the subscription has any sequences, which is significantly cheaper. Since subscriptions are likely to have no sequences in few cases, always connecting to the publisher would introduce additional overhead in the common case just to discover information that we can often infer locally. The local check also avoids unnecessary dependence on publisher connectivity when no sequence-related work is needed. Additionally this ALTER SUBSCRIPTION ... CONNECTION OR SERVER are very rare operations. For these reasons, I think it's better to keep the existing HasSubscriptionSequences() check so that we only connect to the publisher when there is a possibility that sequence-related validation is actually required. > Also, I find a case that REFRESH command can work even after the sequencesync > worker. I think it's harmless because of the LockSharedObject(), but let me > share > just in case. > > 1. run ALTER SUBSCRIPTION REFRESH SEQUENCES on the first session. all states > for > the sequences can be initialized in pg_subscription_rel. > 2. the apply worker finds the needs of sequencesync workers. > 3. run ALTER SUBSCRIPTION REFRESH SEQUENCES on the second session. At that > time > the sequencesync worker does not exist, so it does not raise an ERROR. > 4. a sequencesync worker starts, but it waits for acquiring the shared object > with Access Exclusive. > 5. the second session re-initialize the sequence states, then commit. > 6. a sequencesync worker can resumes. I think this case is okay. Although the second REFRESH SEQUENCES reinitializes the sequence states before the sequence sync worker acquires the lock, the worker has not yet fetched any sequence values from the publisher at that point. Once it starts running, it will read the current sequence values from the publisher and synchronize those, rather than applying any stale values. So I don't think this interleaving can result in outdated sequence values being applied. Regards, Vignesh
