($INBOX_DIR/description missing)
help / color / mirror / Atom feedRe: Rewriting the test of pg_upgrade as a TAP test - take two
12+ messages / 5 participants
[nested] [flat]
* Re: Rewriting the test of pg_upgrade as a TAP test - take two
@ 2018-04-02 14:35 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Peter Eisentraut @ 2018-04-02 14:35 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Craig Ringer <[email protected]>; +Cc: David Steele <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers
On 3/21/18 21:49, Michael Paquier wrote:
> On Thu, Mar 22, 2018 at 09:42:35AM +0800, Craig Ringer wrote:
>> I'm super excited by the idea of multi-version support in TAP, if that's
>> what you mean.
>>
>> Why? Because I use TAP heavily in extensions. Especially replication
>> extensions. Which like to talk across multiple versions. I currently need
>> external test frameworks and some hideous hacks to do this.
>
> Okay, in front of such enthusiasm we could keep at least the refactoring
> part of PostgresNode.pm :)
I took a quick look at that part. It appears to be quite invasive, more
than I would have hoped. Basically, it imposes that from now on all
program invocations must observe the bindir setting, which someone is
surely going to forget. The pg_upgrade tests aren't going to exercise
all possible paths where programs are called, so this is going to lead
to omissions and inconsistencies -- which will then possibly only be
found much later by the extensions that Craig was talking about. I'd
like to see this more isolated, maybe via a path change, or something
inside system_or_bail or something less invasive and more maintainable.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Rewriting the test of pg_upgrade as a TAP test - take two
@ 2018-04-03 06:04 Michael Paquier <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2018-04-03 06:04 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Craig Ringer <[email protected]>; David Steele <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers
On Mon, Apr 02, 2018 at 10:35:09AM -0400, Peter Eisentraut wrote:
> I took a quick look at that part. It appears to be quite invasive, more
> than I would have hoped. Basically, it imposes that from now on all
> program invocations must observe the bindir setting, which someone is
> surely going to forget. The pg_upgrade tests aren't going to exercise
> all possible paths where programs are called, so this is going to lead
> to omissions and inconsistencies -- which will then possibly only be
> found much later by the extensions that Craig was talking about. I'd
> like to see this more isolated, maybe via a path change, or something
> inside system_or_bail or something less invasive and more maintainable.
Hm. PostgresNode.pm uses now a couple of different ways to trigger
commands, which makes the problem harder than it seems:
- system_or_bail
- system_log
- IPC::Run::timeout
- IPC::Run::run
Because of that, and if one wants to minimize the number of places where
bindir is called, would be to modify the command strings themselves.
What I could think of would be to have a wrapper like build_bin_path
which adds $bindir on top of the binary, but that would still cause all
the callers to use this wrapper. The use of this wrapper could still be
forgotten.
Enforcing PATH temporarily in a code path or another implies that the
previous value of PATH needs to be added back, which could be forgotten
as well.
At the end, it seems to me that what I am proposing is themost readable
approach, and with proper documentation things should be handled
finely... Or there is an approach you have in mind I do not foresee?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Rewriting the test of pg_upgrade as a TAP test - take two
@ 2018-04-09 00:38 Michael Paquier <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2018-04-09 00:38 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Craig Ringer <[email protected]>; David Steele <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers
On Tue, Apr 03, 2018 at 03:04:10PM +0900, Michael Paquier wrote:
> At the end, it seems to me that what I am proposing is themost readable
> approach, and with proper documentation things should be handled
> finely... Or there is an approach you have in mind I do not foresee?
With feature freeze which has been reached, I am marking this patch as
returned with feedback. Please note that I have no plans to resubmit
again this patch set after this second attempt as I have no idea where
we want things to go.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v13 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 4aeccd93af..3629caba42 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -287,6 +287,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.
*
@@ -304,6 +310,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 */
@@ -1418,39 +1440,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 */
@@ -1582,21 +1604,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.
@@ -1608,23 +1646,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;
@@ -1633,12 +1665,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;
@@ -1656,12 +1689,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;
@@ -1676,12 +1709,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);
}
/*
@@ -1740,7 +1774,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,
@@ -1761,7 +1795,7 @@ assignVariables(CState *st, char *sql)
continue;
}
- val = getVariable(st, name);
+ val = getVariable(variables, name);
free(name);
if (val == NULL)
{
@@ -1776,12 +1810,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 *
@@ -2649,7 +2684,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;
@@ -2719,7 +2754,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,
@@ -2750,7 +2785,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;
@@ -2811,7 +2846,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);
@@ -2863,7 +2898,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);
@@ -2874,7 +2909,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,
@@ -2921,7 +2956,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);
@@ -3014,7 +3049,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 */
@@ -3075,14 +3110,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;
@@ -3614,7 +3649,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;
@@ -3635,7 +3670,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;
@@ -3705,7 +3740,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;
@@ -3713,7 +3748,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;
@@ -5995,7 +6030,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;
@@ -6335,19 +6370,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);
}
@@ -6382,11 +6417,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);
}
}
@@ -6395,30 +6430,32 @@ 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) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) |
(((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) << 32);
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
--Multipart=_Thu__1_Jul_2021_00_02_13_+0900_iyONG0RfOE79H0vs--
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v15 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 364b5a2e47..c4c2fd3566 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -287,6 +287,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.
*
@@ -304,6 +310,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 */
@@ -466,9 +490,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 */
@@ -1424,39 +1446,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 */
@@ -1588,21 +1610,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.
@@ -1614,23 +1652,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;
@@ -1639,12 +1671,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;
@@ -1662,12 +1695,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;
@@ -1682,12 +1715,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);
}
/*
@@ -1746,7 +1780,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,
@@ -1767,7 +1801,7 @@ assignVariables(CState *st, char *sql)
continue;
}
- val = getVariable(st, name);
+ val = getVariable(variables, name);
free(name);
if (val == NULL)
{
@@ -1782,12 +1816,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 *
@@ -2655,7 +2690,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;
@@ -2725,7 +2760,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,
@@ -2756,7 +2791,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;
@@ -2817,7 +2852,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);
@@ -2869,7 +2904,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);
@@ -2880,7 +2915,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,
@@ -2927,7 +2962,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);
@@ -3020,7 +3055,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 */
@@ -3081,14 +3116,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;
@@ -3620,7 +3655,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;
@@ -3641,7 +3676,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;
@@ -3711,7 +3746,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;
@@ -3719,7 +3754,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;
@@ -6013,7 +6048,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;
@@ -6353,19 +6388,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);
}
@@ -6400,11 +6435,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);
}
}
@@ -6413,30 +6448,32 @@ 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) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) |
(((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) << 32);
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
--Multipart=_Fri__16_Jul_2021_12_05_16_+0900_jCtOsvpoRs.oEJzk--
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v12 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 | 169 ++++++++++++++++++++++++--------------
1 file changed, 106 insertions(+), 63 deletions(-)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e61055b6b7..8acda86cad 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -287,6 +287,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.
*
@@ -304,6 +310,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 */
@@ -1418,39 +1440,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 */
@@ -1582,21 +1604,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.
@@ -1608,23 +1652,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;
@@ -1633,12 +1671,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;
@@ -1656,12 +1695,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;
@@ -1676,12 +1715,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);
}
/*
@@ -1740,7 +1780,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,
@@ -1761,7 +1801,7 @@ assignVariables(CState *st, char *sql)
continue;
}
- val = getVariable(st, name);
+ val = getVariable(variables, name);
free(name);
if (val == NULL)
{
@@ -1776,12 +1816,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 *
@@ -2647,7 +2688,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;
@@ -2717,7 +2758,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,
@@ -2748,7 +2789,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;
@@ -2809,7 +2850,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);
@@ -2861,7 +2902,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);
@@ -2872,7 +2913,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,
@@ -2919,7 +2960,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);
@@ -3012,7 +3053,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 */
@@ -3073,14 +3114,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;
@@ -3612,7 +3653,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;
@@ -3633,7 +3674,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;
@@ -3703,7 +3744,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;
@@ -3711,7 +3752,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;
@@ -5993,7 +6034,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;
@@ -6333,19 +6374,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);
}
@@ -6380,11 +6421,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);
}
}
@@ -6393,30 +6434,32 @@ 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) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) |
(((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) << 32);
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
--Multipart=_Tue__22_Jun_2021_23_54_59_+0900_JJJkapGYdP+ALwtM--
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v16 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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
--Multipart=_Fri__18_Feb_2022_23_45_49_+0900_/e7EPEHuui1P_=wo--
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v17 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 000ffc4a5c..ab2c5dfc5f 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
--Multipart=_Sun__20_Mar_2022_03_23_21_+0900_8dUZljuxkcYgB2tW
Content-Type: text/x-diff;
name="v17-0002-Pgbench-errors-and-serialization-deadlock-retrie.patch"
Content-Disposition: attachment;
filename="v17-0002-Pgbench-errors-and-serialization-deadlock-retrie.patch"
Content-Transfer-Encoding: 7bit
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v18 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 000ffc4a5c..ab2c5dfc5f 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
--Multipart=_Tue__22_Mar_2022_17_27_14_+0900_PTQR43_rmqjQDpKM
Content-Type: text/x-diff;
name="v18-0002-Pgbench-errors-and-serialization-deadlock-retrie.patch"
Content-Disposition: attachment;
filename="v18-0002-Pgbench-errors-and-serialization-deadlock-retrie.patch"
Content-Transfer-Encoding: 7bit
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v14 1/2] Pgbench errors: use the Variables structure for client variables
@ 2021-05-26 07:58 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Yugo Nagata @ 2021-05-26 07:58 UTC (permalink / raw)
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 4aeccd93af..3629caba42 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -287,6 +287,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.
*
@@ -304,6 +310,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 */
@@ -1418,39 +1440,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 */
@@ -1582,21 +1604,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.
@@ -1608,23 +1646,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;
@@ -1633,12 +1665,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;
@@ -1656,12 +1689,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;
@@ -1676,12 +1709,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);
}
/*
@@ -1740,7 +1774,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,
@@ -1761,7 +1795,7 @@ assignVariables(CState *st, char *sql)
continue;
}
- val = getVariable(st, name);
+ val = getVariable(variables, name);
free(name);
if (val == NULL)
{
@@ -1776,12 +1810,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 *
@@ -2649,7 +2684,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;
@@ -2719,7 +2754,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,
@@ -2750,7 +2785,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;
@@ -2811,7 +2846,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);
@@ -2863,7 +2898,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);
@@ -2874,7 +2909,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,
@@ -2921,7 +2956,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);
@@ -3014,7 +3049,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 */
@@ -3075,14 +3110,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;
@@ -3614,7 +3649,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;
@@ -3635,7 +3670,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;
@@ -3705,7 +3740,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;
@@ -3713,7 +3748,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;
@@ -5995,7 +6030,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;
@@ -6335,19 +6370,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);
}
@@ -6382,11 +6417,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);
}
}
@@ -6395,30 +6430,32 @@ 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) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) |
(((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) << 32);
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
--Multipart=_Thu__8_Jul_2021_08_35_06_+0900_rt42K=fIffwe5=xI--
^ permalink raw reply [nested|flat] 12+ messages in thread
* Fix smgrtruncate code comment.
@ 2024-07-29 10:50 Kirill Reshke <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Kirill Reshke @ 2024-07-29 10:50 UTC (permalink / raw)
To: pgsql-hackers
Today I was busy rebasing my Greenplum-related Extendable SMGR
patches on Cloudberry, and I faced some conflicts.
While resolving them I noticed code & comments inconsistency in smgr.c
in smgrtruncate function, which tracks down to
c5315f4f44843c20ada876fdb0d0828795dfbdf5. In this commit,
smgr_fsm_nblocks & smgr_vm_nblocks fields were removed, however
comments were not fixed accordingly.
So i suggest to fix this, PFA
Attachments:
[application/octet-stream] v1-0001-Fix-smgrtruncate-code-comment.patch (1.1K, ../../CALdSSPh9VA6SDSVjrcmSPEYramf+rFisK7GqJo1dtRnD3vddmA@mail.gmail.com/2-v1-0001-Fix-smgrtruncate-code-comment.patch)
download | inline diff:
From bea97f2f35e6f6fce9c4a1c29158fd1433a27680 Mon Sep 17 00:00:00 2001
From: reshke <[email protected]>
Date: Mon, 29 Jul 2024 10:44:22 +0000
Subject: [PATCH v1] Fix smgrtruncate code comment.
---
src/backend/storage/smgr/smgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index a691aed1f4..e8867f860d 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -733,8 +733,8 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nb
/*
* We might as well update the local smgr_cached_nblocks values. The
* smgr cache inval message that this function sent will cause other
- * backends to invalidate their copies of smgr_fsm_nblocks and
- * smgr_vm_nblocks, and these ones too at the next command boundary.
+ * backends to invalidate their copies of smgr_cached_nblocks,
+ * and these ones too at the next command boundary.
* But these ensure they aren't outright wrong until then.
*/
reln->smgr_cached_nblocks[forknum[i]] = nblocks[i];
--
2.25.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Fix smgrtruncate code comment.
@ 2024-07-29 11:27 Heikki Linnakangas <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Heikki Linnakangas @ 2024-07-29 11:27 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: pgsql-hackers
On 29/07/2024 13:50, Kirill Reshke wrote:
> Today I was busy rebasing my Greenplum-related Extendable SMGR
> patches on Cloudberry, and I faced some conflicts.
> While resolving them I noticed code & comments inconsistency in smgr.c
> in smgrtruncate function, which tracks down to
> c5315f4f44843c20ada876fdb0d0828795dfbdf5. In this commit,
> smgr_fsm_nblocks & smgr_vm_nblocks fields were removed, however
> comments were not fixed accordingly.
>
> So i suggest to fix this, PFA
Applied, thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2024-07-29 11:27 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-04-02 14:35 Re: Rewriting the test of pg_upgrade as a TAP test - take two Peter Eisentraut <[email protected]>
2018-04-03 06:04 ` Michael Paquier <[email protected]>
2018-04-09 00:38 ` Michael Paquier <[email protected]>
2021-05-26 07:58 [PATCH v13 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v15 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v16 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v17 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v14 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v12 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2021-05-26 07:58 [PATCH v18 1/2] Pgbench errors: use the Variables structure for client variables Yugo Nagata <[email protected]>
2024-07-29 10:50 Fix smgrtruncate code comment. Kirill Reshke <[email protected]>
2024-07-29 11:27 ` Re: Fix smgrtruncate code comment. Heikki Linnakangas <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox