Re: pgbench tap tests & minor fixes

From: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: pgbench tap tests & minor fixes
Date: 2017-05-09 13:54:03
Message-ID: 2118148.KGiOIhm2IX@x200m
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

В письме от 8 мая 2017 23:17:49 пользователь Fabien COELHO написал:

> > st->cnt -- number of transactions finished successed or failed, right?
>
> Or *skipped*. That is why I changed the declaration comment.

Oh! I now I get the idea... The code became clear to me.
Thanks for the patience...

Meanwhile I have another question. I'd like to understand why it have been
done this way.

Why do you place st->cnt++; inside processXactStats function?

It is called two times, once there is alternative st->cnt++ in the else

if (progress || throttle_delay || latency_limit ||
per_script_stats || use_log)
processXactStats(thread, st, &now, false, agg);
else
{
/* detailed stats are not needed, just count */
thread->stats.cnt++;
st->cnt++;
}

This code would mislead me to conclusion that st->cnt is incremented only on
"else" branch. But actually it is incremented anyway, but you should carefully
read processXactStats function to understand it.

The second time processXactStats is called inside the while loop we've
discussed in previous letters. There is no alternative st->cnt++ here, if we
move st->cnt++ out of processXactStats to the body of the loop, we will be
able to write detailed comment, explaining why exactly we increment it, so I
would be the last person who was confused by this part of the code.

We will have to move thread->stats.cnt++; then. If we decided to move st-
>cnt++;. There would not be great harm, as thread->stats.cnt++; is also done
in the code outside of processXactStats.

So it is just my idea to made the code better. May be there is good reason for
keeping it inside processXactStats, I just can't see. What do you think about
it?

--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2017-05-09 14:00:12 Re: Should pg_current_wal_location() become pg_current_wal_lsn()
Previous Message Mark Dilger 2017-05-09 13:52:18 Re: no test coverage for ALTER FOREIGN DATA WRAPPER name HANDLER ...