| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | 'Vlad Lesin' <vladlesin(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Subject: | Re: ReplicationSlotRelease() clobbers another backend's statusFlags entry |
| Date: | 2026-09-30 01:21:02 |
| Message-ID: | arxj_u6un6JGDmiP@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Sep 29, 2026 at 07:10:38AM +0000, Hayato Kuroda (Fujitsu) wrote:
> I like the part you added Assert() in StartupDecodingContext(). This can be
> worked on separately: can anyone which updates ProcGlobal->statusFlags have
> the same Assert()?
I am not sure that we need that, TBH. This feels like an unnecessary
belt-and-suspenders set of assertions.
> Regarding the code, the code comment in ReplicationSlotRelease() may be too detail.
> Can we have something like below? Or adding the possibility that auxiliary processes
> can reach here.
Proposed code and tests have been AI-generated, hence the prose.
Let's simplify that.
> /* avoid unnecessary dirtying shared cache lines */
>
> Regarding the test, I only used to reproduce the issue but not reviewed well,
> because not sure it's aimed to be included. It may need more polish, i.e.,
> advance_wal() has already been defined.
Regarding this part, I am unconvinced that this is worth the cycles
spent on. I am OK to be proved wrong, but for one the test assumes
that we could crash, which is an anti-pattern with the fix in place
because we don't crash once the status flags are not correctly
filtered.
Saying all that, only doing v2-0001 for the slot release seems good
enough here, down to v14. I'd suspect that for some code out there
clearing the flags where we should not is a trap in disguise..
Will process. :)
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-09-30 01:31:27 | Re: Credits For v19 |
| Previous Message | Michael Paquier | 2026-09-30 01:02:41 | Re: [PATCH] Clear FatalError earlier during crash restart |