Hello Aleksander,

Thanks for the look at the patch.

time pgbench -T 5 -R 0.1 -P 1 -c 2 -j 2

On my laptop this command executes 25 seconds instead of 5.
I'm pretty sure it IS a bug. Probably a minor one though.

Sure.

[...] you should probably write:

if(someint > 0)

Ok.

if(somebool == TRUE)

I like "if (somebool)", the "== TRUE" looks like a tautology, and the short version is also the current practice in the project.

Also I suggest to introduce a few new boolean variables with meaningful
names instead of writing all these long expressions right inside of
if( ... ).

I agree about the lisibility, but there are semantics issues to consider:

    if (short-A && pretty-long-B)

If short-A is false then pretty-long-B is not evaluated, which is a win because it also costs, I try to order conditions... If I move pretty-long-B before then the cost is always paid. Now I could write:

    if (short-A) {
      bool b = pretty-long-B;
      if (b) {
        ...

But this looks contrived and people would raise other questions about such
a strange construct for implementing && in 3 lines, 2 if and 1 variable...

As a side note I noticed that pgbench.c is not pgindent'ed. Since you
are modifying this file anyway probably you cold solve this issue too?
As a separate patch perhaps.

As Robert said, not the purpose of this patch.

Attached is a v3 which test integers more logically. I'm a lazy programmer who tends to minimize the number of key strokes.

--
Fabien.
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 66cfdc9..82707ed 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1362,6 +1362,12 @@ top:
 		thread->throttle_trigger += wait;
 		st->txn_scheduled = thread->throttle_trigger;
 
+		/* stop client if next transaction is beyond pgbench end of execution */
+		if (duration > 0 &&
+			st->txn_scheduled >
+			INSTR_TIME_GET_MICROSEC(thread->start_time) + (int64) 1000000 * duration)
+			return false;
+
 		/*
 		 * If this --latency-limit is used, and this slot is already late so
 		 * that the transaction will miss the latency limit even if it
@@ -3724,7 +3730,10 @@ threadRun(void *arg)
 		}
 	}
 
-	while (remains > 0)
+	while (remains > 0 ||
+		   /* thread zero keeps on printing progress report if any */
+		   (progress > 0 && thread->tid == 0 && duration > 0 &&
+			next_report <= thread_start + (int64) duration * 1000000))
 	{
 		fd_set		input_mask;
 		int			maxsock;	/* max socket number to be waited */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to