Re: "as quickly as possible" (was: remove spurious CREATE INDEX CONCURRENTLY wait) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: "as quickly as possible" (was: remove spurious CREATE INDEX CONCURRENTLY wait)
Date
Msg-id 184042.1606170755@sss.pgh.pa.us
Whole thread Raw
In response to Re: "as quickly as possible" (was: remove spurious CREATE INDEX CONCURRENTLY wait)  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: "as quickly as possible" (was: remove spurious CREATE INDEX CONCURRENTLY wait)  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
Here's a draft patch.

            regards, tom lane

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 03c553e7ea..9cd0b7c11b 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -5344,7 +5344,7 @@ static void
 ShowTransactionState(const char *str)
 {
     /* skip work if message will definitely not be printed */
-    if (log_min_messages <= DEBUG5 || client_min_messages <= DEBUG5)
+    if (message_level_is_interesting(DEBUG5))
         ShowTransactionStateRec(str, CurrentTransactionState);
 }

@@ -5371,7 +5371,6 @@ ShowTransactionStateRec(const char *str, TransactionState s)
     if (s->parent)
         ShowTransactionStateRec(str, s->parent);

-    /* use ereport to suppress computation if msg will not be printed */
     ereport(DEBUG5,
             (errmsg_internal("%s(%d) name: %s; blockState: %s; state: %s, xid/subid/cid: %u/%u/%u%s%s",
                              str, s->nestingLevel,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 7e915bcadf..32a3099c1f 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -105,7 +105,7 @@ log_invalid_page(RelFileNode node, ForkNumber forkno, BlockNumber blkno,
      * tracing of the cause (note the elog context mechanism will tell us
      * something about the XLOG record that generated the reference).
      */
-    if (log_min_messages <= DEBUG1 || client_min_messages <= DEBUG1)
+    if (message_level_is_interesting(DEBUG1))
         report_invalid_page(DEBUG1, node, forkno, blkno, present);

     if (invalid_page_tab == NULL)
@@ -159,7 +159,7 @@ forget_invalid_pages(RelFileNode node, ForkNumber forkno, BlockNumber minblkno)
             hentry->key.forkno == forkno &&
             hentry->key.blkno >= minblkno)
         {
-            if (log_min_messages <= DEBUG2 || client_min_messages <= DEBUG2)
+            if (message_level_is_interesting(DEBUG2))
             {
                 char       *path = relpathperm(hentry->key.node, forkno);

@@ -192,7 +192,7 @@ forget_invalid_pages_db(Oid dbid)
     {
         if (hentry->key.node.dbNode == dbid)
         {
-            if (log_min_messages <= DEBUG2 || client_min_messages <= DEBUG2)
+            if (message_level_is_interesting(DEBUG2))
             {
                 char       *path = relpathperm(hentry->key.node, hentry->key.forkno);

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index b0d037600e..245c2f4fc8 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1146,15 +1146,9 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
      * If no error is to be thrown, and the msglevel is too low to be shown to
      * either client or server log, there's no need to do any of the rest of
      * the work.
-     *
-     * Note: this code doesn't know all there is to be known about elog
-     * levels, but it works for NOTICE and DEBUG2, which are the only values
-     * msglevel can currently have.  We also assume we are running in a normal
-     * operating environment.
      */
     if (behavior == DROP_CASCADE &&
-        msglevel < client_min_messages &&
-        (msglevel < log_min_messages || log_min_messages == LOG))
+        !message_level_is_interesting(msglevel))
         return;

     /*
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index babee386c4..87c3ea450e 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1215,7 +1215,7 @@ ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime)
     walrcv->lastMsgReceiptTime = lastMsgReceiptTime;
     SpinLockRelease(&walrcv->mutex);

-    if (log_min_messages <= DEBUG2)
+    if (message_level_is_interesting(DEBUG2))
     {
         char       *sendtime;
         char       *receipttime;
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 5d1b1a16be..2eb19ad293 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1900,7 +1900,7 @@ ProcessStandbyReplyMessage(void)
     replyTime = pq_getmsgint64(&reply_message);
     replyRequested = pq_getmsgbyte(&reply_message);

-    if (log_min_messages <= DEBUG2)
+    if (message_level_is_interesting(DEBUG2))
     {
         char       *replyTimeStr;

@@ -2082,7 +2082,7 @@ ProcessStandbyHSFeedbackMessage(void)
     feedbackCatalogXmin = pq_getmsgint(&reply_message, 4);
     feedbackCatalogEpoch = pq_getmsgint(&reply_message, 4);

-    if (log_min_messages <= DEBUG2)
+    if (message_level_is_interesting(DEBUG2))
     {
         char       *replyTimeStr;

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 1ba47c194b..04c2245338 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -600,6 +600,42 @@ errfinish(const char *filename, int lineno, const char *funcname)
     CHECK_FOR_INTERRUPTS();
 }

+/*
+ * message_level_is_interesting --- would ereport/elog do anything?
+ *
+ * Returns true if ereport/elog with this elevel will not be a no-op.
+ * This is useful to short-circuit any expensive preparatory work that
+ * might be needed for a logging message.  There is no point in
+ * prepending this to a bare ereport/elog call, however.
+ */
+bool
+message_level_is_interesting(int elevel)
+{
+    bool        output_to_server;
+    bool        output_to_client;
+
+    /*
+     * Keep this in sync with the decision-making in errstart().
+     */
+    if (elevel >= ERROR)
+        return true;
+
+    output_to_server = is_log_level_output(elevel, log_min_messages);
+    if (output_to_server)
+        return true;
+
+    if (whereToSendOutput == DestRemote && elevel != LOG_SERVER_ONLY &&
+        !ClientAuthInProgress)
+    {
+        output_to_client = (elevel >= client_min_messages ||
+                            elevel == INFO);
+        if (output_to_client)
+            return true;
+    }
+
+    return false;
+}
+

 /*
  * errcode --- add SQLSTATE error code to the current error
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 1e09ee0541..de5bba0a4a 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -149,6 +149,8 @@
 extern bool errstart(int elevel, const char *domain);
 extern void errfinish(const char *filename, int lineno, const char *funcname);

+extern bool message_level_is_interesting(int elevel);
+
 extern int    errcode(int sqlerrcode);

 extern int    errcode_for_file_access(void);

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Online verification of checksums
Next
From: Alvaro Herrera
Date:
Subject: Re: "as quickly as possible" (was: remove spurious CREATE INDEX CONCURRENTLY wait)