Skip to content

Commit 34f13cc

Browse files
alvherrealvherre
committed
pgbench: cleanup use of a "logfile" parameter
There is no reason to have the per-thread logfile file pointer as a separate parameter in various functions: it's much simpler to put it in the per-thread state struct instead, which is already being passed to all functions that need the log file anyway. Change the callsites in which it was used as a boolean to test whether logging is active, so that they use the use_log global variable instead. No backpatch, even though this exists since commit a887c48 of March 2010, because this is just for cleanliness' sake and the surrounding code has been modified a lot recently anyway.
1 parent db94419 commit 34f13cc

File tree

1 file changed

+25
-19
lines changed

1 file changed

+25
-19
lines changed

‎src/bin/pgbench/pgbench.c

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ typedef struct
256256
int nstate; /* length of state[] */
257257
unsigned short random_state[3]; /* separate randomness for each thread */
258258
int64 throttle_trigger; /* previous/next throttling (us) */
259+
FILE *logfile; /* where to log, or NULL */
259260

260261
/* per thread collected stats */
261262
instr_time start_time; /* thread start time */
@@ -366,8 +367,8 @@ static void setalarm(int seconds);
366367
static void *threadRun(void *arg);
367368

368369
static void processXactStats(TState *thread, CState *st, instr_time *now,
369-
bool skipped, FILE *logfile, StatsData *agg);
370-
static void doLog(TState *thread, CState *st, FILE *logfile, instr_time *now,
370+
bool skipped, StatsData *agg);
371+
static void doLog(TState *thread, CState *st, instr_time *now,
371372
StatsData *agg, bool skipped, double latency, double lag);
372373

373374

@@ -1246,7 +1247,7 @@ chooseScript(TState *thread)
12461247

12471248
/* return false iff client should be disconnected */
12481249
static bool
1249-
doCustom(TState *thread, CState *st, FILE *logfile, StatsData *agg)
1250+
doCustom(TState *thread, CState *st, StatsData *agg)
12501251
{
12511252
PGresult *res;
12521253
Command **commands;
@@ -1300,7 +1301,7 @@ doCustom(TState *thread, CState *st, FILE *logfile, StatsData *agg)
13001301
now_us = INSTR_TIME_GET_MICROSEC(now);
13011302
while (thread->throttle_trigger < now_us - latency_limit)
13021303
{
1303-
processXactStats(thread, st, &now, true, logfile, agg);
1304+
processXactStats(thread, st, &now, true, agg);
13041305
/* next rendez-vous */
13051306
wait = getPoissonRand(thread, throttle_delay);
13061307
thread->throttle_trigger += wait;
@@ -1361,8 +1362,8 @@ doCustom(TState *thread, CState *st, FILE *logfile, StatsData *agg)
13611362
if (commands[st->state + 1] == NULL)
13621363
{
13631364
if (progress || throttle_delay || latency_limit ||
1364-
per_script_stats || logfile)
1365-
processXactStats(thread, st, &now, false, logfile, agg);
1365+
per_script_stats || use_log)
1366+
processXactStats(thread, st, &now, false, agg);
13661367
else
13671368
thread->stats.cnt++;
13681369
}
@@ -1454,7 +1455,8 @@ doCustom(TState *thread, CState *st, FILE *logfile, StatsData *agg)
14541455
}
14551456

14561457
/* Record transaction start time under logging, progress or throttling */
1457-
if ((logfile || progress || throttle_delay || latency_limit || per_script_stats) && st->state == 0)
1458+
if ((use_log || progress || throttle_delay || latency_limit ||
1459+
per_script_stats) && st->state == 0)
14581460
{
14591461
INSTR_TIME_SET_CURRENT(st->txn_begin);
14601462

@@ -1794,9 +1796,13 @@ doCustom(TState *thread, CState *st, FILE *logfile, StatsData *agg)
17941796
* print log entry after completing one transaction.
17951797
*/
17961798
static void
1797-
doLog(TState *thread, CState *st, FILE *logfile, instr_time *now,
1799+
doLog(TState *thread, CState *st, instr_time *now,
17981800
StatsData *agg, bool skipped, double latency, double lag)
17991801
{
1802+
FILE *logfile = thread->logfile;
1803+
1804+
Assert(use_log);
1805+
18001806
/*
18011807
* Skip the log entry if sampling is enabled and this row doesn't belong
18021808
* to the random sample.
@@ -1879,7 +1885,7 @@ doLog(TState *thread, CState *st, FILE *logfile, instr_time *now,
18791885
*/
18801886
static void
18811887
processXactStats(TState *thread, CState *st, instr_time *now,
1882-
bool skipped, FILE *logfile, StatsData *agg)
1888+
bool skipped, StatsData *agg)
18831889
{
18841890
double latency = 0.0,
18851891
lag = 0.0;
@@ -1906,7 +1912,7 @@ processXactStats(TState *thread, CState *st, instr_time *now,
19061912
thread->stats.cnt++;
19071913

19081914
if (use_log)
1909-
doLog(thread, st, logfile, now, agg, skipped, latency, lag);
1915+
doLog(thread, st, now, agg, skipped, latency, lag);
19101916

19111917
/* XXX could use a mutex here, but we choose not to */
19121918
if (per_script_stats)
@@ -3289,7 +3295,7 @@ main(int argc, char **argv)
32893295
exit(1);
32903296
}
32913297

3292-
/* --sampling-rate may must not be used with --aggregate-interval */
3298+
/* --sampling-rate may not be used with --aggregate-interval */
32933299
if (sample_rate > 0.0 && agg_interval > 0)
32943300
{
32953301
fprintf(stderr, "log sampling (--sampling-rate) and aggregation (--aggregate-interval) cannot be used at the same time\n");
@@ -3460,6 +3466,7 @@ main(int argc, char **argv)
34603466
thread->random_state[0] = random();
34613467
thread->random_state[1] = random();
34623468
thread->random_state[2] = random();
3469+
thread->logfile = NULL; /* filled in later */
34633470
thread->latency_late = 0;
34643471
initStats(&thread->stats, 0.0);
34653472

@@ -3555,7 +3562,6 @@ threadRun(void *arg)
35553562
{
35563563
TState *thread = (TState *) arg;
35573564
CState *state = thread->state;
3558-
FILE *logfile = NULL; /* per-thread log file */
35593565
instr_time start,
35603566
end;
35613567
int nstate = thread->nstate;
@@ -3589,9 +3595,9 @@ threadRun(void *arg)
35893595
snprintf(logpath, sizeof(logpath), "pgbench_log.%d", main_pid);
35903596
else
35913597
snprintf(logpath, sizeof(logpath), "pgbench_log.%d.%d", main_pid, thread->tid);
3592-
logfile = fopen(logpath, "w");
3598+
thread->logfile = fopen(logpath, "w");
35933599

3594-
if (logfile == NULL)
3600+
if (thread->logfile == NULL)
35953601
{
35963602
fprintf(stderr, "could not open logfile \"%s\": %s\n",
35973603
logpath, strerror(errno));
@@ -3628,7 +3634,7 @@ threadRun(void *arg)
36283634
if (debug)
36293635
fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
36303636
sql_script[st->use_file].name);
3631-
if (!doCustom(thread, st, logfile, &aggs))
3637+
if (!doCustom(thread, st, &aggs))
36323638
remains--; /* I've aborted */
36333639

36343640
if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND)
@@ -3767,7 +3773,7 @@ threadRun(void *arg)
37673773
if (st->con && (FD_ISSET(PQsocket(st->con), &input_mask)
37683774
|| commands[st->state]->type == META_COMMAND))
37693775
{
3770-
if (!doCustom(thread, st, logfile, &aggs))
3776+
if (!doCustom(thread, st, &aggs))
37713777
remains--; /* I've aborted */
37723778
}
37733779

@@ -3871,14 +3877,14 @@ threadRun(void *arg)
38713877
disconnect_all(state, nstate);
38723878
INSTR_TIME_SET_CURRENT(end);
38733879
INSTR_TIME_ACCUM_DIFF(thread->conn_time, end, start);
3874-
if (logfile)
3880+
if (thread->logfile)
38753881
{
38763882
if (agg_interval)
38773883
{
38783884
/* log aggregated but not yet reported transactions */
3879-
doLog(thread, state, logfile, &end, &aggs, false, 0, 0);
3885+
doLog(thread, state, &end, &aggs, false, 0, 0);
38803886
}
3881-
fclose(logfile);
3887+
fclose(thread->logfile);
38823888
}
38833889
return NULL;
38843890
}

0 commit comments

Comments
 (0)