From: Michael Paquier <michael@paquier.xyz>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Callahan, Drew <callaan@amazon.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: Potential data loss due to race condition during logical replication slot creation
Date: Mon, 8 Jul 2024 08:56:42 +0900
Message-ID: <ZosrOu7RFnrrsuHL@paquier.xyz> (raw)
In-Reply-To: <CAD21AoCS7Xfswo0v_jxZqJ97ndq=59s4Z2GVY8YjQZU-t6198w@mail.gmail.com>
References: <CAA4eK1+Q+gHADNeEuC=spzCh=JXBMNSWhK1SScu6dQ1owjrBVw@mail.gmail.com>
<CAD21AoDzjFxFiRYE-Qpxe83weewegbWf7f+zK3r4pSFZwxW_rw@mail.gmail.com>
<CAA4eK1JWGXcnT2=PYW4U--j8GME4o3+SR6k+bs_r0thG9RVSpQ@mail.gmail.com>
<CAD21AoCf+PiedCR5ODquL=tnp_2+nj_jemydnyu+KwPQ8beovA@mail.gmail.com>
<CAA4eK1JbN=jTNtejjk3Jb-4Gr6mZ7PXRn6A_Boda0afB0js1oQ@mail.gmail.com>
<CAD21AoDYrJMDazvcR-k3SbNAWFiingyQbDYbPLqBp8Q3ewyZJw@mail.gmail.com>
<CAA4eK1LbpagXYw6eP+qBz2SYjQ3x26ZdVCJBkp033aExqA2MbQ@mail.gmail.com>
<CAD21AoDR3h78U0hxdzWPuPL11nvJCWYMB8h+QoOjd82ZmXjfgw@mail.gmail.com>
<CAA4eK1LEy2oUhncuu5AH1RdYgc+NEJn9NcH_8FB+D3m06hrCTg@mail.gmail.com>
<CAD21AoCS7Xfswo0v_jxZqJ97ndq=59s4Z2GVY8YjQZU-t6198w@mail.gmail.com>
On Fri, Jul 05, 2024 at 11:52:52PM +0900, Masahiko Sawada wrote:
> I've attached updated patches for HEAD and pg17 for now (I will create
> the patch for other backbranches).
>
> In the patches, I used a different approach in between HEAD and
> backbranches. On HEAD, we store a flag indicating whether or not we
> should skip snapshot restores into the SnapBuild struct and set it
> only while finding the start point. Therefore we have to bump
> SNAPBUILD_VERSION. On backbranches, I used the approach we discussed
> above; store the flag in LogicalDecodingContext and set it when
> creating the LogicalDecodingContext for a new logical slot. A possible
> downside of the approach taken for backbranches is that we implicitly
> require for users to use the same LogicalDecodingContext for both
> initializing the context for a new slot and finding its start point.
> IIUC it was not strictly required. This restriction would not be a
> problem at least in the core, but I'm not sure if there are no
> external extensions that create a logical slot in that way. This is
> the main reason why I used a different approach on HEAD and
> backbranches. Therefore, if it does not matter, I think we can use the
> same approach on all branches, which is better in terms of
> maintainability.
--- a/src/backend/replication/logical/snapbuild.c+++ b/src/backend/replication/logical/snapbuild.c@@ -189,6 +189,9 @@ struct SnapBuild
/* Indicates if we are building full snapshot or just catalog one. */
bool building_full_snapshot;
+ /* Indicates if we are finding the start point to extract changes */+ bool finding_start_point;+
FYI, I think that it is still OK to bump SNAPBUILD_VERSION on
REL_17_STABLE. That will reduce by 1 year the time window required to
maintain the tweaks implemented for the versions in the back-branches.
So I'd suggest to do what the v17 version of the patch does for ~16,
and use the snapshot format changes in 17~.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../ZosrOu7RFnrrsuHL@paquier.xyz/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, sawada.mshk@gmail.com, amit.kapila16@gmail.com, callaan@amazon.com, pgsql-bugs@lists.postgresql.org
Subject: Re: Potential data loss due to race condition during logical replication slot creation
In-Reply-To: <ZosrOu7RFnrrsuHL@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox