Thread: incorrect docs for pgbench / skipped transactions
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
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.
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
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.
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
>> 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.
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
>> 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.
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
>> 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.
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
>>> 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.