public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bertrand Drouvot <[email protected]>
To: shveta malik <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: Re: Synchronizing slots from primary to standby
Date: Fri, 19 Jan 2024 08:12:33 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJpy0uD+ODUF+D0GX38F_UQYTqA8wvTEUPOCBa-9guLLRMhFWQ@mail.gmail.com>
References: <CAA4eK1+q8R=yEpTMiOZ_Xksdd_Jg3SXvBMxRutHQtkCbdeK_yQ@mail.gmail.com>
	<CAJpy0uAU+nzufQwSzyB2YmhPHHn71_=1zkauP4Ra32a_eqbcig@mail.gmail.com>
	<CAA4eK1KtwFg6Mr1MRnHUUfPotzc2=5UCTq_D2t8x0PM1Lyhk8g@mail.gmail.com>
	<CAD21AoA03f0QKYjiSymGcnROvh1X-3YSqyB-19mkRXt5dfaxFw@mail.gmail.com>
	<CAJpy0uCneiKeNuz3B91Wf2LRxAenD3fpaxYfdL09y9qHmvVp+w@mail.gmail.com>
	<CAJpy0uCJ7MKgZnKLAACd-AXN0VbWM5gVJ+GRJ1Za_A2UmF3R0A@mail.gmail.com>
	<[email protected]>
	<CAJpy0uBrBc+YNxCGwCjXR+9UJZqM3QvjNq86cBy4K7kp_EXxhQ@mail.gmail.com>
	<CAA4eK1JFhbBf-LywUU0YJUTcKTFjZvbXi-42DwCd-xojWKhRXA@mail.gmail.com>
	<CAJpy0uD+ODUF+D0GX38F_UQYTqA8wvTEUPOCBa-9guLLRMhFWQ@mail.gmail.com>

Hi,

On Fri, Jan 19, 2024 at 11:46:51AM +0530, shveta malik wrote:
> On Fri, Jan 19, 2024 at 11:23 AM Amit Kapila <[email protected]> wrote:
> >
> > > > 5 === (coming from v62-0002)
> > > > +       Assert(tuplestore_tuple_count(res->tuplestore) == 1);
> > > >
> > > > Is it even possible for the related query to not return only one row? (I think the
> > > > "count" ensures it).
> > >
> > > I think you are right. This assertion was added sometime back on the
> > > basis of feedback on hackers. Let me review that again. I can consider
> > > this comment in the next version.
> > >
> >
> > OTOH, can't we keep the assert as it is but remove "= 1" from
> > "count(*) = 1" in the query. There shouldn't be more than one slot
> > with same name on the primary. Or, am I missing something?
> 
> There will be 1 record max and 0 record if the primary_slot_name is
> invalid. 

I think we'd have exactly one record in all the cases (due to the count):

postgres=# SELECT pg_is_in_recovery(), count(*) FROM pg_replication_slots WHERE 1 = 2;
 pg_is_in_recovery | count
-------------------+-------
 f                 |     0
(1 row)

postgres=# SELECT pg_is_in_recovery(), count(*) FROM pg_replication_slots WHERE 1 = 1;
 pg_is_in_recovery | count
-------------------+-------
 f                 |     1
(1 row)

> Keeping 'count(*)=1' gives the benefit that it will straight
> away give us true/false indicating if we are good or not wrt
> primary_slot_name. I feel Assert can be removed and we can simply
> have:
> 
>         if (!tuplestore_gettupleslot(res->tuplestore, true, false, tupslot))
>                 elog(ERROR, "failed to fetch primary_slot_name tuple");
> 

I'd also vote for keeping it as it is and remove the Assert.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com





view thread (119+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Synchronizing slots from primary to standby
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox