Fix 004.watchdog test crash on IBM Z hardware.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Wed, 27 May 2020 06:24:07 +0000 (15:24 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Wed, 27 May 2020 06:40:16 +0000 (15:40 +0900)
When watchdog copies primary node id on the master watchdog node, it
did not consider the case that primary node id on the shared memory
(Req_info->primary_node_id) is remaining in the initial value (-2),
which causes out of range subscript access to backend info
array. Interestingly accessing array[-2] does not crash on intel
architecture but does crash IBM Z hardware. Anyway the reason why the
value remains in -2 is that the regression test is performed in raw
mode. I think the code block handling the primary node id should only
be executed in streaming or logical replication mode.

Bug report and patch provided by gregn123, slightly modified by me.
Mantis bug report: https://www.pgpool.net/mantisbt/view.php?id=614

src/main/pgpool_main.c

index bd6f819d03b9383be32582723801beb8815a1b2a..e94ad95cef86b970efa99e1c4be0cb218eeb8a84 100644 (file)
@@ -4196,7 +4196,12 @@ sync_backend_from_watchdog(void)
                }
        }
 
-       if (Req_info->primary_node_id != backendStatus->primary_node_id)
+       /*
+        * Update primary node id info on the shared memory area if it's different
+        * from the one on master watchdog node. This should be done only in streaming
+        * or logical replication mode.
+        */
+       if (SL_MODE && Req_info->primary_node_id != backendStatus->primary_node_id)
        {
                /* Do not produce this log message if we are starting up the Pgpool-II */
                if (processState != INITIALIZING)
@@ -4204,12 +4209,18 @@ sync_backend_from_watchdog(void)
                                        (errmsg("primary node:%d on master watchdog node \"%s\" is different from local primary node:%d",
                                                        backendStatus->primary_node_id, backendStatus->nodeName, Req_info->primary_node_id)));
                /*
-                * master node returns primary_node_id = -1 when the node primary
-                * node is in  quarantine state on the master.
-                * So we will not update our primary node id when the status of current primary node
-                * is not CON_DOWN while primary_node_id sent by master watchdong node is -1
+                * master node returns primary_node_id = -1 when the primary node is
+                * in quarantine state on the master.  So we will not update our
+                * primary node id when the status of current primary node is not
+                * CON_DOWN while primary_node_id sent by master watchdong node is -1
+                *
+                * Note that Req_info->primary_node_id could be -2, which is the
+                * initial value. So we need to avoid crash by checking the value is
+                * not lower than 0. Otherwise we will get crash while looking up
+                * BACKEND_INFO array. See Mantis bug id 614 for more details.
                 */
-               if (backendStatus->primary_node_id == -1 && BACKEND_INFO(Req_info->primary_node_id).backend_status != CON_DOWN)
+               if (Req_info->primary_node_id >= 0 &&
+                       backendStatus->primary_node_id == -1 && BACKEND_INFO(Req_info->primary_node_id).backend_status != CON_DOWN)
                {
                        ereport(LOG,
                 (errmsg("primary node:%d on master watchdog node \"%s\" seems to be quarantined",