Re: DISCARD ALL failing to acquire locks on pg_listen
От | Tom Lane |
---|---|
Тема | Re: DISCARD ALL failing to acquire locks on pg_listen |
Дата | |
Msg-id | 15385.1234466996@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | Re: DISCARD ALL failing to acquire locks on pg_listen (Matteo Beccati <php@beccati.com>) |
Ответы |
Re: DISCARD ALL failing to acquire locks on pg_listen
|
Список | pgsql-hackers |
Matteo Beccati <php@beccati.com> writes: > Tom Lane ha scritto: >> This seems a bit overcomplicated. I had in mind something like this... > Much easier indeed... I didn't notice the unlistenExitRegistered variable. Just for completeness, I attach another form of the patch that I thought about for a bit. This adds the ability for UNLISTEN ALL to revert the backend to the state where subsequent UNLISTENs don't cost anything. This could be of value in a scenario where you have pooled connections and just a small fraction of the client threads are using LISTEN. That seemed like kind of an unlikely use-case though. The problem is that this patch adds some cycles to transaction commit/abort for everyone, whether they ever use LISTEN/UNLISTEN/DISCARD or not. It's not a lot of cycles, but even so I'm thinking it's not a win overall. Comments? regards, tom lane Index: src/backend/access/transam/xact.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/access/transam/xact.c,v retrieving revision 1.272 diff -c -r1.272 xact.c *** src/backend/access/transam/xact.c 20 Jan 2009 18:59:37 -0000 1.272 --- src/backend/access/transam/xact.c 12 Feb 2009 18:24:12 -0000 *************** *** 1703,1708 **** --- 1703,1709 ---- AtEOXact_SPI(true); AtEOXact_xml(); AtEOXact_on_commit_actions(true); + AtEOXact_Notify(true); AtEOXact_Namespace(true); /* smgrcommit already done */ AtEOXact_Files(); *************** *** 1939,1944 **** --- 1940,1946 ---- AtEOXact_SPI(true); AtEOXact_xml(); AtEOXact_on_commit_actions(true); + AtEOXact_Notify(true); AtEOXact_Namespace(true); /* smgrcommit already done */ AtEOXact_Files(); *************** *** 2084,2089 **** --- 2086,2092 ---- AtEOXact_SPI(false); AtEOXact_xml(); AtEOXact_on_commit_actions(false); + AtEOXact_Notify(false); AtEOXact_Namespace(false); AtEOXact_Files(); AtEOXact_ComboCid(); Index: src/backend/commands/async.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/commands/async.c,v retrieving revision 1.145 diff -c -r1.145 async.c *** src/backend/commands/async.c 1 Jan 2009 17:23:37 -0000 1.145 --- src/backend/commands/async.c 12 Feb 2009 18:24:13 -0000 *************** *** 167,172 **** --- 167,178 ---- /* True if we've registered an on_shmem_exit cleanup */ static bool unlistenExitRegistered = false; + /* True if this backend has (or might have) an active LISTEN entry */ + static bool haveActiveListen = false; + + /* True if current transaction is trying to commit an UNLISTEN ALL */ + static bool committingUnlistenAll = false; + bool Trace_notify = false; *************** *** 277,282 **** --- 283,292 ---- if (Trace_notify) elog(DEBUG1, "Async_Unlisten(%s,%d)", relname, MyProcPid); + /* If we couldn't possibly be listening, no need to queue anything */ + if (pendingActions == NIL && !haveActiveListen) + return; + queue_listen(LISTEN_UNLISTEN, relname); } *************** *** 291,296 **** --- 301,310 ---- if (Trace_notify) elog(DEBUG1, "Async_UnlistenAll(%d)", MyProcPid); + /* If we couldn't possibly be listening, no need to queue anything */ + if (pendingActions == NIL && !haveActiveListen) + return; + queue_listen(LISTEN_UNLISTEN_ALL, ""); } *************** *** 493,499 **** heap_freetuple(tuple); /* ! * now that we are listening, make sure we will unlisten before dying. */ if (!unlistenExitRegistered) { --- 507,526 ---- heap_freetuple(tuple); /* ! * Remember that this backend has at least one active LISTEN. Also, ! * this LISTEN negates the effect of any earlier UNLISTEN ALL in the ! * same transaction. ! * ! * Note: it's still possible for the current transaction to fail before ! * we reach commit. In that case haveActiveListen might be uselessly ! * left true; but that's OK, if not optimal, so we don't expend extra ! * effort to cover that corner case. ! */ ! haveActiveListen = true; ! committingUnlistenAll = false; ! ! /* ! * Now that we are listening, make sure we will unlisten before dying. */ if (!unlistenExitRegistered) { *************** *** 569,574 **** --- 596,608 ---- simple_heap_delete(lRel, &lTuple->t_self); heap_endscan(scan); + + /* + * Remember that we're trying to commit UNLISTEN ALL. Since we might + * still fail before reaching commit, we can't reset haveActiveListen + * immediately. + */ + committingUnlistenAll = true; } /* *************** *** 675,680 **** --- 709,730 ---- } /* + * AtEOXact_Notify + * + * Clean up post-commit or post-abort. This is not called until + * we know that we successfully committed (or didn't). + */ + void + AtEOXact_Notify(bool isCommit) + { + /* If we committed UNLISTEN ALL, we can reset haveActiveListen */ + if (isCommit && committingUnlistenAll) + haveActiveListen = false; + /* In any case, the next xact starts with clean UNLISTEN ALL slate */ + committingUnlistenAll = false; + } + + /* * AtSubStart_Notify() --- Take care of subtransaction start. * * Push empty state for the new subtransaction. Index: src/include/commands/async.h =================================================================== RCS file: /cvsroot/pgsql/src/include/commands/async.h,v retrieving revision 1.37 diff -c -r1.37 async.h *** src/include/commands/async.h 1 Jan 2009 17:23:58 -0000 1.37 --- src/include/commands/async.h 12 Feb 2009 18:24:13 -0000 *************** *** 28,33 **** --- 28,34 ---- extern void AtSubCommit_Notify(void); extern void AtSubAbort_Notify(void); extern void AtPrepare_Notify(void); + extern void AtEOXact_Notify(bool isCommit); /* signal handler for inbound notifies (SIGUSR2) */ extern void NotifyInterruptHandler(SIGNAL_ARGS);
В списке pgsql-hackers по дате отправления: