From: Bossart, Nathan <bossartn@amazon.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: alvherre@alvh.no-ip.org <alvherre@alvh.no-ip.org>
Cc: x4mmm@yandex-team.ru <x4mmm@yandex-team.ru>
Cc: a.lubennikova@postgrespro.ru <a.lubennikova@postgrespro.ru>
Cc: hlinnaka@iki.fi <hlinnaka@iki.fi>
Cc: matsumura.ryo@fujitsu.com <matsumura.ryo@fujitsu.com>
Cc: masao.fujii@gmail.com <masao.fujii@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: archive status ".ready" files may be created too early
Date: Tue, 3 Aug 2021 21:32:18 +0000
Message-ID: <A7D02168-A45F-40EF-B948-BDFF7FC9A321@amazon.com> (raw)
In-Reply-To: <20210803.113728.1879643826551343338.horikyota.ntt@gmail.com>
References: <997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com>
<202108022141.f7ynln3h4e3a@alvherre.pgsql>
<8596F52B-8574-4F51-8933-70C3237DF1F1@amazon.com>
<20210803.113728.1879643826551343338.horikyota.ntt@gmail.com>
On 8/2/21, 7:37 PM, "Kyotaro Horiguchi" <horikyota.ntt@gmail.com> wrote:
> I'm afraid that using hash to store boundary info is too much. Isn't a
> ring buffer enough for this use? In that case it is enough to
> remember only the end LSN of the segment spanning records. It is easy
> to expand the buffer if needed.
I agree that the hash table requires a bit more memory than what is
probably necessary, but I'm not sure I agree that maintaining a custom
data structure to save a few kilobytes of memory is worth the effort.
> @@ -2602,7 +2855,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
> LogwrtResult.Flush = LogwrtResult.Write; /* end of page */
>
> if (XLogArchivingActive())
> - XLogArchiveNotifySeg(openLogSegNo);
> + SetLastNotifiedSegmentIfInvalid(openLogSegNo - 1);
>
> Is it safe? If server didn't notified of WAL files for recent 3
> finished segments in the previous server life, they need to be
> archived this life time. But this omits maybe all of the tree.
> (I didn't confirm that behavior..)
I tested this scenario out earlier [0]. It looks like the call to
XLogArchiveCheckDone() in RemoveOldXlogFiles() will take care of
creating any .ready files we missed.
>> I believe my worry was that we'd miss notifying a segment as soon as
>> possible if the record was somehow flushed prior to registering the
>> record boundary in the map. If that's actually impossible, then I
>> would agree that the extra call to NotifySegmentsReadyForArchive() is
>> unnecessary.
>
> I don't think that XLogWrite(up to LSN=X) can happen before
> XLogInsert(endpos = X) ends.
Is there anything preventing that from happening? At the location
where we are registering the record boundary, we've already called
CopyXLogRecordToWAL(), and neither the WAL insertion lock nor the
WALWriteLock are held. Even if we register the boundary before
updating the shared LogwrtRqst.Write, there's a chance that someone
else has already moved it ahead and called XLogWrite(). I think the
worst case scenario is that we hold off creating .ready files longer
than necessary, but IMO that's still a worthwhile thing to do.
Nathan
[0] https://postgr.es/m/DA71434B-7340-4984-9B91-F085BC47A778%40amazon.com
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, horikyota.ntt@gmail.com, alvherre@alvh.no-ip.org, x4mmm@yandex-team.ru, a.lubennikova@postgrespro.ru, hlinnaka@iki.fi, matsumura.ryo@fujitsu.com, masao.fujii@gmail.com
Subject: Re: archive status ".ready" files may be created too early
In-Reply-To: <A7D02168-A45F-40EF-B948-BDFF7FC9A321@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