>
> On Tue, Jul 7, 2020 at 12:55 PM Andres Freund <
andres@anarazel.de> wrote:
> > What are you proposing? For now we could easily enough work around this
> > by just making it a on_proc_exit() callback, but that doesn't really
> > change the fundamental issue imo.
>
> I think it would be more correct for it to be an on_proc_exit()
> callback, because before_shmem_exit() callbacks can and do perform
> actions which rely on an awful lot of the system being still in a
> working state. RemoveTempRelationsCallback() is a good example: it
> thinks it can start and end transactions and make a bunch of catalog
> changes. I don't know that any of that could use JIT, but moving the
> JIT cleanup to the on_shmem_exit() stage seems better. At that point,
> there shouldn't be anybody doing anything that relies on being able to
> perform logical changes to the database; we're just shutting down
> low-level subsystems at that point, and thus presumably not doing
> anything that could possibly need JIT.
>
I looked at what actually llvm_shutdown() does? It frees up JIT stacks, also if exists perf related resource, using LLVMOrcDisposeInstance() and LLVMOrcUnregisterPerf(), that were dynamically allocated in llvm_session_initialize through a JIT library function LLVMOrcCreateInstance() [1].
It looks like there is no problem in moving llvm_shutdown to either on_shmem_exit() or on_proc_exit().
>
> But I also agree that what pg_start_backup() was doing before v13 was
> wrong; that's why I committed
> 303640199d0436c5e7acdf50b837a027b5726594. The only reason I didn't
> back-patch it is because the consequences are so minor I didn't think
> it was worth worrying about. We could, though. I'd be somewhat
> inclined to both do that and also change LLVM to use on_proc_exit() in
> master, but I don't feel super-strongly about it.
>
Patch: v1-0001-Move-llvm_shutdown-to-on_proc_exit-list-from-befo.patch
Moved llvm_shutdown to on_proc_exit() call back list. Request to consider this change for master, if possible <=13 versions. Basic JIT use cases and regression tests are working fine with the patch.
Patches: PG11-0001-Fix-minor-problems-with-non-exclusive-backup-clea.patch and PG12-0001-Fix-minor-problems-with-non-exclusive-backup-cleanup.patch
Request to consider the commit 303640199d0436c5e7acdf50b837a027b5726594(above two patches are for this commit) to versions < 13, to fix the abort issue. Please note that the above two patches have no difference in the code, just I made it applicable on PG11.
Patch: v1-0001-Modify-cancel_before_shmem_exit-comments.patch
This patch, modifies cancel_before_shmem_exit() function comment to reflect the safe usage of before_shmem_exit_list callback mechanism and also removes the point "For simplicity, only the latest entry can be removed*********" as this gives a meaning that there is still scope for improvement in cancel_before_shmem_exit() search mechanism.
With Regards,
Bharath Rupireddy.
EnterpriseDB:
http://www.enterprisedb.com
