On 2021/09/29 22:11, Fujii Masao wrote:
On 2021/09/24 11:26, Fujii Masao wrote:
On 2021/09/24 7:26, Yugo NAGATA wrote:
That makes sense. Failures of setup connection or initial connection doesn't
seem 'static problems'. I rewrote this description to explain exit status 1
indicates also interal errors and early errors.
Exit status 1 indicates static problems such as invalid command-line options
or internal errors which are supposed to never occur. Early errors that
occur
when starting benchmark such as initial connection failures also exit with
status 1.
LGTM. Barring any objection, I will commit the patch.
I extracted two changes from the patch and pushed (also back-patched) them.
The remainings are the changes of handling of initial connection or
logfile open failures. I agree to push them at least for the master.
But I'm not sure if they should be back-patched. Without these changes,
even when those failures happen, pgbench proceeds the benchmark and
reports the result. But with the changes, pgbench exits immediately in
that case. I'm not sure if there are people who expect this behavior,
but if there are, maybe we should not change it at least at stable branches.
Thought?
The current behavior should be improved, but not a bug.
So I don't think that the patch needs to be back-patched.
Barring any objection, I will push the attached patch to the master.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 0f432767c2..c71dab644c 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -904,10 +904,12 @@ pgbench <optional> <replaceable>options</replaceable>
</optional> <replaceable>d
<para>
A successful run will exit with status 0. Exit status 1 indicates static
- problems such as invalid command-line options. Errors during the run such
- as database errors or problems in the script will result in exit status 2.
- In the latter case, <application>pgbench</application> will print partial
- results.
+ problems such as invalid command-line options or internal errors which
+ are supposed to never occur. Early errors that occur when starting
+ benchmark such as initial connection failures also exit with status 1.
+ Errors during the run such as database errors or problems in the script
+ will result in exit status 2. In the latter case,
+ <application>pgbench</application> will print partial results.
</para>
</refsect1>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d17f69333f..f8331bbb60 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3181,6 +3181,7 @@ advanceConnectionState(TState *thread, CState *st,
StatsData *agg)
if ((st->con = doConnect()) == NULL)
{
+ /* as the bench is already
running, we do not abort the process */
pg_log_error("client %d aborted
while establishing connection", st->id);
st->state = CSTATE_ABORTED;
break;
@@ -4456,7 +4457,10 @@ runInitSteps(const char *initialize_steps)
initPQExpBuffer(&stats);
if ((con = doConnect()) == NULL)
+ {
+ pg_log_fatal("could not create connection for initialization");
exit(1);
+ }
setup_cancel_handler(NULL);
SetCancelConn(con);
@@ -6399,7 +6403,10 @@ main(int argc, char **argv)
/* opening connection... */
con = doConnect();
if (con == NULL)
+ {
+ pg_log_fatal("could not create connection for setup");
exit(1);
+ }
/* report pgbench and server versions */
printVersion(con);
@@ -6625,7 +6632,7 @@ threadRun(void *arg)
if (thread->logfile == NULL)
{
pg_log_fatal("could not open logfile \"%s\": %m",
logpath);
- goto done;
+ exit(1);
}
}
@@ -6650,16 +6657,10 @@ threadRun(void *arg)
{
if ((state[i].con = doConnect()) == NULL)
{
- /*
- * On connection failure, we meet the barrier
here in place of
- * GO before proceeding to the "done" path
which will cleanup,
- * so as to avoid locking the process.
- *
- * It is unclear whether it is worth doing
anything rather
- * than coldly exiting with an error message.
- */
- THREAD_BARRIER_WAIT(&barrier);
- goto done;
+ /* coldly abort on initial connection failure */
+ pg_log_fatal("could not create connection for
client %d",
+ state[i].id);
+ exit(1);
}
}
}
