From ffbf34d489f8f01621866908b9dd418459bb0b45 Mon Sep 17 00:00:00 2001 From: Marina Polyakova Date: Wed, 5 Sep 2018 16:03:15 +0300 Subject: [PATCH v11 2/4] Pgbench errors: use the Variables structure for client variables This is most important when it is used to reset client variables during the repeating of transactions after serialization/deadlock failures. Don't allocate Variable structs one by one. Instead, add a constant margin each time it overflows. --- src/bin/pgbench/pgbench.c | 171 ++++++++++++++++++++++++-------------- 1 file changed, 109 insertions(+), 62 deletions(-) diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 988e37bce5..1b25487bfc 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -201,6 +201,12 @@ const char *progname; volatile bool timer_exceeded = false; /* flag from signal handler */ +/* + * We don't want to allocate variables one by one; for efficiency, add a + * constant margin each time it overflows. + */ +#define VARIABLES_ALLOC_MARGIN 8 + /* * Variable definitions. * @@ -218,6 +224,24 @@ typedef struct PgBenchValue value; /* actual variable's value */ } Variable; +/* + * Data structure for client variables. + */ +typedef struct +{ + Variable *vars; /* array of variable definitions */ + int nvars; /* number of variables */ + + /* + * The maximum number of variables that we can currently store in 'vars' + * without having to reallocate more space. We must always have max_vars >= + * nvars. + */ + int max_vars; + + bool vars_sorted; /* are variables sorted by name? */ +} Variables; + #define MAX_SCRIPTS 128 /* max number of SQL scripts allowed */ #define SHELL_COMMAND_SIZE 256 /* maximum size allowed for shell command */ @@ -349,9 +373,7 @@ typedef struct int command; /* command number in script */ /* client variables */ - Variable *variables; /* array of variable definitions */ - int nvariables; /* number of variables */ - bool vars_sorted; /* are variables sorted by name? */ + Variables variables; /* various times about current transaction */ int64 txn_scheduled; /* scheduled start time of transaction (usec) */ @@ -1212,39 +1234,39 @@ compareVariableNames(const void *v1, const void *v2) /* Locate a variable by name; returns NULL if unknown */ static Variable * -lookupVariable(CState *st, char *name) +lookupVariable(Variables *variables, char *name) { Variable key; /* On some versions of Solaris, bsearch of zero items dumps core */ - if (st->nvariables <= 0) + if (variables->nvars <= 0) return NULL; /* Sort if we have to */ - if (!st->vars_sorted) + if (!variables->vars_sorted) { - qsort((void *) st->variables, st->nvariables, sizeof(Variable), + qsort((void *) variables->vars, variables->nvars, sizeof(Variable), compareVariableNames); - st->vars_sorted = true; + variables->vars_sorted = true; } /* Now we can search */ key.name = name; return (Variable *) bsearch((void *) &key, - (void *) st->variables, - st->nvariables, + (void *) variables->vars, + variables->nvars, sizeof(Variable), compareVariableNames); } /* Get the value of a variable, in string form; returns NULL if unknown */ static char * -getVariable(CState *st, char *name) +getVariable(Variables *variables, char *name) { Variable *var; char stringform[64]; - var = lookupVariable(st, name); + var = lookupVariable(variables, name); if (var == NULL) return NULL; /* not found */ @@ -1362,21 +1384,43 @@ valid_variable_name(const char *name) return true; } +/* + * Make sure there is enough space for 'needed' more variable in the variables + * array. It is assumed that the sum of the number of current variables and the + * number of needed variables is less than or equal to (INT_MAX - + * VARIABLES_ALLOC_MARGIN). + */ +static void +enlargeVariables(Variables *variables, int needed) +{ + /* total number of variables required now */ + needed += variables->nvars; + + if (variables->max_vars < needed) + { + /* + * We don't want to allocate variables one by one; for efficiency, add a + * constant margin each time it overflows. + */ + variables->max_vars = needed + VARIABLES_ALLOC_MARGIN; + variables->vars = (Variable *) + pg_realloc(variables->vars, variables->max_vars * sizeof(Variable)); + } +} + /* * Lookup a variable by name, creating it if need be. * Caller is expected to assign a value to the variable. * Returns NULL on failure (bad name). */ static Variable * -lookupCreateVariable(CState *st, const char *context, char *name) +lookupCreateVariable(Variables *variables, const char *context, char *name) { Variable *var; - var = lookupVariable(st, name); + var = lookupVariable(variables, name); if (var == NULL) { - Variable *newvars; - /* * Check for the name only when declaring a new variable to avoid * overhead. @@ -1389,23 +1433,17 @@ lookupCreateVariable(CState *st, const char *context, char *name) } /* Create variable at the end of the array */ - if (st->variables) - newvars = (Variable *) pg_realloc(st->variables, - (st->nvariables + 1) * sizeof(Variable)); - else - newvars = (Variable *) pg_malloc(sizeof(Variable)); - - st->variables = newvars; + enlargeVariables(variables, 1); - var = &newvars[st->nvariables]; + var = &(variables->vars[variables->nvars]); var->name = pg_strdup(name); var->svalue = NULL; /* caller is expected to initialize remaining fields */ - st->nvariables++; + variables->nvars++; /* we don't re-sort the array till we have to */ - st->vars_sorted = false; + variables->vars_sorted = false; } return var; @@ -1414,12 +1452,13 @@ lookupCreateVariable(CState *st, const char *context, char *name) /* Assign a string value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ static bool -putVariable(CState *st, const char *context, char *name, const char *value) +putVariable(Variables *variables, const char *context, char *name, + const char *value) { Variable *var; char *val; - var = lookupCreateVariable(st, context, name); + var = lookupCreateVariable(variables, context, name); if (!var) return false; @@ -1437,12 +1476,12 @@ putVariable(CState *st, const char *context, char *name, const char *value) /* Assign a value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ static bool -putVariableValue(CState *st, const char *context, char *name, +putVariableValue(Variables *variables, const char *context, char *name, const PgBenchValue *value) { Variable *var; - var = lookupCreateVariable(st, context, name); + var = lookupCreateVariable(variables, context, name); if (!var) return false; @@ -1457,12 +1496,13 @@ putVariableValue(CState *st, const char *context, char *name, /* Assign an integer value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ static bool -putVariableInt(CState *st, const char *context, char *name, int64 value) +putVariableInt(Variables *variables, const char *context, char *name, + int64 value) { PgBenchValue val; setIntValue(&val, value); - return putVariableValue(st, context, name, &val); + return putVariableValue(variables, context, name, &val); } /* @@ -1517,7 +1557,7 @@ replaceVariable(char **sql, char *param, int len, char *value) } static char * -assignVariables(CState *st, char *sql) +assignVariables(Variables *variables, char *sql) { char *p, *name, @@ -1538,7 +1578,7 @@ assignVariables(CState *st, char *sql) continue; } - val = getVariable(st, name); + val = getVariable(variables, name); free(name); if (val == NULL) { @@ -1553,12 +1593,13 @@ assignVariables(CState *st, char *sql) } static void -getQueryParams(CState *st, const Command *command, const char **params) +getQueryParams(Variables *variables, const Command *command, + const char **params) { int i; for (i = 0; i < command->argc - 1; i++) - params[i] = getVariable(st, command->argv[i + 1]); + params[i] = getVariable(variables, command->argv[i + 1]); } static char * @@ -2390,7 +2431,7 @@ evaluateExpr(TState *thread, CState *st, PgBenchExpr *expr, PgBenchValue *retval { Variable *var; - if ((var = lookupVariable(st, expr->u.variable.varname)) == NULL) + if ((var = lookupVariable(&st->variables, expr->u.variable.varname)) == NULL) { fprintf(stderr, "undefined variable \"%s\"\n", expr->u.variable.varname); @@ -2454,7 +2495,7 @@ getMetaCommand(const char *cmd) * Return true if succeeded, or false on error. */ static bool -runShellCommand(CState *st, char *variable, char **argv, int argc) +runShellCommand(Variables *variables, char *variable, char **argv, int argc) { char command[SHELL_COMMAND_SIZE]; int i, @@ -2485,7 +2526,7 @@ runShellCommand(CState *st, char *variable, char **argv, int argc) { arg = argv[i] + 1; /* a string literal starting with colons */ } - else if ((arg = getVariable(st, argv[i] + 1)) == NULL) + else if ((arg = getVariable(variables, argv[i] + 1)) == NULL) { fprintf(stderr, "%s: undefined variable \"%s\"\n", argv[0], argv[i]); @@ -2548,7 +2589,7 @@ runShellCommand(CState *st, char *variable, char **argv, int argc) argv[0], res); return false; } - if (!putVariableInt(st, "setshell", variable, retval)) + if (!putVariableInt(variables, "setshell", variable, retval)) return false; #ifdef DEBUG @@ -2602,7 +2643,7 @@ sendCommand(CState *st, Command *command) char *sql; sql = pg_strdup(command->argv[0]); - sql = assignVariables(st, sql); + sql = assignVariables(&st->variables, sql); if (debug) fprintf(stderr, "client %d sending %s\n", st->id, sql); @@ -2614,7 +2655,7 @@ sendCommand(CState *st, Command *command) const char *sql = command->argv[0]; const char *params[MAX_ARGS]; - getQueryParams(st, command, params); + getQueryParams(&st->variables, command, params); if (debug) fprintf(stderr, "client %d sending %s\n", st->id, sql); @@ -2648,7 +2689,7 @@ sendCommand(CState *st, Command *command) st->prepared[st->use_file] = true; } - getQueryParams(st, command, params); + getQueryParams(&st->variables, command, params); preparedStatementName(name, st->use_file, st->command); if (debug) @@ -2676,14 +2717,14 @@ sendCommand(CState *st, Command *command) * of delay, in microseconds. Returns true on success, false on error. */ static bool -evaluateSleep(CState *st, int argc, char **argv, int *usecs) +evaluateSleep(Variables *variables, int argc, char **argv, int *usecs) { char *var; int usec; if (*argv[1] == ':') { - if ((var = getVariable(st, argv[1] + 1)) == NULL) + if ((var = getVariable(variables, argv[1] + 1)) == NULL) { fprintf(stderr, "%s: undefined variable \"%s\"\n", argv[0], argv[1]); @@ -2956,7 +2997,7 @@ doCustom(TState *thread, CState *st, StatsData *agg) */ int usec; - if (!evaluateSleep(st, argc, argv, &usec)) + if (!evaluateSleep(&st->variables, argc, argv, &usec)) { commandFailed(st, "sleep", "execution of meta-command failed"); st->state = CSTATE_ABORTED; @@ -2997,7 +3038,8 @@ doCustom(TState *thread, CState *st, StatsData *agg) if (command->meta == META_SET) { - if (!putVariableValue(st, argv[0], argv[1], &result)) + if (!putVariableValue(&st->variables, argv[0], + argv[1], &result)) { commandFailed(st, "set", "assignment of meta-command failed"); st->state = CSTATE_ABORTED; @@ -3050,7 +3092,9 @@ doCustom(TState *thread, CState *st, StatsData *agg) } else if (command->meta == META_SETSHELL) { - bool ret = runShellCommand(st, argv[1], argv + 2, argc - 2); + bool ret = runShellCommand(&st->variables, + argv[1], argv + 2, + argc - 2); if (timer_exceeded) /* timeout */ { @@ -3070,7 +3114,8 @@ doCustom(TState *thread, CState *st, StatsData *agg) } else if (command->meta == META_SHELL) { - bool ret = runShellCommand(st, NULL, argv + 1, argc - 1); + bool ret = runShellCommand(&st->variables, NULL, + argv + 1, argc - 1); if (timer_exceeded) /* timeout */ { @@ -5075,7 +5120,7 @@ main(int argc, char **argv) } *p++ = '\0'; - if (!putVariable(&state[0], "option", optarg, p)) + if (!putVariable(&state[0].variables, "option", optarg, p)) exit(1); } break; @@ -5377,19 +5422,19 @@ main(int argc, char **argv) int j; state[i].id = i; - for (j = 0; j < state[0].nvariables; j++) + for (j = 0; j < state[0].variables.nvars; j++) { - Variable *var = &state[0].variables[j]; + Variable *var = &state[0].variables.vars[j]; if (var->value.type != PGBT_NO_VALUE) { - if (!putVariableValue(&state[i], "startup", + if (!putVariableValue(&state[i].variables, "startup", var->name, &var->value)) exit(1); } else { - if (!putVariable(&state[i], "startup", + if (!putVariable(&state[i].variables, "startup", var->name, var->svalue)) exit(1); } @@ -5465,11 +5510,11 @@ main(int argc, char **argv) * :scale variables normally get -s or database scale, but don't override * an explicit -D switch */ - if (lookupVariable(&state[0], "scale") == NULL) + if (lookupVariable(&state[0].variables, "scale") == NULL) { for (i = 0; i < nclients; i++) { - if (!putVariableInt(&state[i], "startup", "scale", scale)) + if (!putVariableInt(&state[i].variables, "startup", "scale", scale)) exit(1); } } @@ -5478,15 +5523,15 @@ main(int argc, char **argv) * Define a :client_id variable that is unique per connection. But don't * override an explicit -D switch. */ - if (lookupVariable(&state[0], "client_id") == NULL) + if (lookupVariable(&state[0].variables, "client_id") == NULL) { for (i = 0; i < nclients; i++) - if (!putVariableInt(&state[i], "startup", "client_id", i)) + if (!putVariableInt(&state[i].variables, "startup", "client_id", i)) exit(1); } /* set default seed for hash functions */ - if (lookupVariable(&state[0], "default_seed") == NULL) + if (lookupVariable(&state[0].variables, "default_seed") == NULL) { uint64 seed = ((uint64) (random() & 0xFFFF) << 48) | ((uint64) (random() & 0xFFFF) << 32) | @@ -5494,15 +5539,17 @@ main(int argc, char **argv) (uint64) (random() & 0xFFFF); for (i = 0; i < nclients; i++) - if (!putVariableInt(&state[i], "startup", "default_seed", (int64) seed)) + if (!putVariableInt(&state[i].variables, "startup", "default_seed", + (int64) seed)) exit(1); } /* set random seed unless overwritten */ - if (lookupVariable(&state[0], "random_seed") == NULL) + if (lookupVariable(&state[0].variables, "random_seed") == NULL) { for (i = 0; i < nclients; i++) - if (!putVariableInt(&state[i], "startup", "random_seed", random_seed)) + if (!putVariableInt(&state[i].variables, "startup", "random_seed", + random_seed)) exit(1); } -- 2.17.1