Horton

Re: Synchronizing slots from primary to standby

shveta malik <shveta.malik@gmail.com>

View thread View on postgr.es
From: shveta malik <shveta.malik@gmail.com>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Bruce Momjian <bruce@momjian.us>, Ashutosh Sharma <ashu.coek88@gmail.com>, Andres Freund <andres@anarazel.de>, pgsql-hackers <pgsql-hackers@postgresql.org>, Ajin Cherian <itsajin@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, shveta malik <shveta.malik@gmail.com>
Date: 2023-10-12T03:48:51Z
Lists: pgsql-hackers

Attachments

On Mon, Oct 9, 2023 at 10:51 AM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On 10/6/23 6:48 PM, Amit Kapila wrote:
> > On Wed, Oct 4, 2023 at 5:34 PM Drouvot, Bertrand
> > <bertranddrouvot.pg@gmail.com> wrote:
> >>
> >> On 10/4/23 1:50 PM, shveta malik wrote:
> >>> On Wed, Oct 4, 2023 at 5:00 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>>>
> >>>> On Wed, Oct 4, 2023 at 11:55 AM Drouvot, Bertrand
> >>>> <bertranddrouvot.pg@gmail.com> wrote:
> >>>>>
> >>>>> On 10/4/23 6:26 AM, shveta malik wrote:
> >>>>>> On Wed, Oct 4, 2023 at 5:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> How about an alternate scheme where we define sync_slot_names on
> >>>>>>> standby but then store the physical_slot_name in the corresponding
> >>>>>>> logical slot (ReplicationSlotPersistentData) to be synced? So, the
> >>>>>>> standby will send the list of 'sync_slot_names' and the primary will
> >>>>>>> add the physical standby's slot_name in each of the corresponding
> >>>>>>> sync_slot. Now, if we do this then even after restart, we should be
> >>>>>>> able to know for which physical slot each logical slot needs to wait.
> >>>>>>> We can even provide an SQL API to reset the value of
> >>>>>>> standby_slot_names in logical slots as a way to unblock decoding in
> >>>>>>> case of emergency (for example, corresponding when physical standby
> >>>>>>> never comes up).
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Looks like a better approach to me. It solves most of the pain points like:
> >>>>>> 1) Avoids the need of multiple GUCs
> >>>>>> 2) Primary and standby need not to worry to be in sync if we maintain
> >>>>>> sync-slot-names GUC on both
> >>>>
> >>>> As per my understanding of this approach, we don't want
> >>>> 'sync-slot-names' to be set on the primary. Do you have a different
> >>>> understanding?
> >>>>
> >>>
> >>> Same understanding. We do not need it to be set on primary by user. It
> >>> will be GUC on standby and standby will convey it to primary.
> >>
> >> +1, same understanding here.
> >>
> >
> > At PGConf NYC, I had a brief discussion on this topic with Andres
> > where yet another approach to achieve this came up.
>
> Great!
>
> > Have a parameter
> > like enable_failover at the slot level (this will be persistent
> > information). Users can set it during the create/alter subscription or
> > via pg_create_logical_replication_slot(). Also, on physical standby,
> > there will be a parameter like enable_syncslot. All the physical
> > standbys that have set enable_syncslot will receive all the logical
> > slots that are marked as enable_failover. To me, whether to sync a
> > particular slot is a slot-level property, so defining it in this new
> > way seems reasonable.
>
> Yeah, as this is a slot-level property, I agree that this seems reasonable.
>
> Also that sounds more natural to me with this approach. The primary
> is really the one that "drives" which slots can be synced. I like it.
>
> One could also set enable_failover while creating a logical slot on a physical
> standby (so that cascading standbys could also have "extra slot" to sync as
> compare to "level 1" standbys).
>
> >
> > I think this will simplify the scheme a bit but still, the list of
> > physical standby's for which logical slots wait during decoding needs
> > to be maintained as we thought.
>
> Right.
>
> > But, how about with the above two
> > parameters (enable_failover and enable_syncslot), we have
> > standby_slot_names defined on the primary. That avoids the need to
> > store the list of standby_slot_names in logical slots and simplifies
> > the implementation quite a bit, right?
>
> Agree.
>
> > Now, one can think if we have a
> > parameter like 'standby_slot_names' then why do we need
> > enable_syncslot on physical standby but that will be required to
> > invoke sync worker which will pull logical slot's information?
>
> yes and enable_sync slot on the standby could also be used to "pause"
> the sync on standbys (by disabling the parameter) if one would want to
> (without the need to modify anything on the primary).
>
> > The
> > advantage of having standby_slot_names defined on primary is that we
> > can selectively wait on the subset of physical standbys where we are
> > syncing the slots.
>
> Yeah and this flexibility/filtering looks somehow mandatory to me.
>
> > I think this will be something similar to
> > 'synchronous_standby_names' in the sense that the physical standbys
> > mentioned in standby_slot_names will behave as synchronous copies with
> > respect to slots and after failover user can switch to one of these
> > physical standby and others can start following new master/publisher.
> >
> > Thoughts?
>
> I like the idea and I think that's the one that seems the more reasonable
> to me. I'd vote for this idea with:
>
> - standby_slot_names on the primary (could also be set on standbys in case of
> cascading context)
> - enable_failover at logical slot creation + API to enable/disable it at wish
> - enable_syncslot on the standbys
>

Thank You Amit and Bertrand for feedback on the new design.

PFA v23 patch set which attempts to implement the new proposed design
to handle sync candidates:
   a) The synchronize_slot_names GUC is removed.  Instead the
'enable_failover' property is added at the slot level which is
persistent. It can be set by the user using create-subscription
command. eg:   create subscription mysub connection '....' publication
mypub WITH (enable_failover = true);
   b) New GUC enable_syncslot is added on standbys to enable disable
slot-sync on standbys
   c) standby_slot_names are maintained on primary.

The patch 002 also addresses Peter's comments dated Oct 6 and Oct10.

Thank You Ajin for implementing 'create subscription' cmd changes to
support 'enable_failover' syntax.

This patch has not implemented below yet, it will be done in next version:
--Provide support to set/alter enable_failover using
alter-subscription and pg_create_logical_replication_slot
--Changes needed to support slot-synchronization on cascading standbys
--Display "enable_failover" property in pg_replication_slots. I think
it makes sense to do this.

thanks
Shveta

Commits

  1. Doc: Add the new section "Logical Replication Failover".

  2. Fix the review comments and a bug in the slot sync code.

  3. Fix a test in failover slots regression test.

  4. Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.

  5. Split XLogCtl->LogwrtResult into separate struct members

  6. Ensure that the sync slots reach a consistent state after promotion without losing data.

  7. Introduce a new GUC 'standby_slot_names'.

  8. Fix BF failure introduced by commit b3f6b14cf4.

  9. Fixups for commit 93db6cbda0.

  10. Fix BF failure in commit 93db6cbda0.

  11. Add a new slot sync worker to synchronize logical slots.

  12. Improve ERROR/LOG messages added by commits ddd5f4f54a and 7a424ece48.

  13. Disable autovacuum on primary in 040_standby_failover_slots_sync test.

  14. Fix the incorrect format specifier used in commit 7a424ece48.

  15. Change the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.

  16. Add more LOG and DEBUG messages for slot synchronization.

  17. Another try to fix BF failure introduced in commit ddd5f4f54a.

  18. Fix BF introduced in commit ddd5f4f54a.

  19. Add a slot synchronization function.

  20. Clean-ups for 776621a5e4 and 7329240437.

  21. Enhance libpqrcv APIs to support slot synchronization.

  22. Add a failover option to subscriptions.

  23. Allow setting failover property in the replication command.

  24. Allow to enable failover property for replication slots via SQL API.

  25. Add macros for looping through a List without a ListCell.

  26. Track conflict_reason in pg_replication_slots.

  27. Allow upgrades to preserve the full subscription's state.

  28. Add support for incremental backup.

  29. Simplify some logic in CreateReplicationSlot()

  30. Fix uninitialized access to InitialRunningXacts during decoding after ERROR.

  31. Flexible options for CREATE_REPLICATION_SLOT.

  32. Empty search_path in logical replication apply worker and walsender.

  33. Fix oldest xmin and LSN computation across repslots after advancing

  34. Require C99 (and thus MSCV 2013 upwards).