On Fri, Apr 22, 2022 at 3:38 AM Thomas Munro <thomas.mu...@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
