| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
| Date: | 2025-12-04 08:23:53 |
| Message-ID: | CAD21AoAtp=Oh3Nnp_8Q_S0WuK7STX2XWk7BOiu=bMfebiyT6aw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Dec 3, 2025 at 7:28 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Dec 4, 2025 at 8:03 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, Dec 1, 2025 at 12:54 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Dec 1, 2025 at 11:40 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > >
> > > >
> > > > > 11)
> > > > > my ($result, $stdout, $stderr) = $standby4->psql('postgres',
> > > > > qq[select pg_logical_slot_get_changes('standby4_slot', null, null)]);
> > > > > like(
> > > > > $stderr,
> > > > > qr/ERROR: logical decoding on standby requires "effective_wal_level"
> > > > > >= "logical" on the primary/,
> > > > > "cannot use logical decoding on standby as it is disabled on primary");
> > > > >
> > > > > Since the slot is invalidated, shouldn't the better error message be
> > > > > what we usually get:
> > > > > ERROR: can no longer access replication slot "slot"
> > > > > DETAIL: This replication slot has been invalidated due to "..".
> > > > >
> > > > > IMO, the 'invalidated' message is better because even if we enable
> > > > > logical-decoding on primary, it is of no use for this slot.
> > > > >
> > > > > But I also see the difficulty in achieving this, as the
> > > > > primary-related error message is by CheckLogicalDecodingRequirements()
> > > > > which happens earlier than
> > > > > ReplicationSlotAcquire(). So if there is no better way, we can leave it as is.
> > > >
> > > > We can achieve it by creating another valid logical slot, but I
> > > > believe that we already have tests to check if we cannot use an
> > > > invalidated slot. What we want to test here is if logical decoding is
> > > > disabled by invalidating the last logical slot. So I think the test
> > > > works as expected.
> > > >
> > >
> > > Sorry for the confusion. My earlier comment wasn’t about the test
> > > mechanics or correctness. I meant that when a slot is invalidated and
> > > someone attempts to use it, the message should reflect that (can no
> > > longer access replication slot "slot"). Currently, the message
> > > suggests enabling logical decoding on the primary, even though that
> > > won’t make an invalidated slot usable. IMO, it would be clearer to
> > > report that the slot is invalidated instead of directing the user to
> > > enable decoding on primary.
> >
> > Hmm, I'm not sure there is a big difference there. If we show the
> > message first that indicates that the slot is invalidated, users would
> > drop the slot and try to create a new logical slot, but they would get
> > the error 'logical decoding on standbys requires "effective_wal_level"
> > >= "logical" on the primary'. With the current implementation, they
> > would enable logical decoding on the primary first, realize that the
> > slot on the standby is already invalidated, and then create a new
> > logical slot.
> >
>
> Okay, makes sense. Current behaviour works for me.
>
I've attached the updated patch that incorporated the review comments
and is rebased to the current HEAD.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v32-0001-Toggle-logical-decoding-dynamically-based-on-log.patch | application/octet-stream | 104.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Antonin Houska | 2025-12-04 08:34:54 | Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements |
| Previous Message | David Geier | 2025-12-04 08:07:20 | Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc? |