| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: sequencesync worker race with REFRESH SEQUENCES |
| Date: | 2026-07-21 04:35:54 |
| Message-ID: | CAA4eK1J1yhcfgdfPo1L=Z_KCZdkcVMvjCLfX8XA9zscY3uyheA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Jul 21, 2026 at 9:47 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Jul 20, 2026 at 12:11 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Wed, 15 Jul 2026 at 17:23, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > So, I prefer the GUC approach and we are
> > > already overriding it for session_replication_role and search_path
> > > GUC's. The one minor difference could be that for PG19, we set it only
> > > for sequencesync worker and in HEAD for all workers.
> >
> > Here is a patch to handle the same. v4-0001 patch has the changes to
> > set default_transaction_read_only guc in InitializeLogRepWorker which
> > is common to logical replication workers in master branch and v4_PG19
> > patch has the changes to set default_transaction_read_only guc in
> > SequenceSyncWorkerMain which is only for sequence sync worker.
> >
>
> Few suggestions:
>
>
> 1)
> + /*
> + * Ignore default_transaction_read_only for logical replication workers,
> + * as they need to be able to modify subscriber-side state regardless of
> + * that setting.
> + */
>
> Shall we say:
> /*
> * Ignore default_transaction_read_only for logical replication workers,
> * as they need to modify subscriber-side state even when the GUC is enabled.
> */
>
Either of these is fine but now that I have committed the patch, let's
keep the original one.
>
> 2)
>
> +##########
> +# Ensure that ALTER SUBSCRIPTION ... REFRESH SEQUENCES can still update
> +# sequence values and mark the sequence as ready even when
> +# default_transaction_read_only is enabled on the subscriber.
> +##########
>
>
> +$node_subscriber->safe_psql(
> + 'postgres', qq(
> + set default_transaction_read_only = off;
> + ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES;
> +));
>
>
> In header we say that REFERSH SEQ can still work with
> default_transaction_read_only=on, but here we temprarily set
> default_transaction_read_only=off before 'REFRESH SEQUENCES'. This can
> be confusing for new readers.
>
The difference is between using SET and ALTER SYSTEM.
>
We shall change the header comment and
> add one comment atop REFRESH.
>
> Suggestion:
>
> ##########
> # Ensure that the sequencesync worker triggered by
> # ALTER SUBSCRIPTION ... REFRESH SEQUENCES can still update sequence
> # values and mark the sequence as ready even when
> # default_transaction_read_only is enabled on the subscriber.
> ##########
>
> Atop REFRESH:
>
> # ALTER SUBSCRIPTION cannot be executed in a read-only transaction.
> # Disable default_transaction_read_only for this session before executing
> # REFRESH SEQUENCES. The sequencesync worker will still see
> # default_transaction_read_only as being enabled.
>
I am not sure if these additional comments are very helpful.
--
With Regards,
Amit Kapila.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-07-21 04:44:24 | Re: sequencesync worker race with REFRESH SEQUENCES |
| Previous Message | shveta malik | 2026-07-21 04:17:41 | Re: sequencesync worker race with REFRESH SEQUENCES |