Thread: review: pgbench - aggregation of info written into log

review: pgbench - aggregation of info written into log

From
Pavel Stehule
Date:
Hello

I did review of pgbench patch
http://archives.postgresql.org/pgsql-hackers/2012-09/msg00737.php

* this patch is cleanly patched

* I had a problem with doc

make[1]: Entering directory `/home/pavel/src/postgresql/doc/src/sgml'
openjade  -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D .
-c /usr/share/sgml/docbook/dsssl-stylesheets/catalog -d stylesheet.dsl
-t sgml -i output-html -V html-index postgres.sgml
openjade:pgbench.sgml:767:8:E: document type does not allow element
"TITLE" here; missing one of "ABSTRACT", "AUTHORBLURB", "MSGSET",
"CALLOUTLIST", "ITEMIZEDLIST", "ORDEREDLIST", "SEGMENTEDLIST",
"VARIABLELIST", "CAUTION", "IMPORTANT", "NOTE", "TIP", "WARNING",
"FORMALPARA", "BLOCKQUOTE", "EQUATION", "EXAMPLE", "FIGURE", "TABLE",
"PROCEDURE", "SIDEBAR", "QANDASET", "REFSECT3" start-tag
make[1]: *** [HTML.index] Error 1
make[1]: *** Deleting file `HTML.index'
make[1]: Leaving directory `/home/pavel/src/postgresql/doc/src/sgml

* pgbench is compiled without warnings

* there is a few issues in source code

+            
+            /* should we aggregate the results or not? */
+            if (use_log_agg)
+            {
+                
+                /* are we still in the same interval? if yes, accumulate the
+                 * values (print them otherwise) */
+                if (agg->start_time + use_log_agg >= INSTR_TIME_GET_DOUBLE(now))
+                {
+                    

* a real time range for aggregation is dynamic (pgbench is not
realtime application), so I am not sure if following constraint has
sense
+    if ((duration > 0) && (use_log_agg > 0) && (duration % use_log_agg != 0)) {
+        fprintf(stderr, "duration (%d) must be a multiple of aggregation
interval (%d)\n", duration, use_log_agg);
+        exit(1);
+    }

* it doesn't flush last aggregated data (it is mentioned by Tomas:
"Also, I've realized the last interval may not be logged at all - I'll
take a look into this in the next version of the patch."). And it can
be significant for higher values of "use_log_agg"

* a name of variable "use_log_agg" is not good - maybe "log_agg_interval" ?

Regards

Pavel Stehule



Re: review: pgbench - aggregation of info written into log

From
Tomas Vondra
Date:
Hi,

attached is a v4 of the patch. There are not many changes, mostly some
simple tidying up, except for handling the Windows.

After a bit more investigation, it seems to me that we can't really get
the same behavior as in other systems - basically the timestamp is
unavailable so we can't log the interval start. So it seems to me the
best we can do is to disable this option on Windows (and this is done in
the patch). So when you try to use "--aggregate-interval" on Win it will
complain it's an unknown option.

Now that I think of it, there might be a better solution that would not
mimic the Linux/Unix behaviour exactly - we do have elapsed time since
the start of the benchmark, so maybe we should use this instead of the
timestamp? I mean on systems with reasonable gettimeofday we'd get

1345828501 5601 1542744 483552416 61 2573
1345828503 7884 1979812 565806736 60 1479
1345828505 7208 1979422 567277552 59 1391
1345828507 7685 1980268 569784714 60 1398
1345828509 7073 1979779 573489941 236 1411

but on Windows we'd get

0 5601 1542744 483552416 61 2573
2 7884 1979812 565806736 60 1479
4 7208 1979422 567277552 59 1391
6 7685 1980268 569784714 60 1398
8 7073 1979779 573489941 236 1411

i.e. the first column is "seconds since start of the test". Hmmmm, and
maybe we could call 'gettimeofday' at the beginning, to get the
timestamp of the test start (AFAIK the issue on Windows is the
resolution, but it should be enough). Then we could add it up with the
elapsed time and we'd get the same output as on Linux.

And maybe this could be done in regular pgbench execution too? But I
really need help from someone who knows Windows and can test this.

Comments regarding Pavel's reviews are below:

On 2.10.2012 20:08, Pavel Stehule wrote:
> Hello
>
> I did review of pgbench patch
> http://archives.postgresql.org/pgsql-hackers/2012-09/msg00737.php
>
> * this patch is cleanly patched
>
> * I had a problem with doc
>
> make[1]: Entering directory `/home/pavel/src/postgresql/doc/src/sgml'
> openjade  -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D .
> -c /usr/share/sgml/docbook/dsssl-stylesheets/catalog -d stylesheet.dsl
> -t sgml -i output-html -V html-index postgres.sgml
> openjade:pgbench.sgml:767:8:E: document type does not allow element
> "TITLE" here; missing one of "ABSTRACT", "AUTHORBLURB", "MSGSET",
> "CALLOUTLIST", "ITEMIZEDLIST", "ORDEREDLIST", "SEGMENTEDLIST",
> "VARIABLELIST", "CAUTION", "IMPORTANT", "NOTE", "TIP", "WARNING",
> "FORMALPARA", "BLOCKQUOTE", "EQUATION", "EXAMPLE", "FIGURE", "TABLE",
> "PROCEDURE", "SIDEBAR", "QANDASET", "REFSECT3" start-tag
> make[1]: *** [HTML.index] Error 1
> make[1]: *** Deleting file `HTML.index'
> make[1]: Leaving directory `/home/pavel/src/postgresql/doc/src/sgml

Hmmm, I've never got the docs to build at all, all I do get is a heap of
some SGML/DocBook related issues. Can you investigate a bit more where's
the issue? I don't remember messing with the docs in a way that might
cause this ... mostly copy'n'paste edits.

> * pgbench is compiled without warnings
>
> * there is a few issues in source code
>
> +
> +            /* should we aggregate the results or not? */
> +            if (use_log_agg)
> +            {
> +
> +                /* are we still in the same interval? if yes, accumulate the
> +                 * values (print them otherwise) */
> +                if (agg->start_time + use_log_agg >= INSTR_TIME_GET_DOUBLE(now))
> +                {
> +

Errr, so what are the issues here?

>
> * a real time range for aggregation is dynamic (pgbench is not
> realtime application), so I am not sure if following constraint has
> sense
>
>  +    if ((duration > 0) && (use_log_agg > 0) && (duration % use_log_agg != 0)) {
> +        fprintf(stderr, "duration (%d) must be a multiple of aggregation
> interval (%d)\n", duration, use_log_agg);
> +        exit(1);
> +    }

I'm not sure what might be the issue here either. If the test duration
is set (using "-T" option), then this kicks in and says that the
duration should be a multiple of aggregation interval. Seems like a
sensible assumption to me. Or do you think this is check should be removed?

> * it doesn't flush last aggregated data (it is mentioned by Tomas:
> "Also, I've realized the last interval may not be logged at all - I'll
> take a look into this in the next version of the patch."). And it can
> be significant for higher values of "use_log_agg"

Yes, and I'm still not sure how to fix this in practice. But I do have
this on my TODO.

>
> * a name of variable "use_log_agg" is not good - maybe "log_agg_interval" ?

I've changed it to agg_interval.

regards
Tomas

Attachment

Re: review: pgbench - aggregation of info written into log

From
Pavel Stehule
Date:
Hello

2012/11/12 Tomas Vondra <tv@fuzzy.cz>:
> Hi,
>
> attached is a v4 of the patch. There are not many changes, mostly some
> simple tidying up, except for handling the Windows.
>
> After a bit more investigation, it seems to me that we can't really get
> the same behavior as in other systems - basically the timestamp is
> unavailable so we can't log the interval start. So it seems to me the
> best we can do is to disable this option on Windows (and this is done in
> the patch). So when you try to use "--aggregate-interval" on Win it will
> complain it's an unknown option.
>
> Now that I think of it, there might be a better solution that would not
> mimic the Linux/Unix behaviour exactly - we do have elapsed time since
> the start of the benchmark, so maybe we should use this instead of the
> timestamp? I mean on systems with reasonable gettimeofday we'd get
>
> 1345828501 5601 1542744 483552416 61 2573
> 1345828503 7884 1979812 565806736 60 1479
> 1345828505 7208 1979422 567277552 59 1391
> 1345828507 7685 1980268 569784714 60 1398
> 1345828509 7073 1979779 573489941 236 1411
>
> but on Windows we'd get
>
> 0 5601 1542744 483552416 61 2573
> 2 7884 1979812 565806736 60 1479
> 4 7208 1979422 567277552 59 1391
> 6 7685 1980268 569784714 60 1398
> 8 7073 1979779 573489941 236 1411
>
> i.e. the first column is "seconds since start of the test". Hmmmm, and
> maybe we could call 'gettimeofday' at the beginning, to get the
> timestamp of the test start (AFAIK the issue on Windows is the
> resolution, but it should be enough). Then we could add it up with the
> elapsed time and we'd get the same output as on Linux.
>
> And maybe this could be done in regular pgbench execution too? But I
> really need help from someone who knows Windows and can test this.
>
> Comments regarding Pavel's reviews are below:
>
> On 2.10.2012 20:08, Pavel Stehule wrote:
>> Hello
>>
>> I did review of pgbench patch
>> http://archives.postgresql.org/pgsql-hackers/2012-09/msg00737.php
>>
>> * this patch is cleanly patched
>>
>> * I had a problem with doc
>>
>> make[1]: Entering directory `/home/pavel/src/postgresql/doc/src/sgml'
>> openjade  -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D .
>> -c /usr/share/sgml/docbook/dsssl-stylesheets/catalog -d stylesheet.dsl
>> -t sgml -i output-html -V html-index postgres.sgml
>> openjade:pgbench.sgml:767:8:E: document type does not allow element
>> "TITLE" here; missing one of "ABSTRACT", "AUTHORBLURB", "MSGSET",
>> "CALLOUTLIST", "ITEMIZEDLIST", "ORDEREDLIST", "SEGMENTEDLIST",
>> "VARIABLELIST", "CAUTION", "IMPORTANT", "NOTE", "TIP", "WARNING",
>> "FORMALPARA", "BLOCKQUOTE", "EQUATION", "EXAMPLE", "FIGURE", "TABLE",
>> "PROCEDURE", "SIDEBAR", "QANDASET", "REFSECT3" start-tag
>> make[1]: *** [HTML.index] Error 1
>> make[1]: *** Deleting file `HTML.index'
>> make[1]: Leaving directory `/home/pavel/src/postgresql/doc/src/sgml
>
> Hmmm, I've never got the docs to build at all, all I do get is a heap of
> some SGML/DocBook related issues. Can you investigate a bit more where's
> the issue? I don't remember messing with the docs in a way that might
> cause this ... mostly copy'n'paste edits.
>
>> * pgbench is compiled without warnings
>>
>> * there is a few issues in source code
>>
>> +
>> +                     /* should we aggregate the results or not? */
>> +                     if (use_log_agg)
>> +                     {
>> +
>> +                             /* are we still in the same interval? if yes, accumulate the
>> +                              * values (print them otherwise) */
>> +                             if (agg->start_time + use_log_agg >= INSTR_TIME_GET_DOUBLE(now))
>> +                             {
>> +
>
> Errr, so what are the issues here?

using a use_log_agg variable - bad name for variable - it is fixed

>
>>
>> * a real time range for aggregation is dynamic (pgbench is not
>> realtime application), so I am not sure if following constraint has
>> sense
>>
>>  +    if ((duration > 0) && (use_log_agg > 0) && (duration % use_log_agg != 0)) {
>> +             fprintf(stderr, "duration (%d) must be a multiple of aggregation
>> interval (%d)\n", duration, use_log_agg);
>> +             exit(1);
>> +     }
>
> I'm not sure what might be the issue here either. If the test duration
> is set (using "-T" option), then this kicks in and says that the
> duration should be a multiple of aggregation interval. Seems like a
> sensible assumption to me. Or do you think this is check should be removed?
>
>> * it doesn't flush last aggregated data (it is mentioned by Tomas:
>> "Also, I've realized the last interval may not be logged at all - I'll
>> take a look into this in the next version of the patch."). And it can
>> be significant for higher values of "use_log_agg"
>
> Yes, and I'm still not sure how to fix this in practice. But I do have
> this on my TODO.
>
>>
>> * a name of variable "use_log_agg" is not good - maybe "log_agg_interval" ?
>
> I've changed it to agg_interval.

ok

issues:

* empty lines with invisible chars (tabs) + and sometimes empty lines
after and before {}

* adjustment of start_time

+                                               * the desired interval */
+                                               while (agg->start_time
+ agg_interval < INSTR_TIME_GET_DOUBLE(now))
+
agg->start_time = agg->start_time + agg_interval;

can "skip" one interval - so when transaction time will be larger or
similar to agg_interval - then results can be strange. We have to know
real length of interval

Regards

Pavel

>
> regards
> Tomas
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



Re: review: pgbench - aggregation of info written into log

From
Andres Freund
Date:
Hi Tomas,

On 2012-11-27 14:55:59 +0100, Pavel Stehule wrote:
> > attached is a v4 of the patch. There are not many changes, mostly some
> > simple tidying up, except for handling the Windows.

After a quick look I am not sure what all the talk about windows is
about? instr_time.h seems to provide all you need, even for windows? The
only issue of gettimeofday() for windows seems to be that it is that its
not all that fast an not too high precision, but that shouldn't be a
problem in this case?

Could you expand a bit on the problems?

> >> * I had a problem with doc

The current patch has conflict markers in the sgml source, there seems
to have been some unresolved merge. Maybe that's all that causes the
errors?

Whats your problem with setting up the doc toolchain?

> issues:
>
> * empty lines with invisible chars (tabs) + and sometimes empty lines
> after and before {}
>
> * adjustment of start_time
>
> +                                               * the desired interval */
> +                                               while (agg->start_time
> + agg_interval < INSTR_TIME_GET_DOUBLE(now))
> +
> agg->start_time = agg->start_time + agg_interval;
>
> can "skip" one interval - so when transaction time will be larger or
> similar to agg_interval - then results can be strange. We have to know
> real length of interval

Could you post a patch that adresses these issues?


Greetings,

Andres Freund

--Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



Re: review: pgbench - aggregation of info written into log

From
Tomas Vondra
Date:
On 8.12.2012 16:33, Andres Freund wrote:
> Hi Tomas,
> 
> On 2012-11-27 14:55:59 +0100, Pavel Stehule wrote:
>>> attached is a v4 of the patch. There are not many changes, mostly some
>>> simple tidying up, except for handling the Windows.
> 
> After a quick look I am not sure what all the talk about windows is
> about? instr_time.h seems to provide all you need, even for windows? The
> only issue of gettimeofday() for windows seems to be that it is that its
> not all that fast an not too high precision, but that shouldn't be a
> problem in this case?
> 
> Could you expand a bit on the problems?

Well, in the current pgbench.c, there's this piece of code

#ifndef WIN32   /* This is more than we really ought to know about instr_time */   fprintf(logfile, "%d %d %.0f %d %ld
%ld\n",          st->id, st->cnt, usec, st->use_file,           (long) now.tv_sec, (long) now.tv_usec);
 
#else   /* On Windows, instr_time doesn't provide a timestamp anyway */   fprintf(logfile, "%d %d %.0f %d 0 0\n",
st->id,st->cnt, usec, st->use_file);
 
#endif

and the Windows-related discussion in this patch is about the same issue.

As I understand it the problem seems to be that INSTR_TIME_SET_CURRENT
does not provide the timestamp at all.

I was thinking about improving this by combining gettimeofday and
INSTR_TIME, but I have no Windows box to test it on :-(

> 
>>>> * I had a problem with doc
> 
> The current patch has conflict markers in the sgml source, there seems
> to have been some unresolved merge. Maybe that's all that causes the
> errors?

Ah, I see. Probably my fault, I'll fix that.

> Whats your problem with setting up the doc toolchain?
> 
>> issues:
>>
>> * empty lines with invisible chars (tabs) + and sometimes empty lines
>> after and before {}
>>
>> * adjustment of start_time
>>
>> +                                               * the desired interval */
>> +                                               while (agg->start_time
>> + agg_interval < INSTR_TIME_GET_DOUBLE(now))
>> +
>> agg->start_time = agg->start_time + agg_interval;
>>
>> can "skip" one interval - so when transaction time will be larger or
>> similar to agg_interval - then results can be strange. We have to know
>> real length of interval
> 
> Could you post a patch that adresses these issues?

Yes, I'll work on it today/tomorrow and I'll send an improved patch.

Tomas



Re: review: pgbench - aggregation of info written into log

From
Tomas Vondra
Date:
Hi,

attached is a v5 of this patch. Details below:

On 8.12.2012 16:33, Andres Freund wrote:
> Hi Tomas,
>
> On 2012-11-27 14:55:59 +0100, Pavel Stehule wrote:
>>> attached is a v4 of the patch. There are not many changes, mostly some
>>> simple tidying up, except for handling the Windows.
>
> After a quick look I am not sure what all the talk about windows is
> about? instr_time.h seems to provide all you need, even for windows? The
> only issue of gettimeofday() for windows seems to be that it is that its
> not all that fast an not too high precision, but that shouldn't be a
> problem in this case?
>
> Could you expand a bit on the problems?

As explained in the previous message, this is an existing problem with
unavailable timestamp. I'm not very fond of adding Linux-only features,
but fixing that was not the goal of this patch.

>>>> * I had a problem with doc
>
> The current patch has conflict markers in the sgml source, there seems
> to have been some unresolved merge. Maybe that's all that causes the
> errors?
>
> Whats your problem with setting up the doc toolchain?

Yeah, my fault because of incomplete merge. But the real culprit was a
missing refsect2. Fixed.

>
>> issues:
>>
>> * empty lines with invisible chars (tabs) + and sometimes empty lines
>> after and before {}

Fixed (I've removed the lines etc.)

>>
>> * adjustment of start_time
>>
>> +                                               * the desired interval */
>> +                                               while (agg->start_time
>> + agg_interval < INSTR_TIME_GET_DOUBLE(now))
>> +
>> agg->start_time = agg->start_time + agg_interval;
>>
>> can "skip" one interval - so when transaction time will be larger or
>> similar to agg_interval - then results can be strange. We have to know
>> real length of interval
>
> Could you post a patch that adresses these issues?

So, in the end I've rewritten the section advancing the start_time. Now
it works so that when skipping an interval (because of a very long
transaction), it will print lines even for those "empty" intervals.

So for example with a transaction file containing a single query

   SELECT pg_sleep(1.5);

and an interval length of 1 second, you'll get something like this:

1355009677 0 0 0 0 0
1355009678 1 1501028 2253085056784 1501028 1501028
1355009679 0 0 0 0 0
1355009680 1 1500790 2252370624100 1500790 1500790
1355009681 1 1500723 2252169522729 1500723 1500723
1355009682 0 0 0 0 0
1355009683 1 1500719 2252157516961 1500719 1500719
1355009684 1 1500703 2252109494209 1500703 1500703
1355009685 0 0 0 0 0

which is IMHO the best way to deal with this.

I've fixed several minor issues, added a few comments.

regards
Tomas

Attachment

Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
Hi,

I'm looking into this as a committer.  It seems that this is the
newest patch and the reviewer(Pavel) stated that it is ready for
commit. However, I noticed that this patch adds a Linux/UNIX only
feature(not available on Windows). So I would like to ask cores if
this is ok or not.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

> Hi,
> 
> attached is a v5 of this patch. Details below:
> 
> On 8.12.2012 16:33, Andres Freund wrote:
>> Hi Tomas,
>> 
>> On 2012-11-27 14:55:59 +0100, Pavel Stehule wrote:
>>>> attached is a v4 of the patch. There are not many changes, mostly some
>>>> simple tidying up, except for handling the Windows.
>> 
>> After a quick look I am not sure what all the talk about windows is
>> about? instr_time.h seems to provide all you need, even for windows? The
>> only issue of gettimeofday() for windows seems to be that it is that its
>> not all that fast an not too high precision, but that shouldn't be a
>> problem in this case?
>> 
>> Could you expand a bit on the problems?
> 
> As explained in the previous message, this is an existing problem with
> unavailable timestamp. I'm not very fond of adding Linux-only features,
> but fixing that was not the goal of this patch.
> 
>>>>> * I had a problem with doc
>> 
>> The current patch has conflict markers in the sgml source, there seems
>> to have been some unresolved merge. Maybe that's all that causes the
>> errors?
>> 
>> Whats your problem with setting up the doc toolchain?
> 
> Yeah, my fault because of incomplete merge. But the real culprit was a
> missing refsect2. Fixed.
> 
>> 
>>> issues:
>>>
>>> * empty lines with invisible chars (tabs) + and sometimes empty lines
>>> after and before {}
> 
> Fixed (I've removed the lines etc.)
> 
>>>
>>> * adjustment of start_time
>>>
>>> +                                               * the desired interval */
>>> +                                               while (agg->start_time
>>> + agg_interval < INSTR_TIME_GET_DOUBLE(now))
>>> +
>>> agg->start_time = agg->start_time + agg_interval;
>>>
>>> can "skip" one interval - so when transaction time will be larger or
>>> similar to agg_interval - then results can be strange. We have to know
>>> real length of interval
>> 
>> Could you post a patch that adresses these issues?
> 
> So, in the end I've rewritten the section advancing the start_time. Now
> it works so that when skipping an interval (because of a very long
> transaction), it will print lines even for those "empty" intervals.
> 
> So for example with a transaction file containing a single query
> 
>    SELECT pg_sleep(1.5);
> 
> and an interval length of 1 second, you'll get something like this:
> 
> 1355009677 0 0 0 0 0
> 1355009678 1 1501028 2253085056784 1501028 1501028
> 1355009679 0 0 0 0 0
> 1355009680 1 1500790 2252370624100 1500790 1500790
> 1355009681 1 1500723 2252169522729 1500723 1500723
> 1355009682 0 0 0 0 0
> 1355009683 1 1500719 2252157516961 1500719 1500719
> 1355009684 1 1500703 2252109494209 1500703 1500703
> 1355009685 0 0 0 0 0
> 
> which is IMHO the best way to deal with this.
> 
> I've fixed several minor issues, added a few comments.
> 
> regards
> Tomas



Re: review: pgbench - aggregation of info written into log

From
Andrew Dunstan
Date:
On 01/16/2013 05:59 PM, Tatsuo Ishii wrote:
> Hi,
>
> I'm looking into this as a committer.  It seems that this is the
> newest patch and the reviewer(Pavel) stated that it is ready for
> commit. However, I noticed that this patch adds a Linux/UNIX only
> feature(not available on Windows). So I would like to ask cores if
> this is ok or not.

I haven't been following the thread, but if the complaint is that 
Windows doesn't have accurate high-resolution timers, which is what it 
kinda looks like from the rest of your message, then it's not true. 
Every version since Windows2000 has had 
QueryPerformanceCounter()/QueryPerformanceFrequency(). And we use it: 
see src/include/portability/instr_time.h

If that's not the problem, then can someone please point me at the 
message that sets the problem out clearly, or else just recap it?

cheers

andrew




Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
>> I'm looking into this as a committer.  It seems that this is the
>> newest patch and the reviewer(Pavel) stated that it is ready for
>> commit. However, I noticed that this patch adds a Linux/UNIX only
>> feature(not available on Windows). So I would like to ask cores if
>> this is ok or not.
> 
> I haven't been following the thread, but if the complaint is that
> Windows doesn't have accurate high-resolution timers, which is what it
> kinda looks like from the rest of your message, then it's not
> true. Every version since Windows2000 has had
> QueryPerformanceCounter()/QueryPerformanceFrequency(). And we use it:
> see src/include/portability/instr_time.h

In my understanding the problem is not related to resolution.

> If that's not the problem, then can someone please point me at the
> message that sets the problem out clearly, or else just recap it?

It seems instr_time.h on Windows simply does not provide current
timestamp. From pgbench.c:
    /*     * if transaction finished, record the time it took in the log     */    if (logfile && commands[st->state +
1]== NULL)    {        instr_time    now;        instr_time    diff;        double        usec;
 
        INSTR_TIME_SET_CURRENT(now);        diff = now;        INSTR_TIME_SUBTRACT(diff, st->txn_begin);        usec =
(double)INSTR_TIME_GET_MICROSEC(diff);
 

#ifndef WIN32        /* This is more than we really ought to know about instr_time */        fprintf(logfile, "%d %d
%.0f%d %ld %ld\n",                st->id, st->cnt, usec, st->use_file,                (long) now.tv_sec, (long)
now.tv_usec);
#else        /* On Windows, instr_time doesn't provide a timestamp anyway */        fprintf(logfile, "%d %d %.0f %d 0
0\n",               st->id, st->cnt, usec, st->use_file);
 
#endif    }
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp



Re: review: pgbench - aggregation of info written into log

From
Andrew Dunstan
Date:
On 01/16/2013 06:48 PM, Tatsuo Ishii wrote:
>>> I'm looking into this as a committer.  It seems that this is the
>>> newest patch and the reviewer(Pavel) stated that it is ready for
>>> commit. However, I noticed that this patch adds a Linux/UNIX only
>>> feature(not available on Windows). So I would like to ask cores if
>>> this is ok or not.
>> I haven't been following the thread, but if the complaint is that
>> Windows doesn't have accurate high-resolution timers, which is what it
>> kinda looks like from the rest of your message, then it's not
>> true. Every version since Windows2000 has had
>> QueryPerformanceCounter()/QueryPerformanceFrequency(). And we use it:
>> see src/include/portability/instr_time.h
> In my understanding the problem is not related to resolution.
>
>> If that's not the problem, then can someone please point me at the
>> message that sets the problem out clearly, or else just recap it?
> It seems instr_time.h on Windows simply does not provide current
> timestamp. From pgbench.c:
>
>         /*
>          * if transaction finished, record the time it took in the log
>          */
>         if (logfile && commands[st->state + 1] == NULL)
>         {
>             instr_time    now;
>             instr_time    diff;
>             double        usec;
>
>             INSTR_TIME_SET_CURRENT(now);
>             diff = now;
>             INSTR_TIME_SUBTRACT(diff, st->txn_begin);
>             usec = (double) INSTR_TIME_GET_MICROSEC(diff);
>
> #ifndef WIN32
>             /* This is more than we really ought to know about instr_time */
>             fprintf(logfile, "%d %d %.0f %d %ld %ld\n",
>                     st->id, st->cnt, usec, st->use_file,
>                     (long) now.tv_sec, (long) now.tv_usec);
> #else
>             /* On Windows, instr_time doesn't provide a timestamp anyway */
>             fprintf(logfile, "%d %d %.0f %d 0 0\n",
>                     st->id, st->cnt, usec, st->use_file);
> #endif
>         }


This might be way more than we want to do, but there is an article that 
describes some techniques for doing what seems to be missing (AIUI):

<http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>

cheers

andrew




Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
>> It seems instr_time.h on Windows simply does not provide current
>> timestamp. From pgbench.c:
>>
>>         /*
>>          * if transaction finished, record the time it took in the log
>>          */
>>         if (logfile && commands[st->state + 1] == NULL)
>>         {
>>             instr_time    now;
>>             instr_time    diff;
>>             double        usec;
>>
>>             INSTR_TIME_SET_CURRENT(now);
>>             diff = now;
>>             INSTR_TIME_SUBTRACT(diff, st->txn_begin);
>>             usec = (double) INSTR_TIME_GET_MICROSEC(diff);
>>
>> #ifndef WIN32
>>             /* This is more than we really ought to know about instr_time */
>>             fprintf(logfile, "%d %d %.0f %d %ld %ld\n",
>>                     st->id, st->cnt, usec, st->use_file,
>>                     (long) now.tv_sec, (long) now.tv_usec);
>> #else
>>             /* On Windows, instr_time doesn't provide a timestamp anyway */
>>             fprintf(logfile, "%d %d %.0f %d 0 0\n",
>>                     st->id, st->cnt, usec, st->use_file);
>> #endif
>>         }
> 
> 
> This might be way more than we want to do, but there is an article
> that describes some techniques for doing what seems to be missing
> (AIUI):
> 
> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>

Even this would be doable, I'm afraid it may not fit in 9.3 if we
think about the current status of CF. So our choice would be:

1) Postpone the patch to 9.4

2) Commit the patch in 9.3 without Windows support

I personally am ok with #2. We traditionally avoid particular paltform
specific features on PostgreSQL.  However I think the policiy could be
losen for contrib staffs. Also pgbench is just a client program. We
could always use pgbench on UNIX/Linux if we truely need the feature.

What do you think?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp



Re: review: pgbench - aggregation of info written into log

From
Andrew Dunstan
Date:
On 01/16/2013 08:05 PM, Tatsuo Ishii wrote:
>>> It seems instr_time.h on Windows simply does not provide current
>>> timestamp. From pgbench.c:
>>>
>>>         /*
>>>          * if transaction finished, record the time it took in the log
>>>          */
>>>         if (logfile && commands[st->state + 1] == NULL)
>>>         {
>>>             instr_time    now;
>>>             instr_time    diff;
>>>             double        usec;
>>>
>>>             INSTR_TIME_SET_CURRENT(now);
>>>             diff = now;
>>>             INSTR_TIME_SUBTRACT(diff, st->txn_begin);
>>>             usec = (double) INSTR_TIME_GET_MICROSEC(diff);
>>>
>>> #ifndef WIN32
>>>             /* This is more than we really ought to know about instr_time */
>>>             fprintf(logfile, "%d %d %.0f %d %ld %ld\n",
>>>                     st->id, st->cnt, usec, st->use_file,
>>>                     (long) now.tv_sec, (long) now.tv_usec);
>>> #else
>>>             /* On Windows, instr_time doesn't provide a timestamp anyway */
>>>             fprintf(logfile, "%d %d %.0f %d 0 0\n",
>>>                     st->id, st->cnt, usec, st->use_file);
>>> #endif
>>>         }
>>
>> This might be way more than we want to do, but there is an article
>> that describes some techniques for doing what seems to be missing
>> (AIUI):
>>
>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
> Even this would be doable, I'm afraid it may not fit in 9.3 if we
> think about the current status of CF. So our choice would be:
>
> 1) Postpone the patch to 9.4
>
> 2) Commit the patch in 9.3 without Windows support
>
> I personally am ok with #2. We traditionally avoid particular paltform
> specific features on PostgreSQL.  However I think the policiy could be
> losen for contrib staffs. Also pgbench is just a client program. We
> could always use pgbench on UNIX/Linux if we truely need the feature.
>
> What do you think?

Fair enough, I was just trying to point out alternatives. We have 
committed platform-specific features before now. I hope it doesn't just 
get left like this, though.

cheers

andrew




Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
>>> This might be way more than we want to do, but there is an article
>>> that describes some techniques for doing what seems to be missing
>>> (AIUI):
>>>
>>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
>> Even this would be doable, I'm afraid it may not fit in 9.3 if we
>> think about the current status of CF. So our choice would be:
>>
>> 1) Postpone the patch to 9.4
>>
>> 2) Commit the patch in 9.3 without Windows support
>>
>> I personally am ok with #2. We traditionally avoid particular paltform
>> specific features on PostgreSQL.  However I think the policiy could be
>> losen for contrib staffs. Also pgbench is just a client program. We
>> could always use pgbench on UNIX/Linux if we truely need the feature.
>>
>> What do you think?
> 
> Fair enough, I was just trying to point out alternatives. We have
> committed platform-specific features before now. I hope it doesn't
> just get left like this, though.

Yeah, I hope someone pick this up and propose as a TODO item. In the
mean time, I'm going to commit the patch without Windows support
unless there's objection.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp



Re: review: pgbench - aggregation of info written into log

From
Magnus Hagander
Date:
On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>>> This might be way more than we want to do, but there is an article
>>>> that describes some techniques for doing what seems to be missing
>>>> (AIUI):
>>>>
>>>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
>>> Even this would be doable, I'm afraid it may not fit in 9.3 if we
>>> think about the current status of CF. So our choice would be:
>>>
>>> 1) Postpone the patch to 9.4
>>>
>>> 2) Commit the patch in 9.3 without Windows support
>>>
>>> I personally am ok with #2. We traditionally avoid particular paltform
>>> specific features on PostgreSQL.  However I think the policiy could be
>>> losen for contrib staffs. Also pgbench is just a client program. We
>>> could always use pgbench on UNIX/Linux if we truely need the feature.
>>>
>>> What do you think?
>>
>> Fair enough, I was just trying to point out alternatives. We have
>> committed platform-specific features before now. I hope it doesn't
>> just get left like this, though.

We have committed platform-specific features before, but generally
only when it's not *possible* to do them for all platforms. For
example the posix_fadvise stuff isn't available on Windows at all, so
there isn't much we can do there.


> Yeah, I hope someone pick this up and propose as a TODO item. In the
> mean time, I'm going to commit the patch without Windows support
> unless there's objection.

Perhaps we should actually hold off until someone committs to actually
getting it fixed in the next version? If we do have that, then we can
commit it as a partial feature, but if we just "hope someone picks it
up", that's leaving it very loose..

--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/



Re: review: pgbench - aggregation of info written into log

From
Dave Page
Date:
On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander <magnus@hagander.net> wrote:
> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>>>> This might be way more than we want to do, but there is an article
>>>>> that describes some techniques for doing what seems to be missing
>>>>> (AIUI):
>>>>>
>>>>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
>>>> Even this would be doable, I'm afraid it may not fit in 9.3 if we
>>>> think about the current status of CF. So our choice would be:
>>>>
>>>> 1) Postpone the patch to 9.4
>>>>
>>>> 2) Commit the patch in 9.3 without Windows support
>>>>
>>>> I personally am ok with #2. We traditionally avoid particular paltform
>>>> specific features on PostgreSQL.  However I think the policiy could be
>>>> losen for contrib staffs. Also pgbench is just a client program. We
>>>> could always use pgbench on UNIX/Linux if we truely need the feature.
>>>>
>>>> What do you think?
>>>
>>> Fair enough, I was just trying to point out alternatives. We have
>>> committed platform-specific features before now. I hope it doesn't
>>> just get left like this, though.
>
> We have committed platform-specific features before, but generally
> only when it's not *possible* to do them for all platforms. For
> example the posix_fadvise stuff isn't available on Windows at all, so
> there isn't much we can do there.

Right - having platform specific features for other reasons like lack
of time is a slippery slope in my opinion. We should not get into such
a habit or Windows will quickly become a second class platform as far
as PostgreSQL features are concerned.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: review: pgbench - aggregation of info written into log

From
Magnus Hagander
Date:
On Thu, Jan 17, 2013 at 11:09 AM, Dave Page <dpage@pgadmin.org> wrote:
> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>>>>> This might be way more than we want to do, but there is an article
>>>>>> that describes some techniques for doing what seems to be missing
>>>>>> (AIUI):
>>>>>>
>>>>>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
>>>>> Even this would be doable, I'm afraid it may not fit in 9.3 if we
>>>>> think about the current status of CF. So our choice would be:
>>>>>
>>>>> 1) Postpone the patch to 9.4
>>>>>
>>>>> 2) Commit the patch in 9.3 without Windows support
>>>>>
>>>>> I personally am ok with #2. We traditionally avoid particular paltform
>>>>> specific features on PostgreSQL.  However I think the policiy could be
>>>>> losen for contrib staffs. Also pgbench is just a client program. We
>>>>> could always use pgbench on UNIX/Linux if we truely need the feature.
>>>>>
>>>>> What do you think?
>>>>
>>>> Fair enough, I was just trying to point out alternatives. We have
>>>> committed platform-specific features before now. I hope it doesn't
>>>> just get left like this, though.
>>
>> We have committed platform-specific features before, but generally
>> only when it's not *possible* to do them for all platforms. For
>> example the posix_fadvise stuff isn't available on Windows at all, so
>> there isn't much we can do there.
>
> Right - having platform specific features for other reasons like lack
> of time is a slippery slope in my opinion. We should not get into such
> a habit or Windows will quickly become a second class platform as far
> as PostgreSQL features are concerned.

Especially since there is no lack of time - the functionality is
there, it just looks (significantly) different.

--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/



Re: review: pgbench - aggregation of info written into log

From
Tomas Vondra
Date:
Dne 17.01.2013 10:36, Magnus Hagander napsal:
> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii@postgresql.org> 
> wrote:
>>>>> This might be way more than we want to do, but there is an 
>>>>> article
>>>>> that describes some techniques for doing what seems to be missing
>>>>> (AIUI):
>>>>>
>>>>> <http://msdn.microsoft.com/en-us/magazine/cc163996.aspx>
>>>> Even this would be doable, I'm afraid it may not fit in 9.3 if we
>>>> think about the current status of CF. So our choice would be:
>>>>
>>>> 1) Postpone the patch to 9.4
>>>>
>>>> 2) Commit the patch in 9.3 without Windows support
>>>>
>>>> I personally am ok with #2. We traditionally avoid particular 
>>>> paltform
>>>> specific features on PostgreSQL.  However I think the policiy 
>>>> could be
>>>> losen for contrib staffs. Also pgbench is just a client program. 
>>>> We
>>>> could always use pgbench on UNIX/Linux if we truely need the 
>>>> feature.
>>>>
>>>> What do you think?
>>>
>>> Fair enough, I was just trying to point out alternatives. We have
>>> committed platform-specific features before now. I hope it doesn't
>>> just get left like this, though.
>
> We have committed platform-specific features before, but generally
> only when it's not *possible* to do them for all platforms. For
> example the posix_fadvise stuff isn't available on Windows at all, so
> there isn't much we can do there.

Maybe, although this platform-dependence already exists in pgbench to 
some
extent (the Windows branch is unable to log the timestamps of 
transactions).

It would certainly be better if pgbench was able to handle Windows and
Linux equally, but that was not the aim of this patch.

>> Yeah, I hope someone pick this up and propose as a TODO item. In the
>> mean time, I'm going to commit the patch without Windows support
>> unless there's objection.
>
> Perhaps we should actually hold off until someone committs to 
> actually
> getting it fixed in the next version? If we do have that, then we can
> commit it as a partial feature, but if we just "hope someone picks it
> up", that's leaving it very loose..

Well, given that I'm an author of that patch, that 'someone' would have
to be me. The problem is I have access to absolutely no Windows 
machines,
not mentioning the development tools (and that I have no clue about 
it).

I vaguely remember there were people on this list doing Windows 
development
on a virtual machine or something. Any interest in working on this / 
giving
me some help?

Tomas



Re: review: pgbench - aggregation of info written into log

From
Tomas Vondra
Date:
Dne 17.01.2013 11:16, Magnus Hagander napsal:
> On Thu, Jan 17, 2013 at 11:09 AM, Dave Page <dpage@pgadmin.org> 
> wrote:
>> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander 
>> <magnus@hagander.net> wrote:
>>> We have committed platform-specific features before, but generally
>>> only when it's not *possible* to do them for all platforms. For
>>> example the posix_fadvise stuff isn't available on Windows at all, 
>>> so
>>> there isn't much we can do there.
>>
>> Right - having platform specific features for other reasons like 
>> lack
>> of time is a slippery slope in my opinion. We should not get into 
>> such
>> a habit or Windows will quickly become a second class platform as 
>> far
>> as PostgreSQL features are concerned.
>
> Especially since there is no lack of time - the functionality is
> there, it just looks (significantly) different.

Really? Any link to relevant docs or something?

When doing some research in this field, the only option I was able to 
come up
with was combining gettimeofday() with the timing functionality, and do 
something
like this:
  1) call gettimeofday() at thread start, giving a common unix 
timestamp  2) measure the time from the thread start using the conters (for each 
transaction)  3) combine those values

This might of course give up to a second difference compared to the 
actual time
(because of the gettimeofday precision), but IMHO that's fine.

An even simpler option would omit the (1), so the timestamps would 
start at 0.

Or is there a better way?

Tomas



Re: review: pgbench - aggregation of info written into log

From
Andrew Dunstan
Date:
On 01/17/2013 06:11 AM, Tomas Vondra wrote:
> Dne 17.01.2013 11:16, Magnus Hagander napsal:
>> On Thu, Jan 17, 2013 at 11:09 AM, Dave Page <dpage@pgadmin.org> wrote:
>>> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander 
>>> <magnus@hagander.net> wrote:
>>>> We have committed platform-specific features before, but generally
>>>> only when it's not *possible* to do them for all platforms. For
>>>> example the posix_fadvise stuff isn't available on Windows at all, so
>>>> there isn't much we can do there.
>>>
>>> Right - having platform specific features for other reasons like lack
>>> of time is a slippery slope in my opinion. We should not get into such
>>> a habit or Windows will quickly become a second class platform as far
>>> as PostgreSQL features are concerned.
>>
>> Especially since there is no lack of time - the functionality is
>> there, it just looks (significantly) different.
>
> Really? Any link to relevant docs or something?
>
> When doing some research in this field, the only option I was able to 
> come up
> with was combining gettimeofday() with the timing functionality, and 
> do something
> like this:
>
>   1) call gettimeofday() at thread start, giving a common unix timestamp
>   2) measure the time from the thread start using the conters (for 
> each transaction)
>   3) combine those values
>
> This might of course give up to a second difference compared to the 
> actual time
> (because of the gettimeofday precision), but IMHO that's fine.

The link I posted showed a technique which essentially uses edge 
detection on gettimeofday() to get an accurate correspondence between 
the time of day and the performance counter, following which you can 
supposedly calculate the time of day with a high degree of accuracy just 
from the performance counter. You should be able to do this just once, 
at program start. If you don't care that much about the initial 
precision then your procedure should work fine, I think.

cheers

andrew



Re: review: pgbench - aggregation of info written into log

From
Andrew Dunstan
Date:
On 01/17/2013 06:04 AM, Tomas Vondra wrote:
> The problem is I have access to absolutely no Windows machines,
> not mentioning the development tools (and that I have no clue about it).
>
> I vaguely remember there were people on this list doing Windows 
> development
> on a virtual machine or something. Any interest in working on this / 
> giving
> me some help?
>
>


One of the items on my very long TODO list (see stuff elsewhere about 
committers not doing enough reviewing and committing) is to create a 
package that can easily be run to set Windows Postgres development 
environments for MSVC, Mingw and Cygwin on Amazon, GoGrid etc.

Once I get that done I'll be a lot less sympathetic to "I don't have 
access to Windows" pleas.

Windows does run in a VM very well, but if you're going to set it up on 
your own VM environment, (e.h. VirtualBox or KVM/qemu) you need your own 
legal copy to install there. If you don't already have one, that will 
set you back about $140.00 (for w7 Pro) in the USA. Note that that's 
significantly better than the situation with OSX, which you can't run at 
all except on Apple hardware.

cheers

andrew




Re: review: pgbench - aggregation of info written into log

From
Dave Page
Date:
On Thu, Jan 17, 2013 at 1:29 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
> On 01/17/2013 06:04 AM, Tomas Vondra wrote:
>>
>> The problem is I have access to absolutely no Windows machines,
>> not mentioning the development tools (and that I have no clue about it).
>>
>> I vaguely remember there were people on this list doing Windows
>> development
>> on a virtual machine or something. Any interest in working on this /
>> giving
>> me some help?
>>
>>
>
>
> One of the items on my very long TODO list (see stuff elsewhere about
> committers not doing enough reviewing and committing) is to create a package
> that can easily be run to set Windows Postgres development environments for
> MSVC, Mingw and Cygwin on Amazon, GoGrid etc.

FYI, I have a similar item on my TODO list, though I was looking
primarily at VC++ on AWS. It did get close to the top last week, but
then I got busy with other things :-/. Anyway, I'd suggest we ping
each other if either actually get started, to avoid duplication of
effort.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: review: pgbench - aggregation of info written into log

From
Magnus Hagander
Date:
On Thu, Jan 17, 2013 at 2:29 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
> On 01/17/2013 06:04 AM, Tomas Vondra wrote:
>>
>> The problem is I have access to absolutely no Windows machines,
>> not mentioning the development tools (and that I have no clue about it).
>>
>> I vaguely remember there were people on this list doing Windows
>> development
>> on a virtual machine or something. Any interest in working on this /
>> giving
>> me some help?
>>
>>
>
>
> One of the items on my very long TODO list (see stuff elsewhere about
> committers not doing enough reviewing and committing) is to create a package
> that can easily be run to set Windows Postgres development environments for
> MSVC, Mingw and Cygwin on Amazon, GoGrid etc.
>
> Once I get that done I'll be a lot less sympathetic to "I don't have access
> to Windows" pleas.
>
> Windows does run in a VM very well, but if you're going to set it up on your
> own VM environment, (e.h. VirtualBox or KVM/qemu) you need your own legal
> copy to install there. If you don't already have one, that will set you back
> about $140.00 (for w7 Pro) in the USA. Note that that's significantly better
> than the situation with OSX, which you can't run at all except on Apple
> hardware.

Yeah. I used to have an AMI with the VS environment preinstalled on
Amazon, but I managed to fat finger things and delete it at some point
and haven't really had time to rebuild it.

Having a script that would download and install all the pre-requisites
on such a box would be *great*. Then you could get up and going pretty
quickly, and getting a Windows box up running for a few hours there is
almost free, and you don't have to deal with licensing hassles.

(Of course, the AMI method doesn't work all the way since you'd be
distributing Visual Studio, but if we can have a script that
auto-downloads-and-installs it as necessary we can get around that)


--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/



Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
> On Thu, Jan 17, 2013 at 2:29 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>> On 01/17/2013 06:04 AM, Tomas Vondra wrote:
>>>
>>> The problem is I have access to absolutely no Windows machines,
>>> not mentioning the development tools (and that I have no clue about it).
>>>
>>> I vaguely remember there were people on this list doing Windows
>>> development
>>> on a virtual machine or something. Any interest in working on this /
>>> giving
>>> me some help?
>>>
>>>
>>
>>
>> One of the items on my very long TODO list (see stuff elsewhere about
>> committers not doing enough reviewing and committing) is to create a package
>> that can easily be run to set Windows Postgres development environments for
>> MSVC, Mingw and Cygwin on Amazon, GoGrid etc.
>>
>> Once I get that done I'll be a lot less sympathetic to "I don't have access
>> to Windows" pleas.
>>
>> Windows does run in a VM very well, but if you're going to set it up on your
>> own VM environment, (e.h. VirtualBox or KVM/qemu) you need your own legal
>> copy to install there. If you don't already have one, that will set you back
>> about $140.00 (for w7 Pro) in the USA. Note that that's significantly better
>> than the situation with OSX, which you can't run at all except on Apple
>> hardware.
> 
> Yeah. I used to have an AMI with the VS environment preinstalled on
> Amazon, but I managed to fat finger things and delete it at some point
> and haven't really had time to rebuild it.
> 
> Having a script that would download and install all the pre-requisites
> on such a box would be *great*. Then you could get up and going pretty
> quickly, and getting a Windows box up running for a few hours there is
> almost free, and you don't have to deal with licensing hassles.
> 
> (Of course, the AMI method doesn't work all the way since you'd be
> distributing Visual Studio, but if we can have a script that
> auto-downloads-and-installs it as necessary we can get around that)

So if my understating is correct, 1)Tomas Vondra commits to work on
Windows support for 9.4, 2)on the assumption that one of Andrew
Dunstan, Dave Page or Magnus Hagander will help him in Windows
development.

Ok? If so, I can commit the patch for 9.3 without Windows support. If
not, I will move the patch to next CF (for 9.4).

Please correct me if I am wrong.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp



Re: review: pgbench - aggregation of info written into log

From
Robert Haas
Date:
On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
> So if my understating is correct, 1)Tomas Vondra commits to work on
> Windows support for 9.4, 2)on the assumption that one of Andrew
> Dunstan, Dave Page or Magnus Hagander will help him in Windows
> development.
>
> Ok? If so, I can commit the patch for 9.3 without Windows support. If
> not, I will move the patch to next CF (for 9.4).
>
> Please correct me if I am wrong.

+1 for this approach.  I agree with Dave and Magnus that we don't want
Windows to become a second-class platform, but this patch isn't making
it so.  The #ifdef that peeks inside of an instr_time is already
there, and it's not Tomas's fault that nobody has gotten around to
fixing it before now.

OTOH, I think that this sort of thing is quite wrong:

+#ifndef WIN32
+           "  --aggregate-interval NUM\n"
+           "               aggregate data over NUM seconds\n"
+#endif

The right approach if this can't be supported on Windows is to still
display the option in the --help output, and to display an error
message if the user tries to use it, saying that it is not currently
supported on Windows.  That fact should also be mentioned in the
documentation.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>> So if my understating is correct, 1)Tomas Vondra commits to work on
>> Windows support for 9.4, 2)on the assumption that one of Andrew
>> Dunstan, Dave Page or Magnus Hagander will help him in Windows
>> development.
>>
>> Ok? If so, I can commit the patch for 9.3 without Windows support. If
>> not, I will move the patch to next CF (for 9.4).
>>
>> Please correct me if I am wrong.
> 
> +1 for this approach.  I agree with Dave and Magnus that we don't want
> Windows to become a second-class platform, but this patch isn't making
> it so.  The #ifdef that peeks inside of an instr_time is already
> there, and it's not Tomas's fault that nobody has gotten around to
> fixing it before now.

Right.

> OTOH, I think that this sort of thing is quite wrong:
> 
> +#ifndef WIN32
> +           "  --aggregate-interval NUM\n"
> +           "               aggregate data over NUM seconds\n"
> +#endif
> 
> The right approach if this can't be supported on Windows is to still
> display the option in the --help output, and to display an error
> message if the user tries to use it, saying that it is not currently
> supported on Windows.  That fact should also be mentioned in the
> documentation.

Agreed. This seems to be much better approach.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp



Re: Making testing on Windows easier

From
Craig Ringer
Date:
<div class="moz-cite-prefix">On 01/17/2013 09:36 PM, Magnus Hagander wrote:<br /></div><blockquote
cite="mid:CABUevEyeiSFk=428ThV5tOR=Xsz9rpOno=uhAznVzgNG3woo7w@mail.gmail.com"type="cite"><br /><pre wrap="">
 
Yeah. I used to have an AMI with the VS environment preinstalled on
Amazon, but I managed to fat finger things and delete it at some point
and haven't really had time to rebuild it.

Having a script that would download and install all the pre-requisites
on such a box would be *great*.</pre></blockquote> I'm working on it:<br /><br /><a
href="https://github.com/2ndQuadrant/pg_build_win">https://github.com/2ndQuadrant/pg_build_win</a><br/><br /><a
href="http://blog.2ndquadrant.com/easier-postgresql-builds-for-windows/">http://blog.2ndquadrant.com/easier-postgresql-builds-for-windows/</a><br
/><br/> I've identified the silent install procedures for most of the required tools (see the docs) and got build
recipeswritten for some of the library dependencies. The next planned step is to have the scripts automatically
downloadand silent-install Visual Studio, etc, rather than have the user run the command lines given in the README
manually.<br/><br /> It's usable as-is, I just need the time to finish it off. The goal is to have a script that turns
buildingPostgreSQL on a clean fresh Windows install into:<br /><br /> - Download ActivePerl<br /> - Install
ActivePerl<br/> - Run "buildgit.pl check install"<br /><br /> Right now it takes a fair bit more than that, but it's
alreadybetter than a fully manual build.<br /><br /><blockquote
cite="mid:CABUevEyeiSFk=428ThV5tOR=Xsz9rpOno=uhAznVzgNG3woo7w@mail.gmail.com"type="cite"><pre wrap="">Then you could
getup and going pretty
 
quickly, and getting a Windows box up running for a few hours there is
almost free, and you don't have to deal with licensing hassles.

(Of course, the AMI method doesn't work all the way since you'd be
distributing Visual Studio, but if we can have a script that
auto-downloads-and-installs it as necessary we can get around that)
</pre></blockquote> I've found EC2 to be unusably slow for Windows builds, with a medium instance taking an hour and a
halfto do a simple build and "vcregress check". They're also restrictive in disk space terms, so you land up needing to
adda second EBS volume.<br /><br /> A local kvm instance works well if a physical host isn't available; so do some of
thealternative cloud providers like LunaCloud, which seems to perform significantly better in the quick testing I did.
Ihaven't tried GoGrid yet.<br /><br /> Many of us have Windows license stickers on laptops/desktops, even if we don't
usethe thing, so for a signficiant proportion of people it's as simple as downloading Windows install media ( <a
href="http://blog.ringerc.id.au/2012/05/you-can-download-legal-windows-7-iso.html">http://blog.ringerc.id.au/2012/05/you-can-download-legal-windows-7-iso.html</a>)
andinstalling a KVM instance then shapshotting it.<br /><br /><br /> I've also put together a Jenkins server that runs
buildson Windows whenever they're pushed to watched git repos. I'd love to make this public, but unfortunately allowing
awide group to run arbitrary code on the current build box isn't something I can afford. I'd need a system that
launcheda snapshot Windows instance for each build and then destroyed it at the end. This is entirely practical with
somethinglike KVM, so if I ever get the chance to work on a Jenkins plugin to do that (or to launch/destroy Windows
cloudinstances automatically), it's possible a semi-public Windows build testing service may be possible.<br /><br />
Whilewe're in fantasy land, the next step would be adding git URLs and branch names to the commitfest app, so it could
pinga continuous integration server to build-test any new patch added to the CF. Right now I'm doing that manually when
Ihave time, but it's slow going.<br /><br /><pre class="moz-signature" cols="72">-- Craig Ringer                   <a
class="moz-txt-link-freetext"href="http://www.2ndQuadrant.com/">http://www.2ndQuadrant.com/</a>PostgreSQL Development,
24x7Support, Training & Services</pre> 

Re: Making testing on Windows easier

From
Noah Misch
Date:
On Mon, Jan 21, 2013 at 10:01:29AM +0800, Craig Ringer wrote:
> I've found EC2 to be unusably slow for Windows builds, with a medium
> instance taking an hour and a half to do a simple build and "vcregress
> check". They're also restrictive in disk space terms, so you land up
> needing to add a second EBS volume.

Yikes.  The "build DEBUG" step takes 5-7 minutes for me; I use EBS-optimized
m1.xlarge spot instances in US East (lately about US$0.19/hr).  Fairly sure I
once used a c1.medium, though, and it still took <10 minutes.  I don't know
why your experience has been so different.



Re: Making testing on Windows easier

From
Craig Ringer
Date:
On 01/21/2013 08:55 PM, Noah Misch wrote:
> On Mon, Jan 21, 2013 at 10:01:29AM +0800, Craig Ringer wrote:
>> I've found EC2 to be unusably slow for Windows builds, with a medium
>> instance taking an hour and a half to do a simple build and "vcregress
>> check". They're also restrictive in disk space terms, so you land up
>> needing to add a second EBS volume.
> Yikes.  The "build DEBUG" step takes 5-7 minutes for me; I use EBS-optimized
> m1.xlarge spot instances in US East (lately about US$0.19/hr).  Fairly sure I
> once used a c1.medium, though, and it still took <10 minutes.  I don't know
> why your experience has been so different.
I was using m1.medium instances, but the same instance type gets Linux
builds done in 15-20 mins. Slow, but not that slow. Performance was
consistently miserable across several instances, including one full
clean rebuild from scratch. Weird.

I should perhaps give the bigger instances a go. Unfortunately Jenkins
can't auto-start and auto-stop Windows instances yet and I don't have
time to improve the Jenkins ec2 plugin right now, so using any instance
bigger than an m1.medium would pretty much require manually stopping and
starting it for builds. Yick.

Instead I'm using my home media PC, a Pentium G630 (like a cut-down Core
2 i3) with a laptop hard drive. It completes builds in 20 minutes. My
more power-hungry laptop does it in 7.

I was never able to determine why the Windows instances were so much
slower than the corresponding Linux instance of the same type.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services




Re: Making testing on Windows easier

From
Dave Page
Date:
On Mon, Jan 21, 2013 at 1:00 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 01/21/2013 08:55 PM, Noah Misch wrote:
>> On Mon, Jan 21, 2013 at 10:01:29AM +0800, Craig Ringer wrote:
>>> I've found EC2 to be unusably slow for Windows builds, with a medium
>>> instance taking an hour and a half to do a simple build and "vcregress
>>> check". They're also restrictive in disk space terms, so you land up
>>> needing to add a second EBS volume.
>> Yikes.  The "build DEBUG" step takes 5-7 minutes for me; I use EBS-optimized
>> m1.xlarge spot instances in US East (lately about US$0.19/hr).  Fairly sure I
>> once used a c1.medium, though, and it still took <10 minutes.  I don't know
>> why your experience has been so different.
> I was using m1.medium instances, but the same instance type gets Linux
> builds done in 15-20 mins. Slow, but not that slow. Performance was
> consistently miserable across several instances, including one full
> clean rebuild from scratch. Weird.

FYI, in my experience VC++ is typically much faster than GCC, on
comparable hardware - particularly with C++.

> I should perhaps give the bigger instances a go. Unfortunately Jenkins
> can't auto-start and auto-stop Windows instances yet and I don't have
> time to improve the Jenkins ec2 plugin right now, so using any instance
> bigger than an m1.medium would pretty much require manually stopping and
> starting it for builds. Yick.

It should be trivial to script I would think - it's a one-liner to
create an instance with ec2 tools.

> Instead I'm using my home media PC, a Pentium G630 (like a cut-down Core
> 2 i3) with a laptop hard drive. It completes builds in 20 minutes. My
> more power-hungry laptop does it in 7.
>
> I was never able to determine why the Windows instances were so much
> slower than the corresponding Linux instance of the same type.

Full vs. para-virtualisation perhaps?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Making testing on Windows easier

From
Andrew Dunstan
Date:
On 01/21/2013 08:11 AM, Dave Page wrote:

>>
>> I was never able to determine why the Windows instances were so much
>> slower than the corresponding Linux instance of the same type.
> Full vs. para-virtualisation perhaps?
>

No, Windows builds just are slower. For some time the buildfarm has been 
reporting run times for various members, so there's plenty of data on 
this. For example, nightjar and currawong are respectively FreeBSD/gcc 
and WindowsXP/VC2008 members running in VMs on the same host. currawong 
takes three times as long for a buildfarm run even though it's doing 
less work.

cheers

andrdew




Re: Making testing on Windows easier

From
Dave Page
Date:
On Mon, Jan 21, 2013 at 1:59 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
> On 01/21/2013 08:11 AM, Dave Page wrote:
>
>>>
>>> I was never able to determine why the Windows instances were so much
>>> slower than the corresponding Linux instance of the same type.
>>
>> Full vs. para-virtualisation perhaps?
>>
>
> No, Windows builds just are slower. For some time the buildfarm has been
> reporting run times for various members, so there's plenty of data on this.
> For example, nightjar and currawong are respectively FreeBSD/gcc and
> WindowsXP/VC2008 members running in VMs on the same host. currawong takes
> three times as long for a buildfarm run even though it's doing less work.

Hmm, OK. I don't build PostgreSQL interactively enough to notice I
guess. For C++ it's definitely the other way round - I can build
pgAdmin in a fraction of the time in a Windows VM than I can on the
host Mac it runs on.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Making testing on Windows easier

From
Magnus Hagander
Date:
On Mon, Jan 21, 2013 at 3:03 PM, Dave Page <dpage@pgadmin.org> wrote:
> On Mon, Jan 21, 2013 at 1:59 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>> On 01/21/2013 08:11 AM, Dave Page wrote:
>>
>>>>
>>>> I was never able to determine why the Windows instances were so much
>>>> slower than the corresponding Linux instance of the same type.
>>>
>>> Full vs. para-virtualisation perhaps?
>>>
>>
>> No, Windows builds just are slower. For some time the buildfarm has been
>> reporting run times for various members, so there's plenty of data on this.
>> For example, nightjar and currawong are respectively FreeBSD/gcc and
>> WindowsXP/VC2008 members running in VMs on the same host. currawong takes
>> three times as long for a buildfarm run even though it's doing less work.
>
> Hmm, OK. I don't build PostgreSQL interactively enough to notice I
> guess. For C++ it's definitely the other way round - I can build
> pgAdmin in a fraction of the time in a Windows VM than I can on the
> host Mac it runs on.

Yes, for C++ it's a difference in at least one order of magnitude.

For C it definitely isn't. MSVC tends to be faster than gcc (both on
windows), which I think is mostly because it builds multiple files in
one run. However, actually starting each build step takes
significantly longer. We've also added some things like the DEF file
magic that can definitely take quite some time, particularly when
building the backend.


--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/



Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
>> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>> So if my understating is correct, 1)Tomas Vondra commits to work on
>>> Windows support for 9.4, 2)on the assumption that one of Andrew
>>> Dunstan, Dave Page or Magnus Hagander will help him in Windows
>>> development.
>>>
>>> Ok? If so, I can commit the patch for 9.3 without Windows support. If
>>> not, I will move the patch to next CF (for 9.4).
>>>
>>> Please correct me if I am wrong.
>> 
>> +1 for this approach.  I agree with Dave and Magnus that we don't want
>> Windows to become a second-class platform, but this patch isn't making
>> it so.  The #ifdef that peeks inside of an instr_time is already
>> there, and it's not Tomas's fault that nobody has gotten around to
>> fixing it before now.
> 
> Right.
> 
>> OTOH, I think that this sort of thing is quite wrong:
>> 
>> +#ifndef WIN32
>> +           "  --aggregate-interval NUM\n"
>> +           "               aggregate data over NUM seconds\n"
>> +#endif
>> 
>> The right approach if this can't be supported on Windows is to still
>> display the option in the --help output, and to display an error
>> message if the user tries to use it, saying that it is not currently
>> supported on Windows.  That fact should also be mentioned in the
>> documentation.
> 
> Agreed. This seems to be much better approach.

Here is the new patch.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 3ca120f..e8867a6 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -153,6 +153,7 @@ char       *index_tablespace = NULL;bool        use_log;            /* log transaction latencies to
afile */bool        use_quiet;            /* quiet logging onto stderr */
 
+int            agg_interval;        /* log aggregates instead of individual transactions */bool        is_connect;
      /* establish connection for each transaction */bool        is_latencies;        /* report per-command latencies
*/int           main_pid;            /* main process id used in log filename */
 
@@ -248,6 +249,18 @@ typedef struct    char       *argv[MAX_ARGS]; /* command word list */} Command;
+typedef struct
+{
+
+    long    start_time;            /* when does the interval start */
+    int     cnt;                /* number of transactions */
+    double    min_duration;        /* min/max durations */
+    double    max_duration;
+    double    sum;                /* sum(duration), sum(duration^2) - for estimates */
+    double    sum2;
+    
+} AggVals;
+static Command **sql_files[MAX_FILES];    /* SQL script files */static int    num_files;            /* number of
scriptfiles */static int    num_commands = 0;    /* total number of Command structs */
 
@@ -381,6 +394,8 @@ usage(void)           "  -l           write transaction times to log file\n"           "
--sampling-rateNUM\n"           "               fraction of transactions to log (e.g. 0.01 for 1%% sample)\n"
 
+           "  --aggregate-interval NUM\n"
+           "               aggregate data over NUM seconds\n"           "  -M simple|extended|prepared\n"           "
            protocol for submitting queries to server (default: simple)\n"           "  -n           do not run VACUUM
beforetests\n"
 
@@ -834,9 +849,25 @@ clientDone(CState *st, bool ok)    return false;                /* always false */}
+static
+void agg_vals_init(AggVals * aggs, instr_time start)
+{
+    /* basic counters */
+    aggs->cnt = 0;        /* number of transactions */
+    aggs->sum = 0;        /* SUM(duration) */
+    aggs->sum2 = 0;        /* SUM(duration*duration) */
+
+    /* min and max transaction duration */
+    aggs->min_duration = 0;
+    aggs->max_duration = 0;
+
+    /* start of the current interval */
+    aggs->start_time = INSTR_TIME_GET_DOUBLE(start);
+}
+/* return false iff client should be disconnected */static bool
-doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile)
+doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile, AggVals * agg){    PGresult   *res;
Command  **commands;
 
@@ -901,22 +932,74 @@ top:            if (sample_rate == 0.0 ||                pg_erand48(thread->random_state) <=
sample_rate)           {
 
-                INSTR_TIME_SET_CURRENT(now);                diff = now;                INSTR_TIME_SUBTRACT(diff,
st->txn_begin);               usec = (double) INSTR_TIME_GET_MICROSEC(diff);
 
+                /* should we aggregate the results or not? */
+                if (agg_interval > 0)
+                {
+                    /* are we still in the same interval? if yes, accumulate the
+                    * values (print them otherwise) */
+                    if (agg->start_time + agg_interval >= INSTR_TIME_GET_DOUBLE(now))
+                    {
+                        agg->cnt += 1;
+                        agg->sum  += usec;
+                        agg->sum2 += usec * usec;
+
+                        /* first in this aggregation interval */
+                        if ((agg->cnt == 1) || (usec < agg->min_duration))
+                            agg->min_duration =  usec;
+
+                        if ((agg->cnt == 1) || (usec > agg->max_duration))
+                            agg->max_duration = usec;
+                    }
+                    else
+                    {
+                        /* Loop until we reach the interval of the current transaction (and
+                         * print all the empty intervals in between). */
+                        while (agg->start_time + agg_interval < INSTR_TIME_GET_DOUBLE(now))
+                        {
+                            /* This is a non-Windows branch (thanks to the ifdef in usage), so
+                             * we don't need to handle this in a special way (see below). */
+                            fprintf(logfile, "%ld %d %.0f %.0f %.0f %.0f\n",
+                                    agg->start_time, agg->cnt, agg->sum, agg->sum2,
+                                    agg->min_duration, agg->max_duration);
+
+                            /* move to the next inteval */
+                            agg->start_time = agg->start_time + agg_interval;
+
+                            /* reset for "no transaction" intervals */
+                            agg->cnt = 0;
+                            agg->min_duration = 0;
+                            agg->max_duration = 0;
+                            agg->sum = 0;
+                            agg->sum2 = 0;
+                        }
+
+                        /* and now update the reset values (include the current) */
+                        agg->cnt = 1;
+                        agg->min_duration = usec;
+                        agg->max_duration = usec;
+                        agg->sum = usec;
+                        agg->sum2 = usec * usec;
+                    }
+                }
+                else
+                {
+                    /* no, print raw transactions */#ifndef WIN32
-                /* This is more than we really ought to know about instr_time */
-                fprintf(logfile, "%d %d %.0f %d %ld %ld\n",
-                        st->id, st->cnt, usec, st->use_file,
-                        (long) now.tv_sec, (long) now.tv_usec);
+                    /* This is more than we really ought to know about instr_time */
+                    fprintf(logfile, "%d %d %.0f %d %ld %ld\n",
+                            st->id, st->cnt, usec, st->use_file,
+                            (long) now.tv_sec, (long) now.tv_usec);#else
-                /* On Windows, instr_time doesn't provide a timestamp anyway */
-                fprintf(logfile, "%d %d %.0f %d 0 0\n",
-                        st->id, st->cnt, usec, st->use_file);
+                    /* On Windows, instr_time doesn't provide a timestamp anyway */
+                    fprintf(logfile, "%d %d %.0f %d 0 0\n",
+                            st->id, st->cnt, usec, st->use_file);#endif
+                }            }        }
@@ -1964,6 +2047,9 @@ main(int argc, char **argv)        {"tablespace", required_argument, NULL, 2},
{"unlogged-tables",no_argument, &unlogged_tables, 1},        {"sampling-rate", required_argument, NULL, 4},
 
+#ifndef WIN32
+        {"aggregate-interval", required_argument, NULL, 5},
+#endif        {NULL, 0, NULL, 0}    };
@@ -2202,6 +2288,19 @@ main(int argc, char **argv)                    exit(1);                }                break;
+            case 5:
+#ifdef WIN32
+                fprintf(stderr, "--aggregate-interval is not currently supported on Windows");
+                exit(1);
+#else
+                agg_interval = atoi(optarg);
+                if (agg_interval <= 0)
+                {
+                    fprintf(stderr, "invalid number of seconds for aggregation: %d\n", agg_interval);
+                    exit(1);
+                }
+#endif
+                break;            default:                fprintf(stderr, _("Try \"%s --help\" for more
information.\n"),progname);                exit(1);
 
@@ -2251,6 +2350,28 @@ main(int argc, char **argv)        exit(1);    }
+    /* --sampling-rate may must not be used with --aggregate-interval */
+    if (sample_rate > 0.0 && agg_interval > 0)
+    {
+        fprintf(stderr, "log sampling (--sampling-rate) and aggregation (--aggregate-interval) can't be used at the
sametime\n");
 
+        exit(1);
+    }
+
+    if (agg_interval > 0 && (! use_log)) {
+        fprintf(stderr, "log aggregation is allowed only when actually logging transactions\n");
+        exit(1);
+    }
+
+    if ((duration > 0) && (agg_interval > duration)) {
+        fprintf(stderr, "number of seconds for aggregation (%d) must not be higher that test duration (%d)\n",
agg_interval,duration);
 
+        exit(1);
+    }
+
+    if ((duration > 0) && (agg_interval > 0) && (duration % agg_interval != 0)) {
+        fprintf(stderr, "duration (%d) must be a multiple of aggregation interval (%d)\n", duration, agg_interval);
+        exit(1);
+    }
+    /*     * is_latencies only works with multiple threads in thread-based     * implementations, not fork-based ones,
becauseit supposes that the
 
@@ -2510,7 +2631,10 @@ threadRun(void *arg)    int            remains = nstate;        /* number of remaining clients
*/   int            i;
 
+    AggVals        aggs;
+    result = pg_malloc(sizeof(TResult));
+        INSTR_TIME_SET_ZERO(result->conn_time);    /* open log file if requested */
@@ -2545,6 +2669,8 @@ threadRun(void *arg)    INSTR_TIME_SET_CURRENT(result->conn_time);
INSTR_TIME_SUBTRACT(result->conn_time,thread->start_time);
 
+    agg_vals_init(&aggs, thread->start_time);
+        /* send start up queries in async manner */    for (i = 0; i < nstate; i++)    {
@@ -2553,7 +2679,7 @@ threadRun(void *arg)        int            prev_ecnt = st->ecnt;        st->use_file =
getrand(thread,0, num_files - 1);
 
-        if (!doCustom(thread, st, &result->conn_time, logfile))
+        if (!doCustom(thread, st, &result->conn_time, logfile, &aggs))            remains--;            /* I've
aborted*/        if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND) 
@@ -2655,7 +2781,7 @@ threadRun(void *arg)            if (st->con && (FD_ISSET(PQsocket(st->con), &input_mask)
                 || commands[st->state]->type == META_COMMAND))            {
 
-                if (!doCustom(thread, st, &result->conn_time, logfile))
+                if (!doCustom(thread, st, &result->conn_time, logfile, &aggs))                    remains--;    /*
I'veaborted */            }
 
@@ -2682,7 +2808,6 @@ done:    return result;}
-/* * Support for duration option: set timer_exceeded after so many seconds. */
diff --git a/doc/src/sgml/pgbench.sgml b/doc/src/sgml/pgbench.sgml
index 58686b1..fc1c051 100644
--- a/doc/src/sgml/pgbench.sgml
+++ b/doc/src/sgml/pgbench.sgml
@@ -346,6 +346,21 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>     </varlistentry>
  <varlistentry>
 
+      <term><option>--aggregate-interval</option> <replaceable>seconds</></term>
+      <listitem>
+       <para>
+        Length of aggregation interval (in seconds). May be used only together
+        with <application>-l</application> - with this option, the log contains
+        per-interval summary (number of transactions, min/max latency and two
+        additional fields useful for variance estimation).
+       </para>
+       <para>
+        This option is not currently supported on Windows.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>      <term><option>-M</option> <replaceable>querymode</></term>      <listitem>       <para>
@@ -741,8 +756,9 @@ END;  <title>Per-Transaction Logging</title>  <para>
-   With the <option>-l</> option, <application>pgbench</> writes the time
-   taken by each transaction to a log file.  The log file will be named
+   With the <option>-l</> option but without the <option>--aggregate-interval</option>,
+   <application>pgbench</> writes the time taken by each transaction
+   to a log file.  The log file will be named   <filename>pgbench_log.<replaceable>nnn</></filename>, where
<replaceable>nnn</>is the PID of the pgbench process.   If the <option>-j</> option is 2 or higher, creating multiple
worker
@@ -788,6 +804,45 @@ END; </refsect2> <refsect2>
+  <title>Aggregated Logging</title>
+  
+  <para>
+   With the <option>--aggregate-interval</option> option, the logs use a bit different format:
+
+<synopsis>
+<replaceable>interval_start</> <replaceable>num_of_transactions</> <replaceable>latency_sum</>
<replaceable>latency_2_sum</><replaceable>min_latency</> <replaceable>max_latency</>
 
+</synopsis>
+
+   where <replaceable>interval_start</> is the start of the interval (UNIX epoch
+   format timestamp), <replaceable>num_of_transactions</> is the number of transactions
+   within the interval, <replaceable>latency_sum</replaceable> is a sum of latencies
+   (so you can compute average latency easily). The following two fields are useful
+   for variance estimation - <replaceable>latency_sum</> is a sum of latencies and
+   <replaceable>latency_2_sum</> is a sum of 2nd powers of latencies. The last two
+   fields are <replaceable>min_latency</> - a minimum latency within the interval, and
+   <replaceable>max_latency</> - maximum latency within the interval. A transaction is
+   counted into the interval when it was committed.
+  </para>
+
+  <para>
+   Here is example outputs:
+<screen>
+1345828501 5601 1542744 483552416 61 2573
+1345828503 7884 1979812 565806736 60 1479
+1345828505 7208 1979422 567277552 59 1391
+1345828507 7685 1980268 569784714 60 1398
+1345828509 7073 1979779 573489941 236 1411
+</screen></para>
+
+  <para>
+   Notice that while the plain (unaggregated) log file contains index
+   of the custom script files, the aggregated log does not. Therefore if
+   you need per script data, you need to aggregate the data on your own.
+  </para>
+
+ </refsect2>
+
+ <refsect2>  <title>Per-Statement Latencies</title>  <para>

Re: review: pgbench - aggregation of info written into log

From
Tatsuo Ishii
Date:
>>> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii@postgresql.org> wrote:
>>>> So if my understating is correct, 1)Tomas Vondra commits to work on
>>>> Windows support for 9.4, 2)on the assumption that one of Andrew
>>>> Dunstan, Dave Page or Magnus Hagander will help him in Windows
>>>> development.
>>>>
>>>> Ok? If so, I can commit the patch for 9.3 without Windows support. If
>>>> not, I will move the patch to next CF (for 9.4).
>>>>
>>>> Please correct me if I am wrong.
>>> 
>>> +1 for this approach.  I agree with Dave and Magnus that we don't want
>>> Windows to become a second-class platform, but this patch isn't making
>>> it so.  The #ifdef that peeks inside of an instr_time is already
>>> there, and it's not Tomas's fault that nobody has gotten around to
>>> fixing it before now.
>> 
>> Right.
>> 
>>> OTOH, I think that this sort of thing is quite wrong:
>>> 
>>> +#ifndef WIN32
>>> +           "  --aggregate-interval NUM\n"
>>> +           "               aggregate data over NUM seconds\n"
>>> +#endif
>>> 
>>> The right approach if this can't be supported on Windows is to still
>>> display the option in the --help output, and to display an error
>>> message if the user tries to use it, saying that it is not currently
>>> supported on Windows.  That fact should also be mentioned in the
>>> documentation.
>> 
>> Agreed. This seems to be much better approach.
> 
> Here is the new patch.

Committed (with minor fix).
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp