Date: Tue, 17 Aug 2021 16:23:03 -0400
From: "alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	"x4mmm@yandex-team.ru" <x4mmm@yandex-team.ru>,
	"a.lubennikova@postgrespro.ru" <a.lubennikova@postgrespro.ru>,
	"hlinnaka@iki.fi" <hlinnaka@iki.fi>,
	"matsumura.ryo@fujitsu.com" <matsumura.ryo@fujitsu.com>,
	"masao.fujii@gmail.com" <masao.fujii@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: archive status ".ready" files may be created too early
Message-ID: <202108172023.b52p6convvvd@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <867B852E-E892-4DD7-8E29-CE74FDBBA234@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202108172023.b52p6convvvd%40alvherre.pgsql>
Precedence: bulk

The thing I still don't understand about this patch is why we call
RegisterSegmentBoundaryEntry and NotifySegmentsReadyForArchive in
XLogInsertRecord.

My model concept of this would have these routines called only in
XLogFlush / XLogWrite, which are conceptually about transferring data
from in-memory WAL buffers into the on-disk WAL store (pg_xlog files).
As I understand, XLogInsertRecord is about copying bytes from the
high-level operation (heap insert etc) into WAL buffers.  So doing the
register/notify dance in both places should be redundant and
unnecessary.


In the NotifySegmentsReadyForArchive() call at the bottom of XLogWrite,
we use flushed=InvalidXLogRecPtr.  Why is that?  Surely we can use
LogwrtResult.Flush, just like in the other callsite there?  (If we're
covering for somebody advancing FlushRecPtr concurrently, I think we
add a comment to explain that.  But why do we need to do that in the
first place?)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"The eagle never lost so much time, as
when he submitted to learn of the crow." (William Blake)