From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Richard Guo <guofenglinux(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | tender wang <tndrwang(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
Subject: | Re: BUG #18259: Assertion in ExtendBufferedRelLocal() fails after no-space-left condition |
Date: | 2024-01-04 18:56:58 |
Message-ID: | 20240104185658.mniixu3lblpdqssz@awork3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
Hi,
On 2023-12-28 14:36:46 +0800, Richard Guo wrote:
> On Wed, Dec 27, 2023 at 5:08 PM tender wang <tndrwang(at)gmail(dot)com> wrote:
>
> > Alexander Lakhin <exclusion(at)gmail(dot)com> 于2023年12月27日周三 15:00写道:
> >
> >> I wonder, if "buf_state &= BM_VALID" is a typo here, maybe it supposed to
> >> be
> >> "buf_state &= ~BM_VALID" as in ExtendBufferedRelShared()...
> >>
> >
> > Yeah, that's true. I analyze this issue again, and I think the root cause
> > is the " buf_state &= BM_VALID" .
> >
>
> Nice catch.
Indeed.
I wonder how this survived - I remember having written a script to locally
test this scenario...
Hm, I dimly remember having a hard time making my approach for this work for
temp tables - IIRC used chattr +i to makr the file immutable, which only works
for file descriptors opened after.
> I believe the intention is to clear the BM_VALID bit.
Indeed.
These paths had many bugs over the years, they are hard to hit in testing but
regularly hit in production. In a more modern world we'd have a unit test for
these scenarios, in isolation. But that's hard at the moment.
I have been wondering if it's worth and possible to write a C level test for
them, perhaps in regress.c. But right now that seems hard because of the
DropRelationBuffers() in smgrtruncate() (seems like a layering violation to
have it there, but it's probably too risky to move at this point).
But perhaps this could actually serve as a good first case for Michael's
failure injection patch? A failure injection to make
FileFallocate()/FileZero() fail should just be a few lines.
We generally have pretty much no coverage for out-of-space, partial
read/write, EINTR of file operations, because that's hard to do with the
current test infrastructure. So this might be a nice first case.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | AC4G | 2024-01-04 21:17:36 | Postgres policy exists bug |
Previous Message | Devrim Gündüz | 2024-01-04 14:46:42 | Re: BUG #18270: RPM signing GPG key URL changed 2024-01-03 |