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

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

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

Hello

2012/11/12 Tomas Vondra <tv(at)fuzzy(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

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

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

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

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

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

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

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

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

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

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

On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii(at)postgresql(dot)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 Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii(at)postgresql(dot)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

On Thu, Jan 17, 2013 at 11:09 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii(at)postgresql(dot)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 Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Dne 17.01.2013 10:36, Magnus Hagander napsal:
> On Thu, Jan 17, 2013 at 2:35 AM, Tatsuo Ishii <ishii(at)postgresql(dot)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

Dne 17.01.2013 11:16, Magnus Hagander napsal:
> On Thu, Jan 17, 2013 at 11:09 AM, Dave Page <dpage(at)pgadmin(dot)org>
> wrote:
>> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander
>> <magnus(at)hagander(dot)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

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(at)pgadmin(dot)org> wrote:
>>> On Thu, Jan 17, 2013 at 9:36 AM, Magnus Hagander
>>> <magnus(at)hagander(dot)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

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

On Thu, Jan 17, 2013 at 1:29 PM, Andrew Dunstan <andrew(at)dunslane(dot)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

On Thu, Jan 17, 2013 at 2:29 PM, Andrew Dunstan <andrew(at)dunslane(dot)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 Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

> On Thu, Jan 17, 2013 at 2:29 PM, Andrew Dunstan <andrew(at)dunslane(dot)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

On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii(at)postgresql(dot)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

> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii(at)postgresql(dot)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

On 01/17/2013 09:36 PM, Magnus Hagander wrote:
>
> 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*.
I'm working on it:

https://github.com/2ndQuadrant/pg_build_win

http://blog.2ndquadrant.com/easier-postgresql-builds-for-windows/

I've identified the silent install procedures for most of the required
tools (see the docs) and got build recipes written for some of the
library dependencies. The next planned step is to have the scripts
automatically download and silent-install Visual Studio, etc, rather
than have the user run the command lines given in the README manually.

It's usable as-is, I just need the time to finish it off. The goal is to
have a script that turns building PostgreSQL on a clean fresh Windows
install into:

- Download ActivePerl
- Install ActivePerl
- Run "buildgit.pl check install"

Right now it takes a fair bit more than that, but it's already better
than a fully manual build.

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

A local kvm instance works well if a physical host isn't available; so
do some of the alternative cloud providers like LunaCloud, which seems
to perform significantly better in the quick testing I did. I haven't
tried GoGrid yet.

Many of us have Windows license stickers on laptops/desktops, even if we
don't use the thing, so for a signficiant proportion of people it's as
simple as downloading Windows install media (
http://blog.ringerc.id.au/2012/05/you-can-download-legal-windows-7-iso.html)
and installing a KVM instance then shapshotting it.

I've also put together a Jenkins server that runs builds on Windows
whenever they're pushed to watched git repos. I'd love to make this
public, but unfortunately allowing a wide group to run arbitrary code on
the current build box isn't something I can afford. I'd need a system
that launched a snapshot Windows instance for each build and then
destroyed it at the end. This is entirely practical with something like
KVM, so if I ever get the chance to work on a Jenkins plugin to do that
(or to launch/destroy Windows cloud instances automatically), it's
possible a semi-public Windows build testing service may be possible.

While we're in fantasy land, the next step would be adding git URLs and
branch names to the commitfest app, so it could ping a continuous
integration server to build-test any new patch added to the CF. Right
now I'm doing that manually when I have time, but it's slow going.

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

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.

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

On Mon, Jan 21, 2013 at 1:00 PM, Craig Ringer <craig(at)2ndquadrant(dot)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

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

On Mon, Jan 21, 2013 at 1:59 PM, Andrew Dunstan <andrew(at)dunslane(dot)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

On Mon, Jan 21, 2013 at 3:03 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> On Mon, Jan 21, 2013 at 1:59 PM, Andrew Dunstan <andrew(at)dunslane(dot)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 Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

>> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii(at)postgresql(dot)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

>>> On Thu, Jan 17, 2013 at 7:43 PM, Tatsuo Ishii <ishii(at)postgresql(dot)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