Re: Fixing order of resowner cleanup in 12, for Windows

On Thu, May 2, 2019 at 10:15 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> > A while back I posted a patch[1] to change the order of resowner
> > cleanup so that DSM handles are released last. That's useful for the
> > error cleanup path on Windows, when a SharedFileSet is cleaned up (a
> > mechanism that's used by parallel CREATE INDEX and parallel hash join,
> > for spilling files to disk under a temporary directory, with automatic
> > cleanup).
>
> I guess what I'm wondering is if there are any potential negative
> consequences, ie code that won't work if we change the order like this.
> I'm finding it hard to visualize what that would be, but then again
> this failure mode wasn't obvious either.

I have a thought about this. It seems to me that when it comes to
backend-private memory, we release it even later: aborting the
transaction does nothing, and we do it only later when we clean up the
transaction. So I wonder whether we're going to find that we actually
want to postpone reclaiming dynamic shared memory for even longer than
this change would do. But in general, I think we've already
established the principle that releasing memory needs to happen last,
because every other resource that you might be using is tracked using
data structures that are, uh, stored in memory. Therefore I suspect
that this change is going in the right direction.

To put that another way, the issue here is that the removal of the
files can't happen after the cleanup of the memory that tells us which
files to remove. If we had the corresponding problem for the
non-parallel case, it would mean that we were deleting the
transaction's memory context before we finished releasing all the
resources managed by the transaction's resowner, which would be
insane. I believe I put the call to release DSM segments where it is
on the theory that "we should release dynamic shared memory as early
as possible because freeing memory is good," completing failing to
take into account that this was not at all like what we do for
backend-private memory.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company