From: Bossart, Nathan <bossartn@amazon.com>
To: alvherre@alvh.no-ip.org <alvherre@alvh.no-ip.org>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: mengjuan.cmj@alibaba-inc.com <mengjuan.cmj@alibaba-inc.com>
Cc: Jakub.Wartak@tomtom.com <Jakub.Wartak@tomtom.com>
Subject: Re: prevent immature WAL streaming
Date: Tue, 24 Aug 2021 23:52:34 +0000
Message-ID: <8687C486-BC80-4150-8519-13FE395485BF@amazon.com> (raw)
In-Reply-To: <202108242301.clv7fhgmvjq7@alvherre.pgsql>
References: <68B0D14A-0304-448A-884E-51D0CD286816@amazon.com>
<202108242301.clv7fhgmvjq7@alvherre.pgsql>
On 8/24/21, 4:03 PM, "alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org> wrote:
> On 2021-Aug-24, Bossart, Nathan wrote:
>
>> I wonder if we need to move the call to RegisterSegmentBoundary() to
>> somewhere before WALInsertLockRelease() for this to work as intended.
>> Right now, boundary registration could take place after the flush
>> pointer has been advanced, which means GetSafeFlushRecPtr() could
>> still return an unsafe position.
>
> Yeah, you're right -- that's a definite risk. I didn't try to reproduce
> a problem with that, but it is seems pretty obvious that it can happen.
>
> I didn't have a lot of luck with a reliable reproducer script. I was
> able to reproduce the problem starting with Ryo Matsumura's script and
> attaching a replica; most of the time the replica would recover by
> restarting from a streaming position earlier than where the problem
> occurred; but a few times it would just get stuck with a WAL segment
> containing a bogus record. Then, after patch, the problem no longer
> occurs.
If moving RegisterSegmentBoundary() is sufficient to prevent the flush
pointer from advancing before we register the boundary, I bet we could
also remove the WAL writer nudge.
Another interesting thing I see is that the boundary stored in
earliestSegBoundary is not necessarily the earliest one. It's just
the first one that has been registered. I did this for simplicity for
the .ready file fix, but I can see it causing problems here. I think
we can move earliestSegBoundary backwards as long as it is greater
than lastNotifiedSeg + 1. However, it's still not necessarily the
earliest one if we copied latestSegBoundary to earliestSegBoundary in
NotifySegmentsReadyForArchive(). To handle this, we could track
several boundaries in an array, but then we'd have to hold the safe
LSN back whenever the array overflowed and we started forgetting
boundaries.
Perhaps there's a simpler solution. I'll keep thinking...
Nathan
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-hackers@postgresql.org
Cc: bossartn@amazon.com, alvherre@alvh.no-ip.org, horikyota.ntt@gmail.com, pgsql-hackers@lists.postgresql.org, mengjuan.cmj@alibaba-inc.com, Jakub.Wartak@tomtom.com
Subject: Re: prevent immature WAL streaming
In-Reply-To: <8687C486-BC80-4150-8519-13FE395485BF@amazon.com>
* 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