From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
Subject: | Re: Synchronizing slots from primary to standby |
Date: | 2024-02-16 10:40:35 |
Message-ID: | CAJpy0uBA4efiWHhS9_FOmXxvdERk0bbMk0qMjo8ZpSTWE4DjVA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Feb 15, 2024 at 10:48 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Looking at v88_0001, random comments:
Thanks for the feedback.
>
> 1 ===
>
> Commit message "Be enabling slot synchronization"
>
> Typo? s:Be/By
Modified.
> 2 ===
>
> + It enables a physical standby to synchronize logical failover slots
> + from the primary server so that logical subscribers are not blocked
> + after failover.
>
> Not sure "not blocked" is the right wording.
> "can be resumed from the new primary" maybe? (was discussed in [1])
Modified.
> 3 ===
>
> +#define SlotSyncWorkerAllowed() \
> + (sync_replication_slots && pmState == PM_HOT_STANDBY && \
> + SlotSyncWorkerCanRestart())
>
> Maybe add a comment above the macro explaining the logic?
Done.
> 4 ===
>
> +#include "replication/walreceiver.h"
> #include "replication/slotsync.h"
>
> should be reverse order?
Removed walreceiver.h inclusion as it was not needed.
> 5 ===
>
> + if (SlotSyncWorker->syncing)
> {
> - SpinLockRelease(&SlotSyncCtx->mutex);
> + SpinLockRelease(&SlotSyncWorker->mutex);
> ereport(ERROR,
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> errmsg("cannot synchronize replication slots concurrently"));
> }
>
> worth to add a test in 040_standby_failover_slots_sync.pl for it?
It will be very difficult to stabilize this test as we have to make
sure that the concurrent users (SQL function(s) and/or worker(s)) are
in that target function at the same time to hit it.
>
> 6 ===
>
> +static void
> +slotsync_reread_config(bool restart)
> +{
>
> worth to add test(s) in 040_standby_failover_slots_sync.pl for it?
Added test.
Please find v89 patch set. The other changes are:
patch001:
1) Addressed some comments by Amit and Ajin given off-list.
2) Removed redundant header inclusions from slotsync.c.
3) Corrected the value returned by validate_remote_info().
4) Restructured code around validate_remote_info.
5) Improved comments and commit msg.
patch002:
Rebased it.
thanks
Shveta
Attachment | Content-Type | Size |
---|---|---|
v89-0003-Document-the-steps-to-check-if-the-standby-is-re.patch | application/octet-stream | 7.0 KB |
v89-0002-Allow-logical-walsenders-to-wait-for-the-physica.patch | application/octet-stream | 43.5 KB |
v89-0001-Add-a-new-slotsync-worker.patch | application/octet-stream | 62.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2024-02-16 10:44:56 | Re: Add trim_trailing_whitespace to editorconfig file |
Previous Message | Pavel Luzanov | 2024-02-16 10:04:15 | Re: Things I don't like about \du's "Attributes" column |