Stabilize the results of pg_notification_queue_usage().

This function wasn't touched in commit 51004c717, 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

diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index a3209d076b3ac6279c88758044b7c5ab0db46cd8..370c2b9a4fba33da612169eb20132465e8e3646c 100644 (file)
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -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;

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index b6b380cfc24f89fe3d5c9b737f911dbec91ca21e..3a50ba0dfe024729ece0ac674b18ef634aea046e 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201911212
+#define CATALOG_VERSION_NO     201911241
 
 #endif

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d4fa6dd6491c104eef53054c93a05422629a5b83..ac8f64b219312d188461df6d943b6b1abf67bbed 100644 (file)
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -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',