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>
 <CAHGQGwHc5yH4Nxp59KXJP0kAr61j3W7QeSKT2HxVjZa3OrLzmg@mail.gmail.com>
 <CAO6_Xqq1h6kggb1o206rgouPS0H5jnjahzZ0We-9ggnBjB2JsA@mail.gmail.com>
In-Reply-To: 
 <CAO6_Xqq1h6kggb1o206rgouPS0H5jnjahzZ0We-9ggnBjB2JsA@mail.gmail.com>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Thu, 5 Mar 2026 02:46:50 +0900
Message-ID: 
 <CAHGQGwFJnNUOMiW9wR-2WjSKzzj0wV8p55J8bnJ6mik=z0oFPQ@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/CAHGQGwFJnNUOMiW9wR-2WjSKzzj0wV8p55J8bnJ6mik%3Dz0oFPQ%40mail.gmail.com>
Precedence: bulk

On Wed, Mar 4, 2026 at 5:52=E2=80=AFPM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:
>
> On Tue, Mar 3, 2026 at 6:29=E2=80=AFPM Fujii Masao <masao.fujii@gmail.com=
> wrote:
> > 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 separa=
tely as
> > an improvement for the next major version.
>
> Agreed, that's definitely a change that can have a large impact. I
> will open a separate thread later.
>
> > As a simpler alternative, would it make sense for walsender to call
> > XLogFlush(GetXLogInsertRecPtr()) instead of XLogBackgroundFlush() durin=
g
> > shutdown? I'm not sure why walsender currently uses XLogBackgroundFlush=
().
> > If there isn't a clear reason for that choice, directly calling XLogFlu=
sh()
> > might be the simpler solution. Thought?
>
> That sounds like a good solution. I've tried it and it fixes the
> issue. And this only changes the shutdown behaviour in the walsender.
>
> The use of XLogBackgroundFlush() has been introduced with
> c6c333436491, but there's no mention why it was specifically used. I
> guess the assumption was that a change would either be flushed with a
> commit, or tracked by async LSN through rollback, so
> XLogBackgroundFlush() would always write pending records. But this
> turns out to be false in the case of this bug.
>
> I've updated the patch with this approach.

Thanks for updating the patch!

Barring any objections, I will commit the patch and backpatch it to
all supported branches.

Regards,

--=20
Fujii Masao