Fix long standing bug in backend flag configuration processing module.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Mon, 16 Mar 2020 02:19:15 +0000 (11:19 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Mon, 16 Mar 2020 02:19:15 +0000 (11:19 +0900)
Recent commit revealed bugs in backend flag (backend_flag0 etc.)
configuration processing module. This leads to massive build farm
failure.

1) The default value for backend flag is ALWAYS_MASTER. This should be
"" (empty string).

2) The final value of the backend flag is the last default for the
flag. This is plain wrong (see BackendFlagsAssignFunc()). The result
value should be OR'ed value of each default value since backend_flag
is a bit data.

Probably we should back port the fix to the other stable branches but
I would like to confirm that the fix does not add bugs or side
effects before doing that.

Discussion:
https://www.pgpool.net/pipermail/pgpool-hackers/2020-March/003553.html

src/config/pool_config_variables.c

index 30114da19a278c3add59186b3a1686ca18fecb58..8d02c31892fbb697a943501c0697a9f96649db16 100644 (file)
@@ -1519,7 +1519,7 @@ static struct config_string_array ConfigureNamesStringArray[] =
                        CONFIG_VAR_TYPE_STRING_ARRAY, true, 0, MAX_NUM_BACKENDS
                },
                NULL,
-               "ALWAYS_MASTER",
+               "",     /* for ALWAYS_MASTER */
                EMPTY_CONFIG_STRING,
                BackendFlagsAssignFunc, NULL, BackendFlagsShowFunc, BackendSlotEmptyCheckFunc
        },
@@ -3874,24 +3874,16 @@ static bool
 BackendFlagsAssignFunc(ConfigContext context, char *newval, int index, int elevel)
 {
 
-       unsigned short flag = 0;
+       unsigned short flag;
        int                     i,
                                n;
        bool            allow_to_failover_is_specified = false;
        bool            disallow_to_failover_is_specified = false;
        char      **flags;
 
-       flags = get_list_from_string(newval, "|", &n);
-       if (!flags || n < 0)
-       {
-               if (flags)
-                       pfree(flags);
+       flag = g_pool_config.backend_desc->backend_info[index].flag;
 
-               ereport(elevel,
-                               (errmsg("invalid configuration for key \"backend_flag%d\"", index),
-                                errdetail("unable to get backend flags")));
-               return false;
-       }
+       flags = get_list_from_string(newval, "|", &n);
 
        for (i = 0; i < n; i++)
        {
@@ -3952,7 +3944,8 @@ BackendFlagsAssignFunc(ConfigContext context, char *newval, int index, int eleve
        g_pool_config.backend_desc->backend_info[index].flag = flag;
        ereport(DEBUG1,
                        (errmsg("setting \"backend_flag%d\" flag: %04x ", index, flag)));
-       pfree(flags);
+       if (flags)
+               pfree(flags);
        return true;
 }