On Fri, Apr 22, 2022 at 3:38 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> So, to summarise the new patch that I'm attaching to this email as 0001:
This all makes sense to me, and I didn't see anything obviously wrong
looking through the patch, either.
> However it seems that I have something wrong, because CI is failing on
> Windows; I ran out of time for looking into that today, but wanted to
> post what I have so far since I know we have an open item or two to
> close here ASAP...
:-(
> Patches 0002-0004 are Andres's, with minor tweaks:
>
> v3-0002-Fix-old-fd-issues-using-global-barriers-everywher.patch:
>
> I'm still slightly confused about whether we need an *extra* global
> barrier in DROP TABLESPACE, not just if destroy_tablespace_directory()
> failed.
Andres wrote this code, but it looks correct to me. Currently, the
reason why we use USE_BARRIER_SMGRRELEASE is that we want to make sure
that we don't fail to remove a tablespace because some other backend
might have files open. However, it might also be that no other backend
has files open, and in that case we don't need to do anything, so the
current placement of the call is correct. With this patch, though, we
want to make sure that no FD that is open before we start dropping
files remains open after we drop files - and that means we need to
force files to be closed before we even try to remove files the first
time. It seems to me that Andres's patch (your 0002) doesn't add a
second call - it moves the existing one earlier. And that seems right:
no new files should be getting opened once we force them closed the
first time, I hope anyway.
--
Robert Haas
EDB: http://www.enterprisedb.com