Re: pgbench tap tests & minor fixes

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
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 15:12:04
Message-ID: alpine.DEB.2.20.1705091641150.29373@lancre
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers


Hello Nikolay,

> 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?

The idea is that it must be counted once. There is a "detailed" statistic
collection fonction which is called when needed (that is the if with
all the different options which imply better statistics), else we just do
the counting part and nothing else.

> [...] 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.

Yep. The increments are just a poor's man (few cycle) stat collection,
when no detailed stats are needed.

> 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.

Hmmm.

> 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?

I understand.

I wanted to have *one* single functions where stats are collected, the
situation before was not too clear about where & when the stats where
collected. There was some pain to avoid useless cycles, so the function
was skipped in some cases.

I've modified the code so that the processXactStats function is always
called, but just does the counting and skip everything else when detailed
stats are not needed. I think it is a good compromise between simplicity
and performance. See attached. Hopefully it will be clearer this way.

--
Fabien.

Attachment Content-Type Size
pgbench-tap-5.patch text/x-diff 27.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2017-05-09 15:40:41 export import bytea from psql
Previous Message Peter Eisentraut 2017-05-09 15:00:53 Re: SUBSCRIPTIONS and pg_upgrade