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:24:07 +0000 (15:24 +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 8b1ea2ae8db71895b54bdb4d1459485e0697f010..bcb99600038b6b8e507195c0c5cfd115e8a9b8f5 100644 (file)
@@ -3854,7 +3854,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)
@@ -3862,12 +3867,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",