From 12eb5ff63cb92adeee86937104e2cbb274337289 Mon Sep 17 00:00:00 2001 From: Yugo Nagata Date: Wed, 26 May 2021 16:58:36 +0900 Subject: [PATCH v16 1/2] 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 | 163 +++++++++++++++++++++++--------------- 1 file changed, 100 insertions(+), 63 deletions(-) diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index f166a77e3a..3743e36dac 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -289,6 +289,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. * @@ -306,6 +312,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 */ @@ -460,9 +484,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 in microseconds */ pg_time_usec_t txn_scheduled; /* scheduled start time of transaction */ @@ -1398,39 +1420,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 */ @@ -1562,21 +1584,37 @@ valid_variable_name(const char *name) return true; } +/* + * Make sure there is enough space for 'needed' more variable in the variables + * array. + */ +static void +enlargeVariables(Variables *variables, int needed) +{ + /* total number of variables required now */ + needed += variables->nvars; + + if (variables->max_vars < needed) + { + 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. @@ -1588,23 +1626,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; @@ -1613,12 +1645,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; @@ -1636,12 +1669,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; @@ -1656,12 +1689,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); } /* @@ -1720,7 +1754,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, @@ -1741,7 +1775,7 @@ assignVariables(CState *st, char *sql) continue; } - val = getVariable(st, name); + val = getVariable(variables, name); free(name); if (val == NULL) { @@ -1756,12 +1790,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 * @@ -2629,7 +2664,7 @@ evaluateExpr(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) { pg_log_error("undefined variable \"%s\"", expr->u.variable.varname); return false; @@ -2699,7 +2734,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, @@ -2730,7 +2765,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) { pg_log_error("%s: undefined variable \"%s\"", argv[0], argv[i]); return false; @@ -2791,7 +2826,7 @@ runShellCommand(CState *st, char *variable, char **argv, int argc) pg_log_error("%s: shell command must return an integer (not \"%s\")", argv[0], res); return false; } - if (!putVariableInt(st, "setshell", variable, retval)) + if (!putVariableInt(variables, "setshell", variable, retval)) return false; pg_log_debug("%s: shell parameter name: \"%s\", value: \"%s\"", argv[0], argv[1], res); @@ -2843,7 +2878,7 @@ sendCommand(CState *st, Command *command) char *sql; sql = pg_strdup(command->argv[0]); - sql = assignVariables(st, sql); + sql = assignVariables(&st->variables, sql); pg_log_debug("client %d sending %s", st->id, sql); r = PQsendQuery(st->con, sql); @@ -2854,7 +2889,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); pg_log_debug("client %d sending %s", st->id, sql); r = PQsendQueryParams(st->con, sql, command->argc - 1, @@ -2901,7 +2936,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); pg_log_debug("client %d sending %s", st->id, name); @@ -2994,7 +3029,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix) varname = psprintf("%s%s", varprefix, varname); /* store last row result as a string */ - if (!putVariable(st, meta == META_ASET ? "aset" : "gset", varname, + if (!putVariable(&st->variables, meta == META_ASET ? "aset" : "gset", varname, PQgetvalue(res, ntuples - 1, fld))) { /* internal error */ @@ -3055,14 +3090,14 @@ error: * 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) { pg_log_error("%s: undefined variable \"%s\"", argv[0], argv[1] + 1); return false; @@ -3627,7 +3662,7 @@ executeMetaCommand(CState *st, pg_time_usec_t *now) * latency will be recorded in CSTATE_SLEEP state, not here, after the * delay has elapsed.) */ - if (!evaluateSleep(st, argc, argv, &usec)) + if (!evaluateSleep(&st->variables, argc, argv, &usec)) { commandFailed(st, "sleep", "execution of meta-command failed"); return CSTATE_ABORTED; @@ -3648,7 +3683,7 @@ executeMetaCommand(CState *st, pg_time_usec_t *now) return CSTATE_ABORTED; } - 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"); return CSTATE_ABORTED; @@ -3718,7 +3753,7 @@ executeMetaCommand(CState *st, pg_time_usec_t *now) } else if (command->meta == META_SETSHELL) { - if (!runShellCommand(st, argv[1], argv + 2, argc - 2)) + if (!runShellCommand(&st->variables, argv[1], argv + 2, argc - 2)) { commandFailed(st, "setshell", "execution of meta-command failed"); return CSTATE_ABORTED; @@ -3726,7 +3761,7 @@ executeMetaCommand(CState *st, pg_time_usec_t *now) } else if (command->meta == META_SHELL) { - if (!runShellCommand(st, NULL, argv + 1, argc - 1)) + if (!runShellCommand(&st->variables, NULL, argv + 1, argc - 1)) { commandFailed(st, "shell", "execution of meta-command failed"); return CSTATE_ABORTED; @@ -6020,7 +6055,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; @@ -6348,19 +6383,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); } @@ -6398,11 +6433,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); } } @@ -6411,28 +6446,30 @@ 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 = pg_prng_uint64(&base_random_sequence); 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