Re: Synchronizing slots from primary to standby

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 <amit(dot)kapila16(at)gmail(dot)com> 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