Horton

Re: pgbench bug candidate: negative "initial connection time"

Yugo Nagata <nagata@sraoss.co.jp>

View thread View on postgr.es
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2021-06-17T09:16:42Z
Lists: pgsql-hackers

Attachments

Hello Fabien,

On Thu, 17 Jun 2021 10:37:05 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

> > ).  Is this acceptable for you?
> 
> I disagree on two counts:
> 
> First, thread[0] should not appear.
> 
> Second, currently the *only* function to change the client state is 
> advanceConnectionState, so it can be checked there and any bug is only 
> there. We had issues before when several functions where doing updates, 
> and it was a mess to understand what was going on. I really want that it 
> stays that way, so I disagree with setting the state to ABORTED from 
> threadRun. Moreover I do not see that it brings a feature, so ISTM that it 
> is not an actual issue not to do it?

Ok. I gave up to change the state in threadRun. Instead, I changed the
condition at the end of bench, which enables to report abortion due to
socket errors.

+@@ -6480,7 +6490,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 */

Does this make sense?

-- 
Yugo NAGATA <nagata@sraoss.co.jp>

Commits

  1. pgbench: Fix typo in comment.

  2. pgbench: Improve error-handling in pgbench.

  3. pgbench: Fix handling of socket errors during benchmark.

  4. pgbench: Correct log level of message output when socket wait method fails.

  5. pgbench: Synchronize client threads.

  6. pgbench: Use common logging API