Thread: incorrect docs for pgbench / skipped transactions

incorrect docs for pgbench / skipped transactions

From
Tomas Vondra
Date:
Hi,

while learning about format of the transaction log produced by pgbench,
I've noticed this sentence in the section describing format of the
per-transaction log:

    The last field skipped_transactions reports the number of
    transactions skipped because they were too far behind schedule.
    It is only present when both options --rate and --latency-limit
    are used.

Which is wrong, because this field is only added in the aggregated log,
not in the per-transaction one. So we should delete this.

It also does not explicitly explain that with --latency-limit the
latency column will contain "skipped" for transactions exceeding the
limit latency (it's only mentioned in the example output).

So I think we should apply the attached patch (and also backpatch it to
9.5, where the --latency-limit got introduced).

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:
Hello Tomas,

> while learning about format of the transaction log produced by pgbench, I've 
> noticed this sentence in the section describing format of the per-transaction 
> log:
>
>   The last field skipped_transactions reports the number of
>   transactions skipped because they were too far behind schedule.
>   It is only present when both options --rate and --latency-limit
>   are used.
>
> Which is wrong, because this field is only added in the aggregated log, not 
> in the per-transaction one. So we should delete this.

Indeed.

Several builtin scripts can be specified with -b as well, and -b and -f 
can be mixed, the file_no should be renamed script_no and refer to both -b 
and -f.

> It also does not explicitly explain that with --latency-limit the latency 
> column will contain "skipped" for transactions exceeding the limit latency 
> (it's only mentioned in the example output).

Ok.

> So I think we should apply the attached patch (and also backpatch it to 9.5, 
> where the --latency-limit got introduced).

Here is an updated version of your proposal: - use <literal> instead of " - use script_no and mention -b as well -
spellout skipped explanation after the sample output
 

Also, while reading the doc, I really think that the timestamp should be 
made explicit milliseconds, i.e. "123.004567" instead of "123 4567", but 
that is another question not relevant to this patch.

-- 
Fabien.

Re: incorrect docs for pgbench / skipped transactions

From
Tomas Vondra
Date:
Hi,

On 03/19/2016 08:29 AM, Fabien COELHO wrote:
>
...
>
> Here is an updated version of your proposal:
>  - use <literal> instead of "
>  - use script_no and mention -b as well
>  - spell out skipped explanation after the sample output

Seems fine to me.

>
> Also, while reading the doc, I really think that the timestamp should
> be made explicit milliseconds, i.e. "123.004567" instead of "123
> 4567", but that is another question not relevant to this patch.

Agreed.

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:
I've created an entry in the next commit fest so that it is not lost, but 
I hope it will be committed before then.

-- 
Fabien.



Re: incorrect docs for pgbench / skipped transactions

From
Alvaro Herrera
Date:
Fabien COELHO wrote:
> 
> I've created an entry in the next commit fest so that it is not lost, but I
> hope it will be committed before then.

If this problem was introduced by commits in 9.6, this issue should be
listed in the open items page,
https://wiki.postgresql.org/wiki/Open_Items

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:
>> I've created an entry in the next commit fest so that it is not lost, but I
>> hope it will be committed before then.
>
> If this problem was introduced by commits in 9.6, this issue should be
> listed in the open items page,
> https://wiki.postgresql.org/wiki/Open_Items

Thanks for the pointer. However, I do not have "editor priviledge" on this 
wiki, maybe Tomas has?

Now, the documentation issue Tomas reported is in 9.5 already and should 
be backported.

Another minor part of the patch is entirely 9.6 specific (script & -b 
reference).

Not sure how to proceed...

-- 
Fabien.



Re: incorrect docs for pgbench / skipped transactions

From
Alvaro Herrera
Date:
Fabien COELHO wrote:
> 
> >>I've created an entry in the next commit fest so that it is not lost, but I
> >>hope it will be committed before then.
> >
> >If this problem was introduced by commits in 9.6, this issue should be
> >listed in the open items page,
> >https://wiki.postgresql.org/wiki/Open_Items
> 
> Thanks for the pointer. However, I do not have "editor priviledge" on this
> wiki, maybe Tomas has?

I gave you editor privs now, but since it's in 9.5 I guess it needs to
be on the bug tracker (Except, of course, we don't have one.)

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:

>> Thanks for the pointer. However, I do not have "editor priviledge" on this
>> wiki, maybe Tomas has?
>
> I gave you editor privs now, but since it's in 9.5 I guess it needs to
> be on the bug tracker (Except, of course, we don't have one.)

Ok, I added a reference to the commitfest entry from this wiki page, and a 
note about partial 9.5 backporting.

-- 
Fabien.



Re: incorrect docs for pgbench / skipped transactions

From
Robert Haas
Date:
On Sat, Mar 19, 2016 at 3:28 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>>> Thanks for the pointer. However, I do not have "editor priviledge" on
>>> this
>>> wiki, maybe Tomas has?
>>
>> I gave you editor privs now, but since it's in 9.5 I guess it needs to
>> be on the bug tracker (Except, of course, we don't have one.)
>
> Ok, I added a reference to the commitfest entry from this wiki page, and a
> note about partial 9.5 backporting.

Please split the patch into one part for backporting and one part for
master-only and post both patches, clearly indicating which is which.

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



Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:
>> Ok, I added a reference to the commitfest entry from this wiki page, and a
>> note about partial 9.5 backporting.
>
> Please split the patch into one part for backporting and one part for
> master-only and post both patches, clearly indicating which is which.

Attached are the full patch for head and the backport part (the patch name 
ends with "backport") separated.

-- 
Fabien.

Re: incorrect docs for pgbench / skipped transactions

From
Robert Haas
Date:
On Mon, Mar 21, 2016 at 2:32 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>>> Ok, I added a reference to the commitfest entry from this wiki page, and
>>> a
>>> note about partial 9.5 backporting.
>>
>> Please split the patch into one part for backporting and one part for
>> master-only and post both patches, clearly indicating which is which.
>
> Attached are the full patch for head and the backport part (the patch name
> ends with "backport") separated.

That's not really what I wanted; the full page includes the stuff for
back-porting, whereas I wanted two independent patches.

I wordsmithed your version of Tomas's patch and pushed that to 9.5 and
master, and I pushed the first hunk of your full patch to master also.
I think the last hunk of that is overkill, so I did not push that.

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



Re: incorrect docs for pgbench / skipped transactions

From
Fabien COELHO
Date:

>>> Please split the patch into one part for backporting and one part for
>>> master-only and post both patches, clearly indicating which is which.
>>
>> Attached are the full patch for head and the backport part (the patch name
>> ends with "backport") separated.
>
> That's not really what I wanted; the full page includes the stuff for
> back-porting, whereas I wanted two independent patches.

Indeed, I misunderstood the requirement.

> I wordsmithed your version of Tomas's patch and pushed that to 9.5 and
> master, and I pushed the first hunk of your full patch to master also.
> I think the last hunk of that is overkill, so I did not push that.

Ok.

-- 
Fabien.