Re: archive status ".ready" files may be created too early

From: "alvherre(at)alvh(dot)no-ip(dot)org" <alvherre(at)alvh(dot)no-ip(dot)org>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "x4mmm(at)yandex-team(dot)ru" <x4mmm(at)yandex-team(dot)ru>, "a(dot)lubennikova(at)postgrespro(dot)ru" <a(dot)lubennikova(at)postgrespro(dot)ru>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "matsumura(dot)ryo(at)fujitsu(dot)com" <matsumura(dot)ryo(at)fujitsu(dot)com>, "masao(dot)fujii(at)gmail(dot)com" <masao(dot)fujii(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: archive status ".ready" files may be created too early
Date: 2021-08-17 00:09:00
Message-ID: 202108170009.urejfpnljykf@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2021-08-17 00:15:36 Re: The Free Space Map: Problems and Opportunities
Previous Message Greg Nancarrow 2021-08-17 00:07:03 Re: Re[5]: On login trigger: take three