MIME-Version: 1.0
References: 
 <CAO6_Xqo3co3BuUVEVzkaBVw9LidBgeeQ_2hfxeLMQcXwovB3GQ@mail.gmail.com>
 <CAO6_XqrZEREa5d+dyjahX6bteBhoN=8Jid-3a4f6Q35sWrv9eg@mail.gmail.com>
 <CAHGQGwHFKF+x4E+SqedMCnmLCitxjTUUtSyL_+mMeuq-GbEt6w@mail.gmail.com>
 <CAO6_Xqp+ADb6KZVWLMALu3xmwVUEO8S1EiCnp38mG6BrHrEnuA@mail.gmail.com>
 <CAO6_XqqKDV+AuP=Gf4kRKPqzyYTsOyGd3LE8Jqkwi7EMPJpbhA@mail.gmail.com>
In-Reply-To: 
 <CAO6_XqqKDV+AuP=Gf4kRKPqzyYTsOyGd3LE8Jqkwi7EMPJpbhA@mail.gmail.com>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Wed, 4 Mar 2026 02:29:27 +0900
Message-ID: 
 <CAHGQGwHc5yH4Nxp59KXJP0kAr61j3W7QeSKT2HxVjZa3OrLzmg@mail.gmail.com>
Subject: Re: Shutdown indefinitely stuck due to unflushed FPI_FOR_HINT record
To: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHGQGwHc5yH4Nxp59KXJP0kAr61j3W7QeSKT2HxVjZa3OrLzmg%40mail.gmail.com>
Precedence: bulk

On Wed, Mar 4, 2026 at 2:11=E2=80=AFAM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:
>
> Here's a small updated version of the patch, with the 2 different approac=
hes.

Thanks for the patches!


> - 0001: This is the XLogSetAsyncXactLSN call in RecordTransactionAbort
> approach. The small difference with v3 is that the 'XactLastRecEnd !=3D
> 0' condition is now merged with !isSubXact:
> +if (!isSubXact && XactLastRecEnd !=3D 0)
> +{
> +        XLogSetAsyncXactLSN(XactLastRecEnd);
>          XactLastRecEnd =3D 0;
> +}

The approach of calling XLogSetAsyncXactLSN() in RecordTransactionAbort() s=
eems
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,

--=20
Fujii Masao