+ * The function behaviors changes depending on sample_rate (a fraction of
+ * transaction is reported) and agg_interval (transactions are aggregated
+ * over the interval and reported once).
The first part of this sentence has an incorrect grammar.
Indeed. v5 attempts to improve comments.
On Wed, Jun 16, 2021 at 08:58:17AM +0200, Fabien COELHO wrote:
> Actually it would work if both are mixed: the code would aggregate a sample.
> However it does not look very useful to do that, so it is arbitrary
> forbidden. Not sure whether this is that useful to prevent this use case.
Okay,
Michaël-san, Yugo-san,
I am fine with this version, but I think it would be better if we have
a comment explaining what "tx" is for.
Yes. Done.
Also, how about adding Assert(tx) instead of using "else if (tx)" because
we are assuming that tx is always true when agg_interval is not used,
On Tue, Jun 15, 2021 at 09:53:06PM +0900, Yugo NAGATA wrote:
> I am fine with this version, but I think it would be better if we have a
> comment
> explaining what "tx" is for.
>
> Also, how about adding Assert(tx) instead of using "else if (tx)" because
> we are assuming that tx is always true
On Tue, 15 Jun 2021 11:38:00 +0200 (CEST)
Fabien COELHO wrote:
>
> > Attached a v3 which adds a boolean to distinguish recording vs flushing.
I am fine with this version, but I think it would be better if we have a comment
explaining what "tx" is for.
Also, how about adding Assert(tx) instead
On Tue, 15 Jun 2021 18:01:18 +0900
Michael Paquier wrote:
> On Tue, Jun 15, 2021 at 05:15:14PM +0900, Yugo NAGATA wrote:
> > On Tue, 15 Jun 2021 10:05:29 +0200 (CEST) Fabien COELHO
> > wrote:
> > It was not a problem because --sampling-rate --aggregate-interval cannot be
> > used at the same
Attached a v3 which adds a boolean to distinguish recording vs flushing.
Better with the attachement… sorry for the noise.
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d7479925cb..3b27ffebf8 100644
--- a/src/bin/pgbench/pgbench.c
+++
On Tue, Jun 15, 2021 at 05:15:14PM +0900, Yugo NAGATA wrote:
> On Tue, 15 Jun 2021 10:05:29 +0200 (CEST) Fabien COELHO
> wrote:
> It was not a problem because --sampling-rate --aggregate-interval cannot be
> used at the same time.
Yep, you are right, thanks. I have missed that both options
On Tue, 15 Jun 2021 10:05:29 +0200 (CEST)
Fabien COELHO wrote:
>
> Hello Michaël,
>
> >> I think we don't have to call doLog() before logAgg(). If we call doLog(),
> >> we will count an extra transaction that is not actually processed because
> >> accumStats() is called in this.
> >
> > Yes,
Hello Michaël,
I think we don't have to call doLog() before logAgg(). If we call doLog(),
we will count an extra transaction that is not actually processed because
accumStats() is called in this.
Yes, calling both is weird.
The motivation to call doLog is to catch up zeros on slow rates,
On Sun, Jun 13, 2021 at 03:07:51AM +0900, Yugo NAGATA wrote:
> It will not work if the transaction is skipped, in which case latency is 0.0.
> It would work if we check also "skipped" as bellow.
>
> + if (!logged && !skipped && latency == 0.0)
>
> However, it still might not work if
>> + while ((next = agg->start_time + agg_interval * INT64CONST(100))
>> <= now)
>>
>> I can find the similar code to convert "seconds" to "us" using casting
>> like
>>
>> end_time = threads[0].create_time + (int64) 100 * duration;
>>
>> or
>>
>> next_report = last_report + (int64) 100
+ while ((next = agg->start_time + agg_interval *
INT64CONST(100)) <= now)
I can find the similar code to convert "seconds" to "us" using casting like
end_time = threads[0].create_time + (int64) 100 * duration;
or
next_report = last_report + (int64) 100 *
On Thu, 10 Jun 2021 23:29:30 +0200 (CEST)
Fabien COELHO wrote:
>
> Bonjour Michaël,
>
> Here is an updated patch. While having a look at Kyotaro-san patch, I
> noticed that the aggregate stuff did not print the last aggregate. I think
> that it is a side effect of switching the precision
On Fri, 11 Jun 2021 16:09:10 +0200 (CEST)
Fabien COELHO wrote:
>
> Bonjour Michaël,
>
> >> + /* flush remaining stats */
> >> + if (!logged && latency == 0.0)
> >> + logAgg(logfile, agg);
> >
> > You are right, this is missing the final stats. Why the choice
Bonjour Michaël,
+ /* flush remaining stats */
+ if (!logged && latency == 0.0)
+ logAgg(logfile, agg);
You are right, this is missing the final stats. Why the choice of
latency here for the check?
For me zero latency really says that
At Fri, 11 Jun 2021 15:56:55 +0900 (JST), Kyotaro Horiguchi
wrote in
> Doesn't threadRun already doing that?
(s/Does/Is)
That's once per thread, sorry for the noise.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Fri, 11 Jun 2021 15:23:41 +0900, Michael Paquier wrote
in
> On Thu, Jun 10, 2021 at 11:29:30PM +0200, Fabien COELHO wrote:
> > + /* flush remaining stats */
> > + if (!logged && latency == 0.0)
> > + logAgg(logfile, agg);
>
> You are right, this is
On Thu, Jun 10, 2021 at 11:29:30PM +0200, Fabien COELHO wrote:
> + /* flush remaining stats */
> + if (!logged && latency == 0.0)
> + logAgg(logfile, agg);
You are right, this is missing the final stats. Why the choice of
latency here for the check?
Bonjour Michaël,
Here is an updated patch. While having a look at Kyotaro-san patch, I
noticed that the aggregate stuff did not print the last aggregate. I think
that it is a side effect of switching the precision from per-second to
per-µs. I've done an attempt at also fixing that which
Hello Michael,
The cause is that the time unit is changed to usec but the patch
forgot to convert agg_interval into the same unit in doLog. I tempted
to change it into pg_time_usec_t but it seems that it is better that
the unit is same with other similar variables like duration.
As the
On Tue, Jun 08, 2021 at 06:59:04PM +0900, Kyotaro Horiguchi wrote:
> The cause is that the time unit is changed to usec but the patch
> forgot to convert agg_interval into the same unit in doLog. I tempted
> to change it into pg_time_usec_t but it seems that it is better that
> the unit is same
At Tue, 8 Jun 2021 12:09:47 +0900, YoungHwan Joo wrote in
> Hello!
>
> While I was using pgbench from the master branch, I discovered an error on
> pgbench logs.
> When I run pgbench, the log file contains a lot of redundant 0s.
>
> I ran git bisect and found out that thi
23 matches
Mail list logo
