Re: Synchronizing slots from primary to standby
shveta malik <shveta.malik@gmail.com>
On Wed, Sep 6, 2023 at 2:18 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Fri, Sep 1, 2023 at 1:59 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Hi Shveta. Here are some comments for patch v14-0002
> >
> > The patch is large, so my code review is a WIP... more later next week...
> >
>
> Thanks Peter for the feedback. I have tried to address most of these
> in v15. Please find my response inline for the ones which I have not
> addressed.
>
> >
> > 26.
> > +typedef struct SlotSyncWorker
> > +{
> > + /* Time at which this worker was launched. */
> > + TimestampTz launch_time;
> > +
> > + /* Indicates if this slot is used or free. */
> > + bool in_use;
> > +
> > + /* The slot in worker pool to which it is attached */
> > + int slot;
> > +
> > + /* Increased every time the slot is taken by new worker. */
> > + uint16 generation;
> > +
> > + /* Pointer to proc array. NULL if not running. */
> > + PGPROC *proc;
> > +
> > + /* User to use for connection (will be same as owner of subscription). */
> > + Oid userid;
> > +
> > + /* Database id to connect to. */
> > + Oid dbid;
> > +
> > + /* Count of Database ids it manages */
> > + uint32 dbcount;
> > +
> > + /* DSA for dbids */
> > + dsa_area *dbids_dsa;
> > +
> > + /* dsa_pointer for database ids it manages */
> > + dsa_pointer dbids_dp;
> > +
> > + /* Mutex to access dbids in dsa */
> > + slock_t mutex;
> > +
> > + /* Info about slot being monitored for worker's naptime purpose */
> > + SlotSyncWorkerWatchSlot monitor;
> > +} SlotSyncWorker;
> >
> > There seems an awful lot about this struct which is common with
> > 'LogicalRepWorker' struct.
> >
> > It seems a shame not to make use of the commonality instead of all the
> > cut/paste here.
> >
> > E.g. Can it be rearranged so all these common fields are shared:
> > - launch_time
> > - in_use
> > - slot
> > - generation
> > - proc
> > - userid
> > - dbid
> >
>
> Sure, I had this in mind along with previous comments where it was
> suggested to merge similar functions like
> WaitForReplicationWorkerAttach, WaitForSlotSyncWorkerAttach etc. That
> merging could only be possible if we try to merge the common part of
> these structures. This is WIP, will be addressed in the next version.
>
This has been addressed in version-17 now.
thanks
Shveta
Commits
-
Doc: Add the new section "Logical Replication Failover".
- b560a98a17ae 17.0 landed
-
Fix the review comments and a bug in the slot sync code.
- 3741f2a09d52 17.0 landed
-
Fix a test in failover slots regression test.
- 7e85d1c75f29 17.0 landed
-
Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.
- 6f3d8d5e7cc2 17.0 landed
-
Split XLogCtl->LogwrtResult into separate struct members
- c9920a9068ea 17.0 cited
-
Ensure that the sync slots reach a consistent state after promotion without losing data.
- 2ec005b4e297 17.0 landed
-
Introduce a new GUC 'standby_slot_names'.
- bf279ddd1c28 17.0 landed
-
Fix BF failure introduced by commit b3f6b14cf4.
- def0ce337068 17.0 landed
-
Fixups for commit 93db6cbda0.
- b3f6b14cf48f 17.0 landed
-
Fix BF failure in commit 93db6cbda0.
- d13ff82319cc 17.0 landed
-
Add a new slot sync worker to synchronize logical slots.
- 93db6cbda037 17.0 landed
-
Improve ERROR/LOG messages added by commits ddd5f4f54a and 7a424ece48.
- 801792e528d6 17.0 landed
-
Disable autovacuum on primary in 040_standby_failover_slots_sync test.
- b7bdade6a42f 17.0 landed
-
Fix the incorrect format specifier used in commit 7a424ece48.
- b987be39c3c7 17.0 landed
-
Change the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.
- d9e225f275b7 17.0 landed
-
Add more LOG and DEBUG messages for slot synchronization.
- 7a424ece4828 17.0 landed
-
Another try to fix BF failure introduced in commit ddd5f4f54a.
- 9bc1eee988c3 17.0 landed
-
Fix BF introduced in commit ddd5f4f54a.
- bd8fc1677b88 17.0 landed
-
Add a slot synchronization function.
- ddd5f4f54a02 17.0 landed
-
Clean-ups for 776621a5e4 and 7329240437.
- 22f7e61a6330 17.0 landed
-
Enhance libpqrcv APIs to support slot synchronization.
- dafbfed9efbe 17.0 landed
-
Add a failover option to subscriptions.
- 776621a5e479 17.0 landed
-
Allow setting failover property in the replication command.
- 73292404370c 17.0 landed
-
Allow to enable failover property for replication slots via SQL API.
- c393308b69d2 17.0 landed
-
Add macros for looping through a List without a ListCell.
- 14dd0f27d7cd 17.0 cited
-
Track conflict_reason in pg_replication_slots.
- 007693f2a3ac 17.0 cited
-
Allow upgrades to preserve the full subscription's state.
- 9a17be1e244a 17.0 cited
-
Add support for incremental backup.
- dc212340058b 17.0 cited
-
Simplify some logic in CreateReplicationSlot()
- e83aa9f92fdd 17.0 cited
-
Fix uninitialized access to InitialRunningXacts during decoding after ERROR.
- c7256e6564fa 15.5 cited
-
Flexible options for CREATE_REPLICATION_SLOT.
- 0266e98c6b86 15.0 cited
-
Empty search_path in logical replication apply worker and walsender.
- 11da97024abb 14.0 cited
-
Fix oldest xmin and LSN computation across repslots after advancing
- b48df818dcbd 14.0 cited
-
Require C99 (and thus MSCV 2013 upwards).
- d9dd406fe281 12.0 cited