Skip to content

Commit

Permalink
Enhancement: optimize guc mechanism
Browse files Browse the repository at this point in the history 
Add new guc type Undispatch that doesn't dipatch from master to primary.
for a guc, it's one of sync, unsync,undispatch.
  • Loading branch information
hanwei committed Feb 12, 2025
1 parent 0f684d1 commit 0ebcc4b
Show file tree
Hide file tree
Showing 9 changed files with 342 additions and 272 deletions.
4 changes: 2 additions & 2 deletions src/backend/utils/misc/README
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ GUC Synchronization
Due to distributed character of gpdb, each GUC needs to declare whether
it needs to sync value between master and primaries. If you want to
introduce a new GUC in guc.c or guc_gp.c, the GUC's name must be populated
into either sync_guc_name.h or unsync_guc_name.h. If not, gpdb will raise an
ERROR in run-time.
into {sync_guc_name.h, unsync_guc_name.h, undispatch_guc_name.h}. If not,
gpdb will raise an ERROR in run-time.
For custom GUC, if it has synchronization requirement, add GUC_GPDB_NEED_SYNC
bit into the GUC flag. Otherwise, system will default add GUC_GPDB_NO_SYNC
flag bit for it as it can not synchronize in cluster.
Expand Down
9 changes: 7 additions & 2 deletions src/backend/utils/misc/guc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1006,8 +1006,8 @@ static const unit_conversion time_unit_conversion_table[] =
* variable_is_guc_list_quote() in src/bin/pg_dump/dumputils.c.
*
* 8. In gpdb, the guc is force explicit declare whether it needs to sync value
* between master and primary. Add guc name into either sync_guc_names_array
* or unsync_guc_names_array.
* between master and primary. Add guc name of {sync_guc_names_array,
* unsync_guc_names_array,undispatch_guc_names_array}.
*/


Expand Down Expand Up @@ -9227,10 +9227,15 @@ DispatchSetPGVariable(const char *name, List *args, bool is_local)
{
ListCell *l;
StringInfoData buffer;
struct config_generic *record;

if (Gp_role != GP_ROLE_DISPATCH || IsBootstrapProcessingMode())
return;

record = find_option(name, false, false, 0);
if (record == NULL || record->flags & GUC_GPDB_NO_DISPATCH)
return;

/*
* client_encoding is always kept at SQL_ASCII in QE processes. (See also
* cdbconn_doConnectStart().)
Expand Down
94 changes: 56 additions & 38 deletions src/backend/utils/misc/guc_gp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5145,9 +5145,9 @@ struct config_enum ConfigureNamesEnum_gp[] =
};

/*
* For system defined GUC must assign a tag either GUC_GPDB_NEED_SYNC
* or GUC_GPDB_NO_SYNC. We deprecated direct define in guc.c, instead,
* add into sync_guc_names_array or unsync_guc_names_array.
* For system defined GUC must assign a tag of {GUC_GPDB_NEED_SYNC,
* GUC_GPDB_NO_SYNC, GUC_GPDB_NO_DISPATCH}. We deprecated direct define in guc.c,
* instead add into sync_guc_names_array or unsync_guc_names_array or undispatch_guc_names_array.
*/
static const char *sync_guc_names_array[] =
{
Expand All @@ -5159,8 +5159,27 @@ static const char *unsync_guc_names_array[] =
#include "utils/unsync_guc_name.h"
};

int sync_guc_num = 0;
int unsync_guc_num = 0;
static const char *undispatch_guc_names_array[] =
{
#include "utils/undispatch_guc_name.h"
};

static const struct guc_names_type GucNamesArray_gp[] =
{
{
sync_guc_names_array, sizeof(sync_guc_names_array) / sizeof(char *), GUC_GPDB_NEED_SYNC
},
{
unsync_guc_names_array, sizeof(unsync_guc_names_array) / sizeof(char *), GUC_GPDB_NO_SYNC
},
{
undispatch_guc_names_array, sizeof(undispatch_guc_names_array) / sizeof(char *), GUC_GPDB_NO_DISPATCH
},
/* End-of-list marker */
{
NULL, 0, 0
}
};

static int guc_array_compare(const void *a, const void *b)
{
Expand All @@ -5175,13 +5194,13 @@ void gpdb_assign_sync_flag(struct config_generic **guc_variables, int size, bool
static bool init = false;
/* ordering guc_name_array alphabets */
if (!init) {
sync_guc_num = sizeof(sync_guc_names_array) / sizeof(char *);
qsort((void *) sync_guc_names_array, sync_guc_num,
sizeof(char *), guc_array_compare);

unsync_guc_num = sizeof(unsync_guc_names_array) / sizeof(char *);
qsort((void *) unsync_guc_names_array, unsync_guc_num,
for (int i = 0; GucNamesArray_gp[i].guc_names_array; i++)
{
struct guc_names_type current_type = GucNamesArray_gp[i];
qsort((void *) (current_type.guc_names_array), current_type.num,
sizeof(char *), guc_array_compare);
}

init = true;
}
Expand All @@ -5191,41 +5210,40 @@ void gpdb_assign_sync_flag(struct config_generic **guc_variables, int size, bool
struct config_generic *var = guc_variables[i];

/* if the sync flags is defined in guc variable, skip it */
if (var->flags & (GUC_GPDB_NEED_SYNC | GUC_GPDB_NO_SYNC))
if (var->flags & (GUC_GPDB_NEED_SYNC | GUC_GPDB_NO_SYNC | GUC_GPDB_NO_DISPATCH))
continue;

char *res = (char *) bsearch((void *) &var->name,
(void *) sync_guc_names_array,
sync_guc_num,
sizeof(char *),
guc_array_compare);
if (!res)
char *res = NULL;
for (int j = 0; GucNamesArray_gp[j].guc_names_array; j++)
{
char *res = (char *) bsearch((void *) &var->name,
(void *) unsync_guc_names_array,
unsync_guc_num,
sizeof(char *),
guc_array_compare);

/* for predefined guc, we force its name in one array.
* for the third-part libraries gucs introduced by customer
* we assign unsync flags as default.
*/
if (!res && predefine)
struct guc_names_type current_type = GucNamesArray_gp[j];
res = (char *) bsearch((void *) &var->name,
(void *) (current_type.guc_names_array),
current_type.num,
sizeof(char *),
guc_array_compare);

if (res)
{
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
errmsg("Neither sync_guc_names_array nor "
"unsync_guc_names_array contains predefined "
"guc name: %s", var->name)));
}

var->flags |= GUC_GPDB_NO_SYNC;
var->flags |= current_type.flag;
break;
}
}
else
/* for predefined guc, we force its name in one array.
* for the third-part libraries gucs introduced by customer
* we assign unsync flags as default.
*/
if (!res && predefine)
{
var->flags |= GUC_GPDB_NEED_SYNC;
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
errmsg("sync_guc_names_array or unsync_guc_names_array or "
"undispatch_guc_names_array doesn't contain predefined "
"guc name: %s", var->name)));
}

if (!res)
var->flags |= GUC_GPDB_NO_DISPATCH;
}
}

Expand Down
41 changes: 19 additions & 22 deletions src/backend/utils/misc/test/guc_gp_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,32 @@ void init(void );

void init()
{
sync_guc_num = sizeof(sync_guc_names_array)/ sizeof(char *);
unsync_guc_num = sizeof(unsync_guc_names_array)/ sizeof(char *);
qsort((void *) sync_guc_names_array, sync_guc_num,
sizeof(char *), guc_array_compare);

qsort((void *) unsync_guc_names_array, unsync_guc_num,
for (int i = 0; GucNamesArray_gp[i].guc_names_array; i++)
{
struct guc_names_type current_type = GucNamesArray_gp[i];
qsort((void *) (current_type.guc_names_array), current_type.num,
sizeof(char *), guc_array_compare);
}
}

static void assert_guc(struct config_generic *conf)
{
char *res = (char *) bsearch((void *) &conf->name,
(void *) sync_guc_names_array,
sync_guc_num,
sizeof(char *),
guc_array_compare);
if (!res)
char *res = NULL;
for (int i = 0; GucNamesArray_gp[i].guc_names_array; i++)
{
char *res = (char *) bsearch((void *) &conf->name,
(void *) unsync_guc_names_array,
unsync_guc_num,
sizeof(char *),
guc_array_compare);

if ( res == NULL)
printf("GUC: '%s' does not exist in both list.\n", conf->name);

assert_true(res);
struct guc_names_type current_type = GucNamesArray_gp[i];
res = (char *) bsearch((void *) &conf->name,
(void *) (current_type.guc_names_array),
current_type.num,
sizeof(char *),
guc_array_compare);

if (res)
break;
}
if ( res == NULL)
printf("GUC: '%s' does not exist in lists.\n", conf->name);
assert_true(res);
}

static void
Expand Down
83 changes: 58 additions & 25 deletions src/backend/utils/misc/test/guc_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,40 +24,56 @@ static const char *unsync_guc_names_array[] =
{
#include "utils/unsync_guc_name.h"
};
static const char *undispatch_guc_names_array[] =
{
#include "utils/undispatch_guc_name.h"
};
void init(void );
extern int sync_guc_num;
extern int unsync_guc_num;

static const struct guc_names_type GucNamesArray_gp[] =
{
{
sync_guc_names_array, sizeof(sync_guc_names_array) / sizeof(char *), GUC_GPDB_NEED_SYNC
},
{
unsync_guc_names_array, sizeof(unsync_guc_names_array) / sizeof(char *), GUC_GPDB_NO_SYNC
},
{
undispatch_guc_names_array, sizeof(undispatch_guc_names_array) / sizeof(char *), GUC_GPDB_NO_DISPATCH
},
/* End-of-list marker */
{
NULL, 0, 0
}
};
void init()
{
sync_guc_num = sizeof(sync_guc_names_array)/ sizeof(char *);
unsync_guc_num = sizeof(unsync_guc_names_array)/ sizeof(char *);
qsort((void *) sync_guc_names_array, sync_guc_num,
sizeof(char *), guc_array_compare);

qsort((void *) unsync_guc_names_array, unsync_guc_num,
for (int i = 0; GucNamesArray_gp[i].guc_names_array; i++)
{
struct guc_names_type current_type = GucNamesArray_gp[i];
qsort((void *) (current_type.guc_names_array), current_type.num,
sizeof(char *), guc_array_compare);
}
}

static void assert_guc(struct config_generic *conf)
{
char *res = (char *) bsearch((void *) &conf->name,
(void *) sync_guc_names_array,
sync_guc_num,
sizeof(char *),
guc_array_compare);
if (!res)
char *res = NULL;
for (int i = 0; GucNamesArray_gp[i].guc_names_array; i++)
{
char *res = (char *) bsearch((void *) &conf->name,
(void *) unsync_guc_names_array,
unsync_guc_num,
sizeof(char *),
guc_array_compare);

if ( res == NULL)
printf("GUC: '%s' does not exist in both list.\n", conf->name);

assert_true(res);
struct guc_names_type current_type = GucNamesArray_gp[i];
res = (char *) bsearch((void *) &conf->name,
(void *) (current_type.guc_names_array),
current_type.num,
sizeof(char *),
guc_array_compare);

if (res)
break;
}
if ( res == NULL)
printf("GUC: '%s' does not exist in lists.\n", conf->name);
assert_true(res);
}

static void
Expand Down Expand Up @@ -121,6 +137,9 @@ test_enum_guc_coverage(void **state)
static void
test_guc_name_list_mutual_exclusion(void **state)
{
int sync_guc_num = sizeof(sync_guc_names_array) / sizeof(char *);
int unsync_guc_num = sizeof(unsync_guc_names_array) / sizeof(char *);
int undispatch_guc_num = sizeof(undispatch_guc_names_array) / sizeof(char *);
for (int i = 0; i < sync_guc_num; i++)
{
char *res = (char *) bsearch((void *) &(sync_guc_names_array[i]),
Expand All @@ -130,7 +149,21 @@ test_guc_name_list_mutual_exclusion(void **state)
guc_array_compare);

if ( res != NULL)
printf("GUC: '%s' exist in both list.\n", sync_guc_names_array[i]);
printf("GUC: '%s' exist in both list(sync, unsync).\n", sync_guc_names_array[i]);

assert_true(res == NULL);
}

for (int i = 0; i < unsync_guc_num; i++)
{
char *res = (char *) bsearch((void *) &(unsync_guc_names_array[i]),
(void *) undispatch_guc_names_array,
undispatch_guc_num,
sizeof(char *),
guc_array_compare);

if ( res != NULL)
printf("GUC: '%s' exist in both list(unsync, undispatch).\n", unsync_guc_names_array[i]);

assert_true(res == NULL);
}
Expand Down
1 change: 1 addition & 0 deletions src/include/utils/guc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ typedef enum
#define GUC_DISALLOW_USER_SET 0x00200000 /* Do not allow this GUC to be set by the user */
#define GUC_GPDB_NEED_SYNC 0x00400000 /* guc value is synced between master and primary */
#define GUC_GPDB_NO_SYNC 0x00800000 /* guc value is not synced between master and primary */
#define GUC_GPDB_NO_DISPATCH 0x01000000 /* guc value is not dispatched between master and primary */

/* GUC lists for gp_guc_list_show(). (List of struct config_generic) */
extern List *gp_guc_list_for_explain;
Expand Down
7 changes: 7 additions & 0 deletions src/include/utils/guc_tables.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,13 @@ struct config_enum
void *reset_extra;
};

struct guc_names_type
{
const char **guc_names_array;
int num;
int flag;
};

/* constant tables corresponding to enums above and in guc.h */
extern const char *const config_group_names[];
extern const char *const config_type_names[];
Expand Down
Loading

0 comments on commit 0ebcc4b

Please sign in to comment.