Hello Fujii-san,
On Tue, 7 Sep 2021 02:34:17 +0900
Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
> On 2021/07/29 13:23, Yugo NAGATA wrote:
> > Hello,
> >
> > On Fri, 18 Jun 2021 15:58:48 +0200 (CEST)
> > Fabien COELHO <coe...@cri.ensmp.fr> wrote:
> >
> >> Attached Yugo-san patch with some updates discussed in the previous mails,
> >> so as to move things along.
> >
> > I attached the patch rebased to a change due to 856de3b39cf.
>
> + pg_log_fatal("connection for initialization failed");
> + pg_log_fatal("setup connection failed");
> + pg_log_fatal("cannot create connection for
> client %d",
>
> These fatal messages output when doConnect() fails should be a bit more
> consistent each other? For example,
>
> could not create connection for initialization
> could not create connection for setup
> could not create connection for client %d
Ok. I fixed as your suggestion.
> > 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.
>
> While reading the code and docs related to the patch, I found
> these descriptions in pgbench docs. The first description needs to be
> updated? Because even database error (e.g., failure of connection for setup)
> can result in exit status 1 if it happens before the benchmark actually runs.
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.
Regards,
Yugo Nagata
--
Yugo NAGATA <nag...@sraoss.co.jp>
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 433abd954b..8d44399c16 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);
@@ -6553,7 +6560,7 @@ main(int argc, char **argv)
#endif /* ENABLE_THREAD_SAFETY */
for (int j = 0; j < thread->nstate; j++)
- if (thread->state[j].state == CSTATE_ABORTED)
+ if (thread->state[j].state != CSTATE_FINISHED)
exit_code = 2;
/* aggregate thread level stats */
@@ -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);
}
}
}
@@ -6784,7 +6785,7 @@ threadRun(void *arg)
continue;
}
/* must be something wrong */
- pg_log_fatal("%s() failed: %m", SOCKET_WAIT_METHOD);
+ pg_log_error("%s() failed: %m", SOCKET_WAIT_METHOD);
goto done;
}
}
