Thread: review: pgbench progress report improvements

review: pgbench progress report improvements

From
Pavel Stehule
Date:
Hello

* patched with minor warning
* compilable cleanly
* zero impact on PostgreSQL server functionality
* it does what was in proposal
** change 5sec progress as default (instead no progress)
** finalise a rate limit support - fixes a latency calculation
* code is clean
* documentation is included
* there is no voices against this patch and this patch increases a pgbench usability/

I have only one question. When I tested this patch with throttling I got a very similar values of lag.

[pavel@localhost ~]$ /usr/local/pgsql/bin/pgbench -T100 -j4 -c32 postgres -R 60
starting vacuum...end.
progress: 5.0 s, 61.3 tps, 15.796 +- 11.287 ms lat, 0.118 ms lag
progress: 10.0 s, 60.8 tps, 16.527 +- 12.965 ms lat, 0.120 ms lag

[pavel@localhost ~]$ /usr/local/pgsql/bin/pgbench -T100 -j4 -c32 postgres -R 80
starting vacuum...end.
progress: 5.0 s, 78.8 tps, 17.009 +- 11.666 ms lat, 0.163 ms lag
progress: 10.1 s, 74.3 tps, 33.510 +- 55.456 ms lat, 0.092 ms lag

[pavel@localhost ~]$ /usr/local/pgsql/bin/pgbench -T100 -j4 -c32 postgres -R 40
starting vacuum...end.
progress: 5.2 s, 39.4 tps, 13.580 +- 10.283 ms lat, 0.182 ms lag
progress: 10.1 s, 49.3 tps, 13.192 +- 6.772 ms lat, 0.135 ms lag

What is sense, or what is semantic of this value? It is not detailed documented. Should be printed this value in this form on every row? We can print some warning when lag is higher than latency instead? Or we can use this value, but it should be better documented, please.

Regards

Pavel Stehule


some minor issue:

patch warning

make[1]: Leaving directory `/home/pavel/src/postgresql/config'
[pavel@localhost postgresql]$ patch -p1 < pgbench-measurements-v2.patch
(Stripping trailing CRs from patch; use --binary to disable.)
patching file contrib/pgbench/pgbench.c
(Stripping trailing CRs from patch; use --binary to disable.)
patching file doc/src/sgml/pgbench.sgml

Re: review: pgbench progress report improvements

From
Fabien COELHO
Date:
Hello Pavel,

Thanks for your review.

> * patched with minor warning
> * compilable cleanly
> * zero impact on PostgreSQL server functionality
> * it does what was in proposal
> ** change 5sec progress as default (instead no progress)
> ** finalise a rate limit support - fixes a latency calculation

Just a point about the motivation: the rationale for having a continuous 
progress report is that benchmarking is subject to possibly long warmup 
times, and thus a test may have to run for hours so as to be significant. 
I find running a command for hours without any hint about what is going on 
quite annoying.

> * code is clean
> * documentation is included
> * there is no voices against this patch and this patch increases a pgbench
> usability/
>
> I have only one question. When I tested this patch with throttling I got a
> very similar values of lag.

Yep. That is just good!

> What is sense, or what is semantic of this value?

The "lag" measures the stochastic processus health. Actually, it measures 
how far behind schedule the clients are when performing throttled 
transactions. If it was to increase, that would mean that something is 
amiss, possibly not enough client threads or other issues. If it is small, 
then all is well.

> It is not detailed documented.

It is documented in the section about the --rate option, see
http://www.postgresql.org/docs/devel/static/pgbench.html

> Should be printed this value in this form on every row? We can
> print some warning when lag is higher than latency instead?

Hmmm... what is important is when the lag changes values.

Generally one would indeed expect that to be smaller than the latency, but 
that is not really possible when transaction are very fast, say under "-S" 
with read-only queries that hit the memory cache.

Also the problem with printing warnings is that it changes the output 
format, but it seems to me more useful to print the value, so that it can 
be processed automatically and simply.

Also, from a remote client perspective, say a web application, the overall 
latency is the lag plus the transaction latency: you first wait to get 
through the database (lag), and then you can perform your transaction 
(latency).

> Or we can use this value, but it should be better documented, please.

Is the documentation pointed above enough?

-- 
Fabien.



Re: review: pgbench progress report improvements

From
Fabien COELHO
Date:
> * patched with minor warning

> some minor issue:
>
> patch warning
>
> make[1]: Leaving directory `/home/pavel/src/postgresql/config'
> [pavel@localhost postgresql]$ patch -p1 < pgbench-measurements-v2.patch
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file contrib/pgbench/pgbench.c
> (Stripping trailing CRs from patch; use --binary to disable.)
> patching file doc/src/sgml/pgbench.sgml

I cannot reproduce these warnings:
  postgresql> git branch test  postgresql> git checkout test    Switched to branch 'test'  postgresql> patch -p1 <
../pgbench-measurements-v2.patch   patching file contrib/pgbench/pgbench.c    patching file doc/src/sgml/pgbench.sgml
 

Some details:
  postgresql> patch --version    patch 2.6.1 [...]  postgresql> sha1sum ../pgbench-measurements-v2.patch
f095557ceae1409d2339f9d29d332cefa96e2153[...]
 

-- 
Fabien.



Re: review: pgbench progress report improvements

From
Pavel Stehule
Date:
<p dir="ltr"><br /> Dne 12. 9. 2013 17:34 "Fabien COELHO" <<a
href="mailto:coelho@cri.ensmp.fr">coelho@cri.ensmp.fr</a>>napsal(a):<br /> ><br /> ><br /> >> * patched
withminor warning<br /> ><br /> ><br /> >> some minor issue:<br /> >><br /> >> patch warning<br
/>>><br /> >> make[1]: Leaving directory `/home/pavel/src/postgresql/config'<br /> >>
[pavel@localhostpostgresql]$ patch -p1 < pgbench-measurements-v2.patch<br /> >> (Stripping trailing CRs from
patch;use --binary to disable.)<br /> >> patching file contrib/pgbench/pgbench.c<br /> >> (Stripping
trailingCRs from patch; use --binary to disable.)<br /> >> patching file doc/src/sgml/pgbench.sgml<br /> ><br
/>><br /> > I cannot reproduce these warnings:<br /> ><br /> >   postgresql> git branch test<br /> >
 postgresql> git checkout test<br /> >     Switched to branch 'test'<br /> >   postgresql> patch -p1 <
../pgbench-measurements-v2.patch<br/> >     patching file contrib/pgbench/pgbench.c<br /> >     patching file
doc/src/sgml/pgbench.sgml<br/> ><p dir="ltr">it can depends on o.s. I did tests on Fedora 14. and for patching
withoutwarning I had to use dos2unix tool.<p dir="ltr">> Some details:<br /> ><br /> >   postgresql> patch
--version<br/> >     patch 2.6.1 [...]<br /> >   postgresql> sha1sum ../pgbench-measurements-v2.patch<br />
>    f095557ceae1409d2339f9d29d332cefa96e2153 [...]<br /> ><br /> > -- <br /> > Fabien.<br /> 

Re: review: pgbench progress report improvements

From
Fabien COELHO
Date:
Hello,

About patch eols:

>>   postgresql> patch -p1 < ../pgbench-measurements-v2.patch
>>     patching file contrib/pgbench/pgbench.c
>>     patching file doc/src/sgml/pgbench.sgml
>
> it can depends on o.s. I did tests on Fedora 14. and for patching without
> warning I had to use dos2unix tool.

Hmmm. I use a Linux Ubuntu laptop, so generating DOS end of lines is 
unlikely if it is not there at the beginning. Running "dos2unix" on the 
patch file locally does not seem to change anything. So I assume that the 
patch encoding was changed somewhere along the path you used to get it.

-- 
Fabien.



Re: review: pgbench progress report improvements

From
Pavel Stehule
Date:



2013/9/13 Fabien COELHO <coelho@cri.ensmp.fr>

Hello,

About patch eols:


  postgresql> patch -p1 < ../pgbench-measurements-v2.patch
    patching file contrib/pgbench/pgbench.c
    patching file doc/src/sgml/pgbench.sgml

it can depends on o.s. I did tests on Fedora 14. and for patching without
warning I had to use dos2unix tool.

Hmmm. I use a Linux Ubuntu laptop, so generating DOS end of lines is unlikely if it is not there at the beginning. Running "dos2unix" on the patch file locally does not seem to change anything. So I assume that the patch encoding was changed somewhere along the path you used to get it.

It is possible - but, this is only minor issue

Pavel
 

--
Fabien.

Re: review: pgbench progress report improvements

From
Pavel Stehule
Date:



2013/9/12 Fabien COELHO <coelho@cri.ensmp.fr>

Hello Pavel,

Thanks for your review.


* patched with minor warning
* compilable cleanly
* zero impact on PostgreSQL server functionality
* it does what was in proposal
** change 5sec progress as default (instead no progress)
** finalise a rate limit support - fixes a latency calculation

Just a point about the motivation: the rationale for having a continuous progress report is that benchmarking is subject to possibly long warmup times, and thus a test may have to run for hours so as to be significant. I find running a command for hours without any hint about what is going on quite annoying.


* code is clean
* documentation is included
* there is no voices against this patch and this patch increases a pgbench
usability/

I have only one question. When I tested this patch with throttling I got a
very similar values of lag.

Yep. That is just good!


What is sense, or what is semantic of this value?

The "lag" measures the stochastic processus health. Actually, it measures how far behind schedule the clients are when performing throttled transactions. If it was to increase, that would mean that something is amiss, possibly not enough client threads or other issues. If it is small, then all is well.


It is not detailed documented.

It is documented in the section about the --rate option, see
http://www.postgresql.org/docs/devel/static/pgbench.html

ok, I see it now.

So this patch is ready for commit

Regards

Pavel
 


Should be printed this value in this form on every row? We can
print some warning when lag is higher than latency instead?

Hmmm... what is important is when the lag changes values.

Generally one would indeed expect that to be smaller than the latency, but that is not really possible when transaction are very fast, say under "-S" with read-only queries that hit the memory cache.

Also the problem with printing warnings is that it changes the output format, but it seems to me more useful to print the value, so that it can be processed automatically and simply.

Also, from a remote client perspective, say a web application, the overall latency is the lag plus the transaction latency: you first wait to get through the database (lag), and then you can perform your transaction (latency).


Or we can use this value, but it should be better documented, please.

Is the documentation pointed above enough?

--
Fabien.