Hi, On 2021-08-12 15:06:23 +0530, Amit Kapila wrote: > On Thu, Aug 12, 2021 at 1:52 PM Andres Freund <and...@anarazel.de> wrote: > > I'm not so sure. Why does sharedfileset have its own proc exit hook in the > > first place? ISTM that this should be dealt with using resowners, rathers > > than > > a sharedfileset specific mechanism? > >
> The underlying temporary files need to be closed at xact end but need > to survive across transactions. Why do they need to be closed at xact end? To avoid wasting memory due to too many buffered files? > These are registered with the resource owner via > PathNameOpenTemporaryFile/PathNameCreateTemporaryFile and then closed > at xact end. So, we need a way to remove the files used by the process > (apply worker in this particular case) before process exit and used > this proc_exit hook (possibly on the lines of AtProcExit_Files). What I'm wondering is why it is a good idea to have a SharedFileSet specific cleanup mechanism. One that only operates on process lifetime level, rather than something more granular. I get that the of the files here needs to be longer than a transaction, but that can easily be addressed by having a longer lived resource owner. Process lifetime may work well for the current worker.c, but even there it doesn't seem optimal. One e.g. could easily imagine that we'd want to handle connection errors or configuration changes without restarting the worker, in which case process lifetime obviously isn't a good idea anymore. I think SharedFileSetInit() needs a comment explaining that it needs to be called in a process-lifetime memory context if used without dsm segments. Because otherwise SharedFileSetDeleteOnProcExit() will access already freed memory (both for filesetlist and the SharedFileSet itself). Greetings, Andres Freund
