From: alvherre@alvh.no-ip.org <alvherre@alvh.no-ip.org>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
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: Mon, 16 Aug 2021 20:09:00 -0400
Message-ID: <202108170009.urejfpnljykf@alvherre.pgsql> (raw)
In-Reply-To: <867B852E-E892-4DD7-8E29-CE74FDBBA234@amazon.com>
So why do we call this structure "record boundary map", when the
boundaries it refers to are segment boundaries? I think we should call
it "segment boundary map" instead ... and also I think we should use
that name in the lock that protects it, so instead of ArchNotifyLock, it
could be SegmentBoundaryLock or perhaps WalSegmentBoundaryLock.
The reason for the latter is that I suspect the segment boundary map
will also have a use to fix the streaming replication issue I mentioned
earlier in the thread. This also makes me think that we'll want the wal
record *start* address to be in the hash table too, not just its *end*
address. So we'll use the start-1 as position to send, rather than the
end-of-segment when GetFlushRecPtr() returns that.
As for 0xFFFFFFFFFFFFFFFF, I think it would be cleaner to do a
#define MaxXLogSegNo with that value in the line immediately after
typedef XLogSegNo, rather than use the numerical value directly in the
assignment.
Typo in comment atop RemoveRecordBoundariesUpTo: it reads "up to an",
should read "up to and".
I think the API of GetLatestRecordBoundarySegment would be better by
returning the boolean and having the segment as out argument. Then you
could do the caller more cleanly,
if (GetLatestRecordBoundarySegment(last_notified, flushed, &latest_boundary_segment))
{
SetLastNotified( ... );
RemoveRecordBoundaries( ... );
LWLockRelease( ... );
for (..)
XLogArchiveNotifySeg(...);
PgArchWakeup();
}
else
LWLockRelease(...);
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"La virtud es el justo medio entre dos defectos" (Aristóteles)
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: alvherre@alvh.no-ip.org, bossartn@amazon.com, horikyota.ntt@gmail.com, 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: <202108170009.urejfpnljykf@alvherre.pgsql>
* 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