sequencesync worker race with REFRESH SEQUENCES

On Fri, 10 Jul 2026 at 10:22, Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Thanks for reporting this. The test case reproduces this issue. This
issue is not limited to lock contention. It can also occur if the
sequence synchronization worker is simply slow. For example, the
worker may fetch the sequence value from the publisher, after which
the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
SEQUENCES is executed. When the worker eventually updates the
subscriber, it uses the stale value that it fetched earlier,
overwriting the sequence with an out-of-date value.

How about raising a warning for the second REFRESH SEQUENCES
indicating that the sequence is already being synchronized and
skipping it? The warning could also include a hint to rerun ALTER
SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
completes. This approach avoids making ALTER SUBSCRIPTION ... REFRESH
SEQUENCES wait for the sequence synchronization worker, which could
itself be blocked waiting for a lock held by a long-running
transaction on the sequence.

Regards,
Vignesh

Dear Vignesh, Noah,

Agree the basic approach that we avoid the re-initializing the catalog.
My primitive idea is to follow what slotsync does. SlotSyncCtxStruct::syncing is
checked from the SQL function and the slotsync worker, and either of them can
continue. So we may be able to have LogicalRepCtxStruct::synching_seqs or
something to indicate the status.
But there might be other ways, needs to be investigated.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Fri, Jul 10, 2026 at 12:30 PM vignesh C <vignesh21@gmail.com> wrote:

On Fri, 10 Jul 2026 at 10:22, Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Thanks for reporting this. The test case reproduces this issue. This
issue is not limited to lock contention. It can also occur if the
sequence synchronization worker is simply slow. For example, the
worker may fetch the sequence value from the publisher, after which
the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
SEQUENCES is executed. When the worker eventually updates the
subscriber, it uses the stale value that it fetched earlier,
overwriting the sequence with an out-of-date value.

I have the same opinion here.

How about raising a warning for the second REFRESH SEQUENCES
indicating that the sequence is already being synchronized and
skipping it? The warning could also include a hint to rerun ALTER
SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
completes.

I agree. This is the simplest solution here.

~~

Agree the basic approach that we avoid the re-initializing the catalog.
My primitive idea is to follow what slotsync does. SlotSyncCtxStruct::syncing is
checked from the SQL function and the slotsync worker, and either of them can
continue. So we may be able to have LogicalRepCtxStruct::synching_seqs or
something to indicate the status.
But there might be other ways, needs to be investigated.

If this approach is accepted, I would prefer implementing it similarly
to how ALTER SUBSCRIPTION ... SET (two_phase) is handled. The command
simply fails (with an appropriate hint) if the apply worker for the
subscription is still running, by checking for the presence of the
apply worker.
IMO, a mechanism similar to SlotSync is not needed here because there
is currently no scenario in which continuous incremental
synchronization and one-time manual synchronization can run
concurrently. The worker itself is started only by the manual
synchronization operation and is not a continuously running process.

thanks
Shveta

On Fri, 10 Jul 2026 at 14:10, shveta malik <shveta.malik@gmail.com> wrote:

On Fri, Jul 10, 2026 at 12:30 PM vignesh C <vignesh21@gmail.com> wrote:

On Fri, 10 Jul 2026 at 10:22, Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Thanks for reporting this. The test case reproduces this issue. This
issue is not limited to lock contention. It can also occur if the
sequence synchronization worker is simply slow. For example, the
worker may fetch the sequence value from the publisher, after which
the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
SEQUENCES is executed. When the worker eventually updates the
subscriber, it uses the stale value that it fetched earlier,
overwriting the sequence with an out-of-date value.

I have the same opinion here.

How about raising a warning for the second REFRESH SEQUENCES
indicating that the sequence is already being synchronized and
skipping it? The warning could also include a hint to rerun ALTER
SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
completes.

I agree. This is the simplest solution here.

The attached v1-0001 patch avoids resetting the synchronization state
of sequences that are already being synchronized. Instead, it emits a
warning informing the user that the sequence is already being
synchronized and suggests rerunning ALTER SUBSCRIPTION ... REFRESH
SEQUENCES after the current synchronization completes. I'm not sure
whether a test is necessary for this change. Nevertheless, the
attached v1-0002 patch adds a regression test that uses an injection
point to deterministically reproduce the race condition and verifies
that the expected warning is emitted when ALTER SUBSCRIPTION ...
REFRESH SEQUENCES is invoked while a sequence synchronization is
already in progress.

Regards,
Vignesh

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

--
With Regards,
Amit Kapila.

On Sat, 11 Jul 2026 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

Both approaches seem reasonable to me. One downside of the WARNING
approach is that if a subscription contains many sequences and the
user immediately reruns ALTER SUBSCRIPTION ... REFRESH SEQUENCES, they
could receive a large number of warnings one for each sequence that is
already being synchronized which may be noisy and not particularly
useful.

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

Regards,
Vignesh

On Sat, Jul 11, 2026 at 10:31:06AM +0530, Amit Kapila wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

I haven't formed a preference, but I would use these principles to decide.
Assume we eventually support continuous, WAL-decoded replication of sequences,
not just snapshot replication. Which choice would make sequence replication
behave most like table replication under the analogous concurrent actions?

If it's an ERROR, I'd probably omit the doc note, since the ERROR would be
clear and unsurprising. In general, there's no need to document everything
that causes an error. Documentation is most useful when an error is
surprising or affects how the user writes the application.

If the command merely raises a WARNING, documentation becomes more important,
because warnings are easy to overlook. That is also an argument against a
WARNING: if the user must take action to avoid incorrect replicated state, it
is risky to rely on the user noticing one.

Does that help?

On Sun, Jul 12, 2026 at 1:44 AM Noah Misch <noah@leadboat.com> wrote:

On Sat, Jul 11, 2026 at 10:31:06AM +0530, Amit Kapila wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

I haven't formed a preference, but I would use these principles to decide.
Assume we eventually support continuous, WAL-decoded replication of sequences,
not just snapshot replication. Which choice would make sequence replication
behave most like table replication under the analogous concurrent actions?

The other point that we could consider is a future extension of
autosync sequences by allowing syncing at periodic intervals without
user intervention.

If it's an ERROR, I'd probably omit the doc note, since the ERROR would be
clear and unsurprising. In general, there's no need to document everything
that causes an error. Documentation is most useful when an error is
surprising or affects how the user writes the application.

If the command merely raises a WARNING, documentation becomes more important,
because warnings are easy to overlook. That is also an argument against a
WARNING: if the user must take action to avoid incorrect replicated state, it
is risky to rely on the user noticing one.

Sounds reasonable. Considering both your points, I am leaning towards
option (a) (during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we make the command return ERROR
suggesting sequence sync is already in-progress). If in future auto
sequence sync functionality is implemented, such an ERROR could still
be appropriate because the auto sync should automatically take care of
syncing. Or, we can even go in the direction that we can use this
command to somehow immediately perform an auto sequence sync cycle
instead of time-based re-sync.

Does that help?

Yes, thanks. But share your suggestion if you disagree with the above
or any other fix for the problem(s) reported by you.

--
With Regards,
Amit Kapila.

On Mon, Jul 13, 2026 at 08:53:42AM +0530, Amit Kapila wrote:

On Sun, Jul 12, 2026 at 1:44 AM Noah Misch <noah@leadboat.com> wrote:

Sounds reasonable. Considering both your points, I am leaning towards
option (a) (during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we make the command return ERROR
suggesting sequence sync is already in-progress). If in future auto
sequence sync functionality is implemented, such an ERROR could still
be appropriate because the auto sync should automatically take care of
syncing. Or, we can even go in the direction that we can use this
command to somehow immediately perform an auto sequence sync cycle
instead of time-based re-sync.

Sounds good.

' every

On Sat, Jul 11, 2026 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:

On Sat, 11 Jul 2026 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

Both approaches seem reasonable to me. One downside of the WARNING
approach is that if a subscription contains many sequences and the
user immediately reruns ALTER SUBSCRIPTION ... REFRESH SEQUENCES, they
could receive a large number of warnings one for each sequence that is
already being synchronized which may be noisy and not particularly
useful.

Right.

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

As I suggested earlier, I think this approach is preferable.

One potential issue I anticipate is the following: if sequence
synchronization repeatedly fails due to an error that the user is not
concerned about (for example, some sequences are missing on the
publisher), the worker may keep getting started and exiting every few
seconds. If the user happens to retry ALTER SUBSCRIPTION ... REFRESH
SEQUENCES while those worker retries are in progress, they may
repeatedly fail with the new error because the worker keeps getting
restarted. Thus, the user may not easily get the sequences that matter
to sync again. It's difficult to predict how likely this scenario is,
but if the overall situation itself is rare, I think the current
suggested solution should be acceptable.

Another potential solution, slightly more complex than the previous
one, but which could completely avoid such race scenarios is by
introducing a new DATASYNC state. Before starting sequence
synchronization, the worker would move the affected sequences to
DATASYNC. If ALTER SUBSCRIPTION ... REFRESH SEQUENCES is executed
concurrently, it would reset their state back to INIT. Before marking
a sequence as READY, the worker would recheck its current state; if it
finds that the state has been reset to INIT, it would simply skip
synchronizing that sequence. Since the sequence remains in the INIT
state, it will automatically be picked up during the next
synchronization cycle. This avoids applying stale sequence values
while ensuring that fresh values are fetched from the publisher during
the subsequent synchronization attempt.

thanks
Shveta

On Mon, Jul 13, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com> wrote:

On Sat, Jul 11, 2026 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

As I suggested earlier, I think this approach is preferable.

One potential issue I anticipate is the following: if sequence
synchronization repeatedly fails due to an error that the user is not
concerned about (for example, some sequences are missing on the
publisher), the worker may keep getting started and exiting every few
seconds. If the user happens to retry ALTER SUBSCRIPTION ... REFRESH
SEQUENCES while those worker retries are in progress, they may
repeatedly fail with the new error because the worker keeps getting
restarted. Thus, the user may not easily get the sequences that matter
to sync again. It's difficult to predict how likely this scenario is,
but if the overall situation itself is rare, I think the current
suggested solution should be acceptable.

Yeah, I think this is no different than other ERRORs, one can get
during apply like a constraint violation. The way to break the
repeated restart loop is that users need to figure out that by
checking LOGs or may be conflict related information if we ever detect
this as a conflict. So, I agree that even in this situation giving
ERROR on REFRESH SEQUENCES is reasonable.

Another potential solution, slightly more complex than the previous
one, but which could completely avoid such race scenarios is by
introducing a new DATASYNC state. Before starting sequence
synchronization, the worker would move the affected sequences to
DATASYNC. If ALTER SUBSCRIPTION ... REFRESH SEQUENCES is executed
concurrently, it would reset their state back to INIT. Before marking
a sequence as READY, the worker would recheck its current state; if it
finds that the state has been reset to INIT, it would simply skip
synchronizing that sequence. Since the sequence remains in the INIT
state, it will automatically be picked up during the next
synchronization cycle. This avoids applying stale sequence values
while ensuring that fresh values are fetched from the publisher during
the subsequent synchronization attempt.

Hmm, yeah, this is another version (with a bit of additional
complexity) of auto sync as discussed in my response. I think if we
later decide to go in this direction, we may want to achieve it via
some generation_number kind of concept where the command will indicate
somewhere in shared memory that one sync cycle is required.

--
With Regards,
Amit Kapila.

On Sat, Jul 11, 2026 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:

On Sat, 11 Jul 2026 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

Both approaches seem reasonable to me. One downside of the WARNING
approach is that if a subscription contains many sequences and the
user immediately reruns ALTER SUBSCRIPTION ... REFRESH SEQUENCES, they
could receive a large number of warnings one for each sequence that is
already being synchronized which may be noisy and not particularly
useful.

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

Please find my comments:

1)
+ /*
+ * Disallow a concurrent REFRESH SEQUENCES while a sequencesync worker
+ * for this subscription is still synchronizing the sequences from a
+ * previous REFRESH SEQUENCES.  Without this check, this command would
+ * reset the sequences' state back to INIT while the running worker is
+ * midway through applying the values it already fetched from the
+ * publisher, which could leave the sequences marked READY with stale
+ * data.
+ */

The wording "reset the sequences' state back to INIT" is incorrect.
There will be no issue if REFRESH resets the state back to INIT (from
READY) as those will then be picked up in the next cycle. The problem
is that there is no reset haappening. I think we can improve this
comment.

Suggestion:

/*
* Disallow a concurrent REFRESH SEQUENCES while a sequence sync worker
* for this subscription is still running. This avoids a race where the
* publisher's sequence advances after the current worker has fetched its
* value but before it marks the sequence READY. A user may then issue
* another REFRESH SEQUENCES to synchronize the updated value. Since the
* affected sequences are already in the INIT state, the running worker
* has no indication that a new synchronization has been requested. It
* would then apply the stale value it already fetched and mark the
* sequence READY, causing the new synchronization request to be lost and
* preventing the updated publisher values from being synchronized.
*/

2)
I am unsure if a testcase is really needed here as it is a very simple
fix. But I'd like to see what others think here.
If a test is needed, I can review it, currently I have skipped it.

thanks
Shveta

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

Fable 5 also wrote a lot more that neither it nor I confirmed by test case
construction. I'm attaching the report; feel free to disregard. Finding-2
about default_transaction_read_only=on looks worth fixing if true,

Agreed on Finding-2 as well. The issue is that the sequencesync
worker sets the value via SetSequence(), which calls
PreventCommandIfReadOnly("setval()") for non-temp sequences, so with
"default_transaction_read_only=on" on the subscriber the worker's
transaction is read-only and sequence sync fails and never reaches
READY. Table apply is unaffected only because
ExecSimpleRelationInsert() bypasses the executor's
ExecCheckXactReadOnly() path which is an undocumented, untested detail
rather than a stated guarantee.

For a minimal backpatch, we can force the sequencesync worker to run
read-write (e.g. set default_transaction_read_only=off for its session
at startup) so it matches table apply, plus a test that sets the GUC
on the subscriber and verifies sequences reach READY. Separately, it's
worth documenting that logical replication apply is exempt from
default_transaction_read_only — it's a per-transaction default meant
to guard user writes and never makes the node physically read-only —
and making that exemption explicit for all logical replication workers
so tables no longer rely on the bypass. What do you think?

--
With Regards,
Amit Kapila.

On Mon, 13 Jul 2026 at 11:08, shveta malik <shveta.malik@gmail.com> wrote:

On Sat, Jul 11, 2026 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:

On Sat, 11 Jul 2026 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

Both approaches seem reasonable to me. One downside of the WARNING
approach is that if a subscription contains many sequences and the
user immediately reruns ALTER SUBSCRIPTION ... REFRESH SEQUENCES, they
could receive a large number of warnings one for each sequence that is
already being synchronized which may be noisy and not particularly
useful.

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

Please find my comments:

1)
+ /*
+ * Disallow a concurrent REFRESH SEQUENCES while a sequencesync worker
+ * for this subscription is still synchronizing the sequences from a
+ * previous REFRESH SEQUENCES.  Without this check, this command would
+ * reset the sequences' state back to INIT while the running worker is
+ * midway through applying the values it already fetched from the
+ * publisher, which could leave the sequences marked READY with stale
+ * data.
+ */

The wording "reset the sequences' state back to INIT" is incorrect.
There will be no issue if REFRESH resets the state back to INIT (from
READY) as those will then be picked up in the next cycle. The problem
is that there is no reset haappening. I think we can improve this
comment.

Suggestion:

/*
* Disallow a concurrent REFRESH SEQUENCES while a sequence sync worker
* for this subscription is still running. This avoids a race where the
* publisher's sequence advances after the current worker has fetched its
* value but before it marks the sequence READY. A user may then issue
* another REFRESH SEQUENCES to synchronize the updated value. Since the
* affected sequences are already in the INIT state, the running worker
* has no indication that a new synchronization has been requested. It
* would then apply the stale value it already fetched and mark the
* sequence READY, causing the new synchronization request to be lost and
* preventing the updated publisher values from being synchronized.
*/

Modified

2)
I am unsure if a testcase is really needed here as it is a very simple
fix. But I'd like to see what others think here.
If a test is needed, I can review it, currently I have skipped it.

Even I feel a test case is not needed for this.

The attached v2 version patch has the changes for the same.
In addition, it includes a fix for Finding 2
(default_transaction_read_only) reported by Noah at [1]/messages/by-id/20260710045217.f0.noahmisch@microsoft.com, following the
approach suggested by Amit at [2]/messages/by-id/CAA4eK1K8LD243UzHgVNCm4skJZ4UCjR3vowDhKp=cWnK5oBT-Q@mail.gmail.com.
This version also addresses Findings 6 and 17 by reporting an error
when ALTER SUBSCRIPTION ... REFRESH SEQUENCES is executed against
subscriptions created prior to PostgreSQL 19, and documents this
behavior accordingly.

[1]: /messages/by-id/20260710045217.f0.noahmisch@microsoft.com
[2]: /messages/by-id/CAA4eK1K8LD243UzHgVNCm4skJZ4UCjR3vowDhKp=cWnK5oBT-Q@mail.gmail.com

Regards,
Vignesh

On Mon, Jul 13, 2026 at 4:38 PM vignesh C <vignesh21@gmail.com> wrote:

On Mon, 13 Jul 2026 at 11:08, shveta malik <shveta.malik@gmail.com> wrote:

On Sat, Jul 11, 2026 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:

On Sat, 11 Jul 2026 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Good catch. We have following ways to fix: (a) As mentioned by
Kuroda-san, during REFRESH SEQUENCES command, if we detect that the
sequencesync worker is in progress, we can either make the command
wait till the sequencesync is finished, return ERROR suggesting
sequence sync already in-progress, or first stop the sequencesync
worker and then complete the command and let the worker restart after
REFRESH command is finished; (b) raise a WARNING+HINT for sequences
that are not in ready state as proposed by Vignesh. Shall we
additionally add a Note for user to ensure seuencesync worker is not
in-progress before REFRESH SEQUENCES command?

Do you have any preference? I think WARNING+HINT should be sufficient
for users as this shouldn't be a common scenario but going the other
way is also fine.

Both approaches seem reasonable to me. One downside of the WARNING
approach is that if a subscription contains many sequences and the
user immediately reruns ALTER SUBSCRIPTION ... REFRESH SEQUENCES, they
could receive a large number of warnings one for each sequence that is
already being synchronized which may be noisy and not particularly
useful.

Here is a patch implementing approach (a), which detects whether a
sequence synchronization worker is already running for the
subscription. If a synchronization is already in progress, ALTER
SUBSCRIPTION ... REFRESH SEQUENCES reports an error and asks the user
to rerun the command after the current synchronization completes.

Please find my comments:

1)
+ /*
+ * Disallow a concurrent REFRESH SEQUENCES while a sequencesync worker
+ * for this subscription is still synchronizing the sequences from a
+ * previous REFRESH SEQUENCES.  Without this check, this command would
+ * reset the sequences' state back to INIT while the running worker is
+ * midway through applying the values it already fetched from the
+ * publisher, which could leave the sequences marked READY with stale
+ * data.
+ */

The wording "reset the sequences' state back to INIT" is incorrect.
There will be no issue if REFRESH resets the state back to INIT (from
READY) as those will then be picked up in the next cycle. The problem
is that there is no reset haappening. I think we can improve this
comment.

Suggestion:

/*
* Disallow a concurrent REFRESH SEQUENCES while a sequence sync worker
* for this subscription is still running. This avoids a race where the
* publisher's sequence advances after the current worker has fetched its
* value but before it marks the sequence READY. A user may then issue
* another REFRESH SEQUENCES to synchronize the updated value. Since the
* affected sequences are already in the INIT state, the running worker
* has no indication that a new synchronization has been requested. It
* would then apply the stale value it already fetched and mark the
* sequence READY, causing the new synchronization request to be lost and
* preventing the updated publisher values from being synchronized.
*/

Modified

2)
I am unsure if a testcase is really needed here as it is a very simple
fix. But I'd like to see what others think here.
If a test is needed, I can review it, currently I have skipped it.

Even I feel a test case is not needed for this.

The attached v2 version patch has the changes for the same.
In addition, it includes a fix for Finding 2
(default_transaction_read_only) reported by Noah at [1], following the
approach suggested by Amit at [2].
This version also addresses Findings 6 and 17 by reporting an error
when ALTER SUBSCRIPTION ... REFRESH SEQUENCES is executed against
subscriptions created prior to PostgreSQL 19, and documents this
behavior accordingly.

[1] - /messages/by-id/20260710045217.f0.noahmisch@microsoft.com
[2] - /messages/by-id/CAA4eK1K8LD243UzHgVNCm4skJZ4UCjR3vowDhKp=cWnK5oBT-Q@mail.gmail.com

Thanks. A few comments:

001:
1)
+ if (walrcv_server_version(wrconn) < 190000)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot synchronize sequences for subscription \"%s\" because
the publisher is running a version earlier than PostgreSQL 19",
+    sub->name));
+

I think the error can be independent of subscription name, we can give
a generic error, similar to:
"cannot enable retain_dead_tuples if the publisher is running a
version earlier than PostgreSQL 19"

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.

002: looks good.

003: yet to review.

thanks
Shveta

On Fri, 10 Jul 2026 at 10:22, Noah Misch <noah@leadboat.com> wrote:

A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.

Fable 5 also wrote a lot more that neither it nor I confirmed by test case
construction. I'm attaching the report; feel free to disregard. Finding-2
about default_transaction_read_only=on looks worth fixing if true, as do
finding-7 assert failure

I agree that the assertion failure reported in Finding 7 can occur
when a sequence is dropped concurrently while ALTER SUBSCRIPTION ...
REFRESH SEQUENCES is running. I've analyzed the issue and posted a
proposed fix at [1]/messages/by-id/CALDaNm2fHGLeiQKj0r6OG7N9QeayxSmpLrWYJRyt4dL_m3VRWw@mail.gmail.com, the thread where the relevant code was originally
introduced.

[1]: /messages/by-id/CALDaNm2fHGLeiQKj0r6OG7N9QeayxSmpLrWYJRyt4dL_m3VRWw@mail.gmail.com

Regards,
Vignesh

On Mon, Jul 13, 2026 at 5:46 PM shveta malik <shveta.malik@gmail.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().

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.

--
With Regards,
Amit Kapila.

On Mon, Jul 13, 2026 at 4:38 PM vignesh C <vignesh21@gmail.com> wrote:

The attached v2 version patch has the changes for the same.
In addition, it includes a fix for Finding 2
(default_transaction_read_only) reported by Noah at [1], following the
approach suggested by Amit at [2].

*
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5809,6 +5809,14 @@ InitializeLogRepWorker(void)
  */
  SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);

+ /*
+ * Ignore default_transaction_read_only for logical replication workers,
+ * as they need to be able to modify subscriber-side state (e.g. apply
+ * changes, update sequences) regardless of that setting.
+ */
+ SetConfigOption("default_transaction_read_only", "off", PGC_SUSET,
+ PGC_S_OVERRIDE);
+
  ApplyContext = AllocSetContextCreate(TopMemoryContext,
  "ApplyContext",
  ALLOCSET_DEFAULT_SIZES);

As pointed out in my previous email, initially let's make
backpatchable fix by adding this override only for sequencesync worker
in SequenceSyncWorkerMain(). We can discuss in a separate thread to
make this generic for all logical rep workers as a HEAD-only patch.

--
With Regards,
Amit Kapila.

On Tue, Jul 14, 2026 at 8:21 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Mon, Jul 13, 2026 at 5:46 PM shveta malik <shveta.malik@gmail.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().

Yes, that makes sense, as the user will not be able to change the
connection string unless they modify the subscription to remove
sequences.

thanks
Shveta