!1285 修复gs_guc reload时并发检查synchronous_standby_names导致的core问题

Merge pull request !1285 from chenxiaobin/check_sync

src/common/backend/utils/misc/guc.cpp

@@ -163,6 +163,7 @@
 #include "utils/guc_memory.h"
 #include "utils/guc_network.h"
 #include "utils/guc_resource.h"
+#include "common/config/cm_config.h"
 
 #ifndef PG_KRB_SRVTAB
 #define PG_KRB_SRVTAB ""
@@ -506,6 +507,9 @@ static bool validate_conf_enum(struct config_generic *record, const char *name,
                           int elevel, bool freemem, void *newvalue, void **newextra);
 
 #ifndef ENABLE_MULTIPLE_NODES
+int init_gauss_cluster_config(void);
+static bool AlterSystemSetCheckSyncStandbyNames(struct config_generic *record, const char *name, const char *value,
+                                                void *newextra);
 static void CheckAndGetAlterSystemSetParam(AlterSystemStmt* altersysstmt,
     char** outer_name, char** outer_value, struct config_generic** outer_record);
 static void FinishAlterSystemSet(GucContext context);
@@ -8141,6 +8145,121 @@ static void CheckAlterSystemSetPrivilege(const char* name)
     }
 }
 
+/*
+ * Obtain cluster information from cluster_static_config.
+ */
+int init_gauss_cluster_config(void)
+{
+    char path[MAXPGPATH] = {0};
+    int err_no = 0;
+    int nRet = 0;
+    int status = 0;
+    uint32 nodeidx = 0;
+    struct stat statbuf {};
+
+    char* gausshome = gs_getenv_r("GAUSSHOME");
+    check_backend_env(gausshome);
+
+    nRet = snprintf_s(path, MAXPGPATH, MAXPGPATH - 1, "%s/bin/%s", gausshome, STATIC_CONFIG_FILE);
+    securec_check_ss_c(nRet, "\0", "\0");
+
+    if (lstat(path, &statbuf) != 0) {
+        return 1;
+    }
+
+    status = read_config_file(path, &err_no);
+
+    if (status != 0) {
+        return 1;
+    }
+
+    if (g_nodeHeader.node <= 0) {
+        free(g_node);
+        g_node = nullptr;
+        return 1;
+    }
+
+    for (nodeidx = 0; nodeidx < g_node_num; nodeidx++) {
+        if (g_node[nodeidx].node == g_nodeHeader.node) {
+            g_currentNode = &g_node[nodeidx];
+            break;
+        }
+    }
+
+    if (g_currentNode == NULL) {
+        free(g_node);
+        g_node = nullptr;
+        return 1;
+    }
+
+    if (get_dynamic_dn_role() != 0) {
+        // failed to get dynamic dn role
+        free(g_node);
+        g_node = nullptr;
+        return 1;
+    }
+
+    return 0;
+}
+
+static bool AlterSystemSetCheckSyncStandbyNames(struct config_generic *record, const char *name, const char *value,
+                                                void *newextra)
+{
+    char* newvalue = NULL;
+    bool res = false;
+    SyncRepConfigData *pconf = NULL;
+    char* data_dir = t_thrd.proc_cxt.DataDir;
+    char* p = NULL;
+
+    if (value != NULL) {
+        newvalue = guc_strdup(DEBUG5, value);
+    }
+    if (!validate_conf_option(record, name, value, PGC_S_FILE, ERROR, false, &newvalue, &newextra)) {
+        goto ret;
+    }
+
+    pconf = (SyncRepConfigData *)newextra;
+
+    if (pconf == NULL || strcmp(value, "*") == 0) {
+        res = true;
+        goto ret;
+    }
+
+    if (pconf->num_sync > pconf->nmembers) {
+        // The sync number must less or equals to the number of standby node names.
+        goto ret;
+    }
+    /* get current cluster information from cluster_staic_config */
+    if (has_static_config()) {
+        if (init_gauss_cluster_config() != 0) {
+            res = true;
+            goto ret;
+        }
+    } else {
+        res = true;
+        goto ret;
+    }
+
+    p = pconf->member_names;
+    for (int i = 1; i <= pconf->nmembers; i++) {
+        if (!CheckDataNameValue(p, data_dir)) {
+            goto ret;
+        }
+        p += strlen(p) + 1;
+    }
+
+ret:
+    if (newvalue != NULL) {
+        pfree(newvalue);
+        newvalue = NULL;
+    }
+    if (newextra != NULL) {
+        pfree(newextra);
+        newextra = NULL;
+    }
+    return res;
+}
+
 /*
  * Persist the configuration parameter value.
  *
@@ -8198,10 +8317,14 @@ static void CheckAndGetAlterSystemSetParam(AlterSystemStmt* altersysstmt,
                     "ALTER SYSTEM SET only support POSTMASTER-level, SIGHUP-level and BACKEND-level guc variable,\n"
                     "and it must be allowed to set in postgresql.conf.", name)));
 
-    if (!validate_conf_option(record, name, value, PGC_S_FILE, ERROR, true, NULL, &newextra))
+    if ((strcmp(name, "synchronous_standby_names") == 0 &&
+        !AlterSystemSetCheckSyncStandbyNames(record, name, value, newextra)) ||
+        (strcmp(name, "synchronous_standby_names") != 0 &&
+        !validate_conf_option(record, name, value, PGC_S_FILE, ERROR, true, NULL, &newextra))) {
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                  errmsg("invalid value for parameter \"%s\": \"%s\"", name, value)));
+    }
 
     *outer_name = name;
     *outer_value = value;

src/gausskernel/storage/replication/syncrep.cpp

@@ -62,7 +62,6 @@
 #include "utils/ps_status.h"
 #include "utils/distribute_test.h"
 #include "replication/dcf_replication.h"
-#include "common/config/cm_config.h"
 
 /*
  * To control whether a master configured with synchronous commit is
@@ -1742,60 +1741,6 @@ static bool SyncPaxosQueueIsOrderedByLSN(void)
 }
 #endif
 
-/*
- * Obtain cluster information from cluster_static_config.
- */
-int init_gauss_cluster_config(void)
-{
-    char path[MAXPGPATH] = {0};
-    int err_no = 0;
-    int nRet = 0;
-    int status = 0;
-    uint32 nodeidx = 0;
-    struct stat statbuf {};
-
-    char* gausshome = gs_getenv_r("GAUSSHOME");
-    check_backend_env(gausshome);
-
-    nRet = snprintf_s(path, MAXPGPATH, MAXPGPATH - 1, "%s/bin/%s", gausshome, STATIC_CONFIG_FILE);
-    securec_check_ss_c(nRet, "\0", "\0");
-
-    if (lstat(path, &statbuf) != 0) {
-        return 1;
-    }
-
-    status = read_config_file(path, &err_no);
-
-    if (0 != status) {
-        return 1;
-    }
-
-    if (g_nodeHeader.node <= 0) {
-        free(g_node);
-        return 1;
-    }
-
-    for (nodeidx = 0; nodeidx < g_node_num; nodeidx++) {
-        if (g_node[nodeidx].node == g_nodeHeader.node) {
-            g_currentNode = &g_node[nodeidx];
-            break;
-        }
-    }
-
-    if (NULL == g_currentNode) {
-        free(g_node);
-        return 1;
-    }
-
-    if (get_dynamic_dn_role() != 0) {
-        // failed to get dynamic dn role
-        free(g_node);
-        return 1;
-    }
-
-    return 0;
-}
-
 /*
  * ===========================================================
  * Synchronous Replication functions executed by any process
@@ -1809,9 +1754,6 @@ bool check_synchronous_standby_names(char **newval, void **extra, GucSource sour
         int parse_rc;
         SyncRepConfigData *pconf = NULL;
         syncrep_scanner_yyscan_t yyscanner;
-        char* data_dir = t_thrd.proc_cxt.DataDir;
-        uint32 idx;
-        char* p = NULL;
 
         /* Reset communication variables to ensure a fresh start */
         t_thrd.syncrepgram_cxt.syncrep_parse_result = NULL;
@@ -1837,38 +1779,6 @@ bool check_synchronous_standby_names(char **newval, void **extra, GucSource sour
                       t_thrd.syncrepgram_cxt.syncrep_parse_result->config_size);
         securec_check(rc, "", "");
 
-        if (strcmp(pconf->member_names, "*") == 0) {
-            goto pass;
-        }
-
-        if (pconf->num_sync > pconf->nmembers) {
-            // The sync number must less or equals to the number of standby node names.
-            return false;
-        }
-
-        /* get current cluster information from cluster_staic_config */
-        if (strcmp(u_sess->attr.attr_common.application_name, "gsql") == 0 && has_static_config()
-            && 0 == init_gauss_cluster_config()) {
-            for (idx = 0; idx < g_node_num; idx++) {
-                if (g_currentNode->node == g_node[idx].node) {
-                    g_local_node_idx = idx;
-                    break;
-                }
-            }
-        } else {
-            goto pass;
-        }
-        g_local_node_name = g_node[g_local_node_idx].nodeName;
-
-        p = pconf->member_names;
-        for (int i = 1; i <= pconf->nmembers; i++) {
-            if (!CheckDataNameValue(p, data_dir)) {
-                return false;
-            }
-            p += strlen(p) + 1;
-        }
-
-pass:
         *extra = (void *)pconf;
         if (t_thrd.syncrepgram_cxt.syncrep_parse_result) {
             pfree(t_thrd.syncrepgram_cxt.syncrep_parse_result);

src/lib/config/cm_config.cpp

@@ -294,11 +294,6 @@ int read_config_file(const char* file_path, int* err_no, bool inReload, int mall
     FREAD(&g_nodeHeader.nodeCount, 1, sizeof(uint32), fd);
     FREAD(&g_nodeHeader.node, 1, sizeof(uint32), fd);
 
-    if (g_node != NULL && !inReload) {
-        free(g_node);
-        g_node = NULL;
-    }
-
     if (g_nodeHeader.nodeCount > CM_NODE_MAXNUM || g_nodeHeader.nodeCount == 0) {
         fclose(fd);
         return READ_FILE_ERROR;
@@ -308,14 +303,16 @@ int read_config_file(const char* file_path, int* err_no, bool inReload, int mall
         goto read_failed;
 
     if (!inReload) {
-        if (mallocByNodeNum == MALLOC_BY_NODE_NUM) {
-            g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * g_nodeHeader.nodeCount);
-        } else {
-            g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * CM_NODE_MAXNUM);
-        }
         if (g_node == NULL) {
-            fclose(fd);
+            if (mallocByNodeNum == MALLOC_BY_NODE_NUM) {
-            return OUT_OF_MEMORY;
+                g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * g_nodeHeader.nodeCount);
+            } else {
+                g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * CM_NODE_MAXNUM);
+            }
+            if (g_node == NULL) {
+                fclose(fd);
+                return OUT_OF_MEMORY;
+            }
         }
 
         /* g_node size may be larger than SECUREC_STRING_MAX_LEN in large cluster.*/
@@ -816,15 +813,12 @@ int read_lc_config_file(const char* file_path, int* err_no)
         return READ_FILE_ERROR;
     }
 
-    if (g_node != NULL) {
-        free(g_node);
-        g_node = NULL;
-    }
-
-    g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * g_node_num);
     if (g_node == NULL) {
-        fclose(fd);
+        g_node = (staticNodeConfig*)malloc(sizeof(staticNodeConfig) * g_node_num);
-        return OUT_OF_MEMORY;
+        if (g_node == NULL) {
+            fclose(fd);
+            return OUT_OF_MEMORY;
+        }
     }
 
     for (ii = 0; ii < g_node_num; ii++) {