Re: [HACKERS] pgbench tap tests & minor fixes - Mailing list pgsql-hackers

From Nikolay Shaplov
Subject Re: [HACKERS] pgbench tap tests & minor fixes
Date
Msg-id 2118148.KGiOIhm2IX@x200m
Whole thread Raw
In response to Re: [HACKERS] pgbench tap tests & minor fixes  (Fabien COELHO <coelho@cri.ensmp.fr>)
Responses Re: [HACKERS] pgbench tap tests & minor fixes
List 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               {                   /*
detailedstats 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.



pgsql-hackers by date:

Previous
From: Mark Dilger
Date:
Subject: Re: [HACKERS] no test coverage for ALTER FOREIGN DATA WRAPPER name HANDLER ...
Next
From: Stephen Frost
Date:
Subject: Re: [HACKERS] Should pg_current_wal_location() becomepg_current_wal_lsn()