From: | Tomas Vondra <tv(at)fuzzy(dot)cz> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: review: pgbench - aggregation of info written into log |
Date: | 2012-12-08 16:16:47 |
Message-ID: | 50C367EF.60405@fuzzy.cz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2012-12-08 16:34:08 | Re: CommitFest 2012-11 Progress |
Previous Message | Andrew Dunstan | 2012-12-08 16:13:30 | Re: parallel pg_dump |