Re: [HACKERS] pgbench regression test failure

[Robert Haas]

On Tue, Nov 21, 2017 at 3:28 PM, Tom Lane  wrote:
> Seems like a good idea, but the way you've written it is inconsistent
> with the "n/m" notation used just above.  I'd suggest
>
> ... latency limit: 33 (33/33, 100.000 %)
>
> or just
>
> ... latency limit: 33/33 (100.000 %)

Oh, yeah.  That last one sounds good; no reason to print the same
value more than once.

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

Re: [HACKERS] pgbench regression test failure

[Tom Lane]

Robert Haas  writes:
> On Mon, Nov 20, 2017 at 1:40 PM, Tom Lane  wrote:
>> I dunno, it just looks odd to me that when we've set up a test case in
>> which every one of the transactions is guaranteed to exceed the latency
>> limit, that it doesn't say that they all did.  I don't particularly buy
>> your assumption that the percentages should sum.  Anybody else have an
>> opinion there?

> I agree with you, but I don't think either approach is free from
> possible confusion.  I think it would help to show the numerator and
> the denominator explicitly, e.g.:

> number of clients: 1
> number of threads: 1
> number of transactions per client: 100
> number of transactions actually processed: 33/100
> number of transactions skipped: 67 (67.000 %)
> number of transactions above the 1.0 ms latency limit: 33 (33 of 33, 100.000 
> %)

> (My proposed change is in the last line.)

Seems like a good idea, but the way you've written it is inconsistent
with the "n/m" notation used just above.  I'd suggest

... latency limit: 33 (33/33, 100.000 %)

or just

... latency limit: 33/33 (100.000 %)

regards, tom lane

Re: [HACKERS] pgbench regression test failure

[Fabien COELHO]

Hello Tom,


2. ISTM that we should report that 100% of the transactions were
above the latency limit, not 33%; that is, the appropriate base
for the "number of transactions above the latency limit" percentage
is the number of actual transactions not the number of scheduled
transactions.



Hmmm. Allow me to disagree.


I dunno, it just looks odd to me that when we've set up a test case in
which every one of the transactions is guaranteed to exceed the latency
limit, that it doesn't say that they all did.  I don't particularly buy
your assumption that the percentages should sum.


This is a side effect. The reason for me is that the user asked for some 
transactions, and the results should be given relative to what was asked.



Anybody else have an opinion there?


Good question.


I also noticed that if I specify "-f sleep-100.sql" more than once,
the per-script TPS reports are out of line.  This is evidently because
that calculation isn't excluding skipped xacts; but if we're going to
define tps as excluding skipped xacts, surely we should do so there too.



I do not think that we should exclude skipped xacts.


Uh ... why not?


Because I totally misinterpreted your sentence.

Indeed, the skipped transactions needs to be substracted from the count.
This is yet another bug.


but I find that unduly optimistic.  It should really read more like
"if no transactions were executed, at best we'll get some platform-
dependent spelling of NaN.  At worst we'll get a SIGFPE."



Hmmm. Alas you must be right about spelling. There has been no report of
SIGFPE issue, so I would not bother with that.


The core issue here really is that you're assuming IEEE float arithmetic.
We have not gone as far as deciding that Postgres will only run on IEEE
hardware, and I don't want to start in pgbench, especially not in
seldom-exercised corner cases.


Hmmm. It has already started for some years without complaint. Do as you 
feel about NaN. I can only say that I do not like much having zero to 
stand for undefined.


--
Fabien.

Re: [HACKERS] pgbench regression test failure

[Tom Lane]

Fabien COELHO  writes:
>> 1. The per-script stats shouldn't be printed at all if there's
>> only one script.  They're redundant with the overall stats.

> Indeed.
> I think the output should tend to be the same for possible automatic 
> processing, whether there is one script or more, even at the price of some 
> redundancy.
> Obviously this is highly debatable.

I think that ship has already sailed.  It's certainly silly that the
code prints *only* per-script latency stats, and not all the per-script
stats, when there's just one script.  To me the answer is to make the
latency stats conform to the rest, not make the printout even more
redundant.  None of this output was designed for machine-friendliness.

(Maybe there is an argument for a "--machine-readable-output" switch
that would dump the data in some more-machine-friendly format.  Though
I'm sure we'd have lots of debates about exactly what that is...)

>> 2. ISTM that we should report that 100% of the transactions were
>> above the latency limit, not 33%; that is, the appropriate base
>> for the "number of transactions above the latency limit" percentage
>> is the number of actual transactions not the number of scheduled
>> transactions.

> Hmmm. Allow me to disagree.

I dunno, it just looks odd to me that when we've set up a test case in
which every one of the transactions is guaranteed to exceed the latency
limit, that it doesn't say that they all did.  I don't particularly buy
your assumption that the percentages should sum.  Anybody else have an
opinion there?

>> I also noticed that if I specify "-f sleep-100.sql" more than once,
>> the per-script TPS reports are out of line.  This is evidently because
>> that calculation isn't excluding skipped xacts; but if we're going to
>> define tps as excluding skipped xacts, surely we should do so there too.

> I do not think that we should exclude skipped xacts.

Uh ... why not?

>> I'm also still exceedingly unhappy about the NaN business.
>> You have this comment in printSimpleStats:
>> /* print NaN if no transactions where executed */
>> but I find that unduly optimistic.  It should really read more like
>> "if no transactions were executed, at best we'll get some platform-
>> dependent spelling of NaN.  At worst we'll get a SIGFPE."

> Hmmm. Alas you must be right about spelling. There has been no report of 
> SIGFPE issue, so I would not bother with that.

The core issue here really is that you're assuming IEEE float arithmetic.
We have not gone as far as deciding that Postgres will only run on IEEE
hardware, and I don't want to start in pgbench, especially not in
seldom-exercised corner cases.

regards, tom lane