| From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Yilin Zhang <jiezhilove(at)126(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | RE: Improve pg_sync_replication_slots() to wait for primary to advance |
| Date: | 2026-01-30 05:48:06 |
| Message-ID: | TY4PR01MB1690757DB4829EB571AB4D211949FA@TY4PR01MB16907.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thursday, December 18, 2025 7:40 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Dec 17, 2025 at 3:58 PM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Here is a small patch to fix it.
> >
>
> Thanks, I've pushed the patch. BTW, looking at the code of slot_sync API code
> path, I could think of the following improvements.
>
> *
> if (remote_slot->confirmed_lsn > latestFlushPtr)
> { update_slotsync_skip_stats(SS_SKIP_WAL_NOT_FLUSHED);
>
> /*
> * Can get here only if GUC 'synchronized_standby_slots' on the
> * primary server was not configured correctly.
> */
> ereport(AmLogicalSlotSyncWorkerProcess() ? LOG : ERROR,
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>
> Can we change this ERROR to LOG even for API as now the API also retires to
> sync the slots during initial sync?
>
> * The use of the slot_persistence_pending flag in the internal APIs seems to
> be the reverse of what it should be. I mean to say that initially it should be
> true and when we actually persist the slot then we can set it to false.
>
> * We can retry to sync all the slots present in the primary at the start of API,
> not only temporary slots. If we do this then the previous point may not be
> required. Also, please mention something
> like: "It retries cyclically until all the failover slots that existed on primary at
> the start of the function call are synchronized." in the function description [1]
> as well.
Here is the patch to address these points. The patch improves the function to
retry for both slots that fail to persist and those persistent slots that have
skipped subsequent synchronizations.
Best Regards,
Hou zj
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0002-Add-a-taptest.patch | application/octet-stream | 3.4 KB |
| v1-0001-Improve-the-retry-logic-in-pg_sync_replication_sl.patch | application/octet-stream | 10.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-01-30 05:55:46 | Re: Add expressions to pg_restore_extended_stats() |
| Previous Message | Corey Huinker | 2026-01-30 05:34:09 | Re: Optional skipping of unchanged relations during ANALYZE? |