On Thu, Mar 9, 2017 at 6:25 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Thu, Mar 9, 2017 at 7:29 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> Suppressing ENOENT because it's not clear which backend actually owns >> a file is a bit different from using it to detect that there are no >> more segments... > > +1, emphatically.
I don't feel strongly either way on this, FWIW. >> Obviously I screwed some things up in the code I >> posted, but I think the general idea that the DSM segment owns the >> files on disk is a good one. > > +1 to that, too. The DSM has exactly the lifetime that we want here; > no backend or resowner involved in the transaction does. I actually agree with you, from a theoretical perspective. But the consequences of that theoretical point seem pretty extensive. Following through with that by pushing much more file state into shared memory has significant complexity and risk, if it's possible at all without something like "palloc(), but for shared memory". I would like to see a practical benefit. >> I figure that it (via the detach hook) >> should be able to delete the files using only data in shmem, without >> reference to any backend-local memory, so that file cleanup is >> entirely disconnected from backend-local resource cleanup (fds and >> memory). > > That's one approach. Another approach is to somehow tie the two > together; for example, I thought about teaching FileClose that where > it currently calls unlink() to get rid of the temporary file, it would > first go check some shared reference count and decrement it, skipping > the unlink if the result was >0. But that only works if you have a > separate chunk of shared memory per File, which seems like it won't > work for what you need. It won't work for what Thomas needs because that decision is made per segment. But, the decision to decrement refcount really does need to be made at the BufFile level, so Thomas is still not wrong to structure things that way. What somewhat justifies the idea of an on_dsm_detach() callback using a pointer located in shared memory to get to local memory in one backend (for error handling purposes) is the fact that it's already tacitly assumed that the local memory used for the BufFile is released after the resowner clean-up of BufFiles. Otherwise, somebody might get in big trouble if they called BufFileClose() or something in an error path. Arguably, the reliance on ordering already exists today. I'm not saying that that's a good plan, or even an acceptable trade-off. Pick your poison. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
