Re: sequencesync worker race with REFRESH SEQUENCES

On Tue, 14 Jul 2026 at 08:21, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Jul 13, 2026 at 5:46 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > 2)
> > The fix will solve the purpose. But I have a doubt for the check in
> > copy_sequences(). Say a few sequences are already in the INIT state,
> > and the subscription is then repointed to a pre-PG19 publisher. Would
> > the apply worker keep launching the sequence sync worker, only for it
> > to exit each time with this error of copy_sequences? Is my
> > understanding correct? If so, is there a way to avoid launching the
> > sequence sync worker altogether when connected to a pre-PG19
> > publisher? We do have 'LogRepWorkerWalRcvConn' in the apply worker to
> > check the version.
> >
>
> Good question. I think to handle the case you pointed and even REFRESH
> SEQUENCES case, isn't it better to raise an ERROR when the user is
> trying to point connection to a version prior to 19 and the subscriber
> has sequences? For example, we do something similar retain_dead_tuples
> parameter in AlterSubscription, see handling of
> ALTER_SUBSCRIPTION_SERVER/ALTER_SUBSCRIPTION_CONNECTION->check_pub_rdt
> in AlterSubscription().

This is a better approach, the updated patch has the changes for the same.

> One additional comment on 0001:
> ===========================
> *
> @@ -435,6 +435,19 @@ copy_sequences(WalReceiverConn *conn)
> StringInfoData cmd;
> MemoryContext oldctx;
>
> + /*
> + * Sequence synchronization relies on pg_get_sequence_data(), which is
> + * only available since PostgreSQL 19. Fail with a clear diagnosis
> + * instead of sending a query that the publisher cannot execute (or
> + * cannot even parse), which would otherwise make this worker exit with
> + * a confusing "invalid query response" error.
> + */
> + if (walrcv_server_version(conn) < 190000)
> + ereport(ERROR,
> + errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot synchronize sequences for subscription \"%s\" because
> the publisher is running a version earlier than PostgreSQL 19",
> + MySubscription->name));
>
> I think it is better to move this check to the caller immediately
> after making a connection to the publisher. Also, the second part of
> the comment: "Fail with a clear diagnosis instead of sending a query
> that the ..." appears redundant to me as the code is explicit about
> the case.

Now that we are checking it while setting the connection itself, this
cannot happen from the sequence synchronization worker, so removed
this code.

The attached v3 version patch has the changes for the same.

This patch also addresses Shveta's comments from [1], Amit's comment
from [2] and [3].

[1] - https://www.postgresql.org/message-id/CAJpy0uAgM5qg5OrwL3YsiZeaq7aCyXhjSTKWHuuXoxpVcpytBw%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAA4eK1%2Ba6FyNW-LEqWAmohM3oC6h6KOrBBSeU%2B-cfopZQLXs3g%40mail.gmail.com
[3] - https://www.postgresql.org/message-id/CAA4eK1Kf9kRm%2BT8DvkgeiPt6Az7%3DTBwWJgZ%2B3UfUEz9q9CDQeA%40mail.gmail.com

Regards,
Vignesh