Re: USE_BARRIER_SMGRRELEASE on Linux? - Mailing list pgsql-hackers

Hi Nathan,

On Wed, Feb 16, 2022 at 12:58 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:
> I've noticed that my WAL pre-allocation patches [0] routinely fail with
> "tablespace is not empty" errors on Linux [1]:
>
>          DROP TABLESPACE regress_tblspace_renamed;
>         +ERROR:  tablespace "regress_tblspace_renamed" is not empty
>
> This appears to have been discussed elsewhere (and fixed) for Windows [2]
> [3] [4].  However, I'm able to reliably reproduce the issue on Linux when
> the WAL pre-allocation patches are applied.  Adding a short sleep before
> dropping the tablespace or defining USE_BARRIER_SMGRRELEASE seems to clear
> up the issue.
>
> My current thinking is that the performance boost from pre-allocating WAL
> is revealing this issue, but I'm not 100% certain it's not due to a bug in
> my code.  I'm continuing to investigate.

Huh.  I applied your patches and saw no failure on my local Linux
hacking station.  I noticed that it failed a couple of times recently
on Linux CI as you mentioned:

https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/37/3158

But it didn't fail before (sorry, cfbot branch names needlessly
include the commitfest number, which is stupid and breaks the history;
I'll fix that one day):

https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/36/3158

It also hasn't failed on those other Unices (FreeBSD, macOS).

I pushed a hack to log the name of the file that's getting in the way:

2022-02-16 01:10:54.180 UTC [22286][client backend]
[pg_regress/tablespace][3/682:918] LOG:  XXXX found 16748
2022-02-16 01:10:54.180 UTC [22286][client backend]
[pg_regress/tablespace][3/682:918] STATEMENT:  DROP TABLESPACE
regress_tblspace_renamed;
2022-02-16 01:10:54.187 UTC [21880][checkpointer] LOG:  checkpoint
starting: immediate force wait
2022-02-16 01:10:54.188 UTC [21880][checkpointer] LOG:  checkpoint
complete: wrote 37 buffers (0.2%); 0 WAL file(s) added, 0 removed, 0
recycled; write=0.001 s, sync=0.001 s, total=0.002 s; sync files=0,
longest=0.000 s, average=0.000 s; distance=274 kB, estimate=7214 kB
2022-02-16 01:10:54.189 UTC [22286][client backend]
[pg_regress/tablespace][3/682:918] LOG:  XXXX found 16748
2022-02-16 01:10:54.189 UTC [22286][client backend]
[pg_regress/tablespace][3/682:918] STATEMENT:  DROP TABLESPACE
regress_tblspace_renamed;
2022-02-16 01:10:54.189 UTC [22286][client backend]
[pg_regress/tablespace][3/682:918] ERROR:  tablespace
"regress_tblspace_renamed" is not empty

So we can see that the checkpoint didn't get rid of (presumed)
tombstone file 16748.  At least in unpatched PostgreSQL, this
shouldn't be possible, if 16748 is on the pending delete list (since
CreateCheckPoint() runs SyncPostCheckpoint() before returning, and
only after that do we kick the condition variable and release the
backend), and USE_BARRIER_SMGRRELEASE shouldn't really make a
difference on a Unix, since all it does it close file descriptors, it
doesn't unlink anything.

I tried adding another log line so I could see the unlink() happening,
but with that it doesn't fail (though I'm still trying).  We may be in
heisenbug territory.

A conspiracy theorist with a siege mentality might wonder if Linux has
developed a name cache invalidation bug so we're seeing a slightly out
of date directory list (hmm, that CI image's kernel did go from
5.10.84-1 to 5.10.92-1 ~24 days ago), but I don't really understand
what your patch has to do with that...  Hrmph.