Re: including PID or backend ID in relpath of temp rels
От | Robert Haas |
---|---|
Тема | Re: including PID or backend ID in relpath of temp rels |
Дата | |
Msg-id | AANLkTinKP6Ejo6S9L6I7ahjivXO1omdZeI4PfwZoPpUh@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: including PID or backend ID in relpath of temp rels (Alvaro Herrera <alvherre@commandprompt.com>) |
Ответы |
Re: including PID or backend ID in relpath of temp rels
(Jim Nasby <decibel@decibel.org>)
|
Список | pgsql-hackers |
On Tue, May 4, 2010 at 3:03 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: >> >> [smgr.c,inval.c] Do we need to call CacheInvalidSmgr for temporary >> >> relations? I think the only backend that can have an smgr reference >> >> to a temprel other than the owning backend is bgwriter, and AFAICS >> >> bgwriter will only have such a reference if it's responding to a >> >> request by the owning backend to unlink the associated files, in which >> >> case (I think) the owning backend will have no reference. This turns out to be wrong, I think. It seems that what we do is prevent backends other than the opening backend from reading pages from non-local temp rels into private or shared buffers, but we don't actually prevent them from having smgr references. This allows autovacuum to drop them, for example, in an anti-wraparound situation. (Thanks to Tom for helping me get my head around this better.) >> > Hmm, wasn't there a proposal to have the owning backend delete the files >> > instead of asking the bgwriter to? >> >> I did propose that upthread; it may have been proposed previously >> also. This might be worth doing independently of the rest of the patch >> (which I'm starting to fear is doomed, cue ominous soundtrack) since >> it would reduce the chance of orphaning data files and possibly >> simplify the logic also. > > +1 for doing it separately, but hopefully that doesn't mean the rest of > this patch is doomed ... Doom has been averted. Proposed patch attached. It passes regression tests and seems to work, but could use additional testing and, of course, some code-reading also. Some notes on this patch: It seems prett clear that it isn't desirable to simply add backend ID to RelFileNode, because there are too many places using RelFileNode already for purposes where the backend ID can be inferred from context, such as buffer headers and most of xlog. Instead, I introduced BackendRelFileNode, which consists of an ordinary RelFileNode augmented with a backend ID, and use that only where needed. In particular, the smgr layer must use BackendRelFileNode throughout, since it operates on both permanent and temporary relations. smgr invalidations must also include the backend ID. xlog generally happens only for non-temporary relations and can thus continue to use an ordinary RelFileNode; however, commit/abort records must use BackendRelFileNode as there may be physical storage associated with temporary relations that must be unlinked. Communication with the bgwriter must use BackendRelFileNode for similar reasons. The relcache now stores rd_backend rather than rd_islocaltemp so that it remains straightforward to call smgropen() based on a relcache entry. Some smgr functions no longer require an isTemp argument, because they can infer the necessary information from their BackendRelFileNode. smgrwrite() and smgrextend() now take a skipFsync argument rather than an isTemp argument. I'm not totally sure whether it makes sense to do what we were talking about above, viz, transfer unlink responsibility for temp rels from the bgwriter to the owning backend. I haven't done that here. Nor have I implemented any kind of improved temporary file cleanup strategy, though I hope such a thing is possible. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Вложения
В списке pgsql-hackers по дате отправления: