Commit 8b7ae5a8 authored by Tom Lane's avatar Tom Lane

Stabilize the results of pg_notification_queue_usage().

This function wasn't touched in commit 51004c71, but that turns out
to be a bad idea, because its results now include any dead space
that exists in the NOTIFY queue on account of our being lazy about
advancing the queue tail.  Notably, the isolation tests now fail
if run twice without a server restart between, because async-notify's
first test of the function will already show a positive value.
It seems likely that end users would be equally unhappy about the
result's instability.  To fix, just make the function call
asyncQueueAdvanceTail before computing its result.  That should end
in producing the same value as before, and it's hard to believe that
there's any practical use-case where pg_notification_queue_usage()
is called so often as to create a performance degradation, especially
compared to what we did before.

Out of paranoia, also mark this function parallel-restricted (it
was volatile, but parallel-safe by default, before).  Although the
code seems to work fine when run in a parallel worker, that's outside
the design scope of async.c, and it's a bit scary to have intentional
side-effects happening in a parallel worker.  There seems no plausible
use-case where it'd be important to try to parallelize this, so let's
not take any risk of introducing new bugs.

In passing, re-pgindent async.c and run reformat-dat-files on
pg_proc.dat, just because I'm a neatnik.

Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us
parent 91da65f4
......@@ -347,7 +347,7 @@ typedef struct
typedef struct ActionList
{
int nestingLevel; /* current transaction nesting depth */
List *actions; /* list of ListenAction structs */
List *actions; /* list of ListenAction structs */
struct ActionList *upper; /* details for upper transaction levels */
} ActionList;
......@@ -393,7 +393,7 @@ typedef struct NotificationList
int nestingLevel; /* current transaction nesting depth */
List *events; /* list of Notification structs */
HTAB *hashtab; /* hash of NotificationHash structs, or NULL */
struct NotificationList *upper; /* details for upper transaction levels */
struct NotificationList *upper; /* details for upper transaction levels */
} NotificationList;
#define MIN_HASHABLE_NOTIFIES 16 /* threshold to build hashtab */
......@@ -1554,6 +1554,9 @@ pg_notification_queue_usage(PG_FUNCTION_ARGS)
{
double usage;
/* Advance the queue tail so we don't report a too-large result */
asyncQueueAdvanceTail();
LWLockAcquire(AsyncQueueLock, LW_SHARED);
usage = asyncQueueUsage();
LWLockRelease(AsyncQueueLock);
......@@ -1834,9 +1837,8 @@ AtSubAbort_Notify(void)
* those are allocated in TopTransactionContext.
*
* Note that there might be no entries at all, or no entries for the
* current subtransaction level, either because none were ever created,
* or because we reentered this routine due to trouble during subxact
* abort.
* current subtransaction level, either because none were ever created, or
* because we reentered this routine due to trouble during subxact abort.
*/
while (pendingActions != NULL &&
pendingActions->nestingLevel >= my_level)
......@@ -2400,9 +2402,9 @@ ClearPendingActionsAndNotifies(void)
{
/*
* Everything's allocated in either TopTransactionContext or the context
* for the subtransaction to which it corresponds. So, there's nothing
* to do here except rest the pointers; the space will be reclaimed when
* the contexts are deleted.
* for the subtransaction to which it corresponds. So, there's nothing to
* do here except reset the pointers; the space will be reclaimed when the
* contexts are deleted.
*/
pendingActions = NULL;
pendingNotifies = NULL;
......
......@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 201911212
#define CATALOG_VERSION_NO 201911241
#endif
......@@ -7653,7 +7653,7 @@
{ oid => '3296',
descr => 'get the fraction of the asynchronous notification queue currently in use',
proname => 'pg_notification_queue_usage', provolatile => 'v',
prorettype => 'float8', proargtypes => '',
proparallel => 'r', prorettype => 'float8', proargtypes => '',
prosrc => 'pg_notification_queue_usage' },
# non-persistent series generator
......@@ -9333,14 +9333,13 @@
proargtypes => 'jsonb jsonpath jsonb bool', prosrc => 'jsonb_path_match' },
{ oid => '1177', descr => 'jsonpath exists test with timezone',
proname => 'jsonb_path_exists_tz', provolatile => 's',
prorettype => 'bool', proargtypes => 'jsonb jsonpath jsonb bool',
proname => 'jsonb_path_exists_tz', provolatile => 's', prorettype => 'bool',
proargtypes => 'jsonb jsonpath jsonb bool',
prosrc => 'jsonb_path_exists_tz' },
{ oid => '1179', descr => 'jsonpath query with timezone',
proname => 'jsonb_path_query_tz', provolatile => 's',
prorows => '1000', proretset => 't',
prorettype => 'jsonb', proargtypes => 'jsonb jsonpath jsonb bool',
prosrc => 'jsonb_path_query_tz' },
proname => 'jsonb_path_query_tz', prorows => '1000', proretset => 't',
provolatile => 's', prorettype => 'jsonb',
proargtypes => 'jsonb jsonpath jsonb bool', prosrc => 'jsonb_path_query_tz' },
{ oid => '1180', descr => 'jsonpath query wrapped into array with timezone',
proname => 'jsonb_path_query_array_tz', provolatile => 's',
prorettype => 'jsonb', proargtypes => 'jsonb jsonpath jsonb bool',
......@@ -9350,9 +9349,8 @@
prorettype => 'jsonb', proargtypes => 'jsonb jsonpath jsonb bool',
prosrc => 'jsonb_path_query_first_tz' },
{ oid => '2030', descr => 'jsonpath match with timezone',
proname => 'jsonb_path_match_tz', provolatile => 's',
prorettype => 'bool', proargtypes => 'jsonb jsonpath jsonb bool',
prosrc => 'jsonb_path_match_tz' },
proname => 'jsonb_path_match_tz', provolatile => 's', prorettype => 'bool',
proargtypes => 'jsonb jsonpath jsonb bool', prosrc => 'jsonb_path_match_tz' },
{ oid => '4010', descr => 'implementation of @? operator',
proname => 'jsonb_path_exists_opr', prorettype => 'bool',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment