On Wed, Mar 4, 2026 at 2:11 AM Anthonin Bonnefoy
<[email protected]> wrote:
>
> Here's a small updated version of the patch, with the 2 different approaches.
Thanks for the patches!
> - 0001: This is the XLogSetAsyncXactLSN call in RecordTransactionAbort
> approach. The small difference with v3 is that the 'XactLastRecEnd !=
> 0' condition is now merged with !isSubXact:
> +if (!isSubXact && XactLastRecEnd != 0)
> +{
> + XLogSetAsyncXactLSN(XactLastRecEnd);
> XactLastRecEnd = 0;
> +}
The approach of calling XLogSetAsyncXactLSN() in RecordTransactionAbort() seems
more like an improvement than a bug fix. Since it changes
RecordTransactionAbort(), it could have unintended impact on the system.
It may be a reasonable idea (though I'm not certain yet), but for a bug fix
I believe we should first apply the minimal change necessary to resolve
the issue. If needed, this approach could then be proposed later separately as
an improvement for the next major version.
> - 0002: This is the ShutdownXLOG approach. I've used
> XLogFlush(WriteRqstPtr) instead of updating the async LSN. It feels
> like if we're going to stop the walsenders, we may as well flush
> everything and get the WAL in a good state.
> The spinlock to access XLogCtl->LogwrtRqst.Write is probably
> unnecessary since we're at a point where no additional WAL records
> should be written, but it doesn't hurt to keep consistency.
As a simpler alternative, would it make sense for walsender to call
XLogFlush(GetXLogInsertRecPtr()) instead of XLogBackgroundFlush() during
shutdown? I'm not sure why walsender currently uses XLogBackgroundFlush().
If there isn't a clear reason for that choice, directly calling XLogFlush()
might be the simpler solution. Thought?
Regards,
--
Fujii Masao
