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
Attachments:
pgbench-skipped-doc-fix.patchbinary/octet-stream; name=pgbench-skipped-doc-fix.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index cc80b3f..3155a3a 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1089,10 +1089,8 @@ END;
Field <replaceable>schedule_lag</> is the difference between the
transaction's scheduled start time, and the time it actually started, in
microseconds. It is only present when the <option>--rate</> option is used.
- The last field <replaceable>skipped_transactions</> reports the number of
- transactions skipped because they were too far behind schedule. It is only
- present when both options <option>--rate</> and <option>--latency-limit</>
- are used.
+ When both options <option>--rate</> and <option>--latency-limit</> are used,
+ the skipped transactions will use "skipped" in place of latency.
</para>
<para>
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
- spell out 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.
Attachments:
pgbench-skipped-doc-fix-2.patchtext/x-diff; name=pgbench-skipped-doc-fix-2.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index cc80b3f..1a4c1f4 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1075,12 +1075,12 @@ END;
The format of the log is:
<synopsis>
-<replaceable>client_id</> <replaceable>transaction_no</> <replaceable>time</> <replaceable>file_no</> <replaceable>time_epoch</> <replaceable>time_us</> <optional><replaceable>schedule_lag</replaceable></optional>
+<replaceable>client_id</> <replaceable>transaction_no</> <replaceable>time</> <replaceable>script_no</> <replaceable>time_epoch</> <replaceable>time_us</> <optional><replaceable>schedule_lag</replaceable></optional>
</synopsis>
where <replaceable>time</> is the total elapsed transaction time in microseconds,
- <replaceable>file_no</> identifies which script file was used
- (useful when multiple scripts were specified with <option>-f</>),
+ <replaceable>script_no</> identifies which script file was used (useful when
+ multiple scripts were specified with <option>-f</> or <option>-b</>),
and <replaceable>time_epoch</>/<replaceable>time_us</> are a
Unix epoch format time stamp and an offset
in microseconds (suitable for creating an ISO 8601
@@ -1089,10 +1089,8 @@ END;
Field <replaceable>schedule_lag</> is the difference between the
transaction's scheduled start time, and the time it actually started, in
microseconds. It is only present when the <option>--rate</> option is used.
- The last field <replaceable>skipped_transactions</> reports the number of
- transactions skipped because they were too far behind schedule. It is only
- present when both options <option>--rate</> and <option>--latency-limit</>
- are used.
+ When both options <option>--rate</> and <option>--latency-limit</> are used,
+ the skipped transactions will use <literal>skipped</> in place of latency.
</para>
<para>
@@ -1117,7 +1115,8 @@ END;
</screen>
In this example, transaction 82 was late, because it's latency (6.173 ms) was
over the 5 ms limit. The next two transactions were skipped, because they
- were already late before they were even started.
+ were already late before they were even started (5217 and 5099
+ microsecond schedule lags on the last column are above the 5 ms limit).
</para>
<para>
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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_ItemsThanks 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
Attachments:
pgbench-skipped-doc-fix-2.patchtext/x-diff; name=pgbench-skipped-doc-fix-2.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index cc80b3f..1a4c1f4 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1075,12 +1075,12 @@ END;
The format of the log is:
<synopsis>
-<replaceable>client_id</> <replaceable>transaction_no</> <replaceable>time</> <replaceable>file_no</> <replaceable>time_epoch</> <replaceable>time_us</> <optional><replaceable>schedule_lag</replaceable></optional>
+<replaceable>client_id</> <replaceable>transaction_no</> <replaceable>time</> <replaceable>script_no</> <replaceable>time_epoch</> <replaceable>time_us</> <optional><replaceable>schedule_lag</replaceable></optional>
</synopsis>
where <replaceable>time</> is the total elapsed transaction time in microseconds,
- <replaceable>file_no</> identifies which script file was used
- (useful when multiple scripts were specified with <option>-f</>),
+ <replaceable>script_no</> identifies which script file was used (useful when
+ multiple scripts were specified with <option>-f</> or <option>-b</>),
and <replaceable>time_epoch</>/<replaceable>time_us</> are a
Unix epoch format time stamp and an offset
in microseconds (suitable for creating an ISO 8601
@@ -1089,10 +1089,8 @@ END;
Field <replaceable>schedule_lag</> is the difference between the
transaction's scheduled start time, and the time it actually started, in
microseconds. It is only present when the <option>--rate</> option is used.
- The last field <replaceable>skipped_transactions</> reports the number of
- transactions skipped because they were too far behind schedule. It is only
- present when both options <option>--rate</> and <option>--latency-limit</>
- are used.
+ When both options <option>--rate</> and <option>--latency-limit</> are used,
+ the skipped transactions will use <literal>skipped</> in place of latency.
</para>
<para>
@@ -1117,7 +1115,8 @@ END;
</screen>
In this example, transaction 82 was late, because it's latency (6.173 ms) was
over the 5 ms limit. The next two transactions were skipped, because they
- were already late before they were even started.
+ were already late before they were even started (5217 and 5099
+ microsecond schedule lags on the last column are above the 5 ms limit).
</para>
<para>
pgbench-skipped-doc-fix-2-backport.patchtext/x-diff; name=pgbench-skipped-doc-fix-2-backport.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index c6d1454..e82db70 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1112,10 +1112,8 @@ END;
Field <replaceable>schedule_lag</> is the difference between the
transaction's scheduled start time, and the time it actually started, in
microseconds. It is only present when the <option>--rate</> option is used.
- The last field <replaceable>skipped_transactions</> reports the number of
- transactions skipped because they were too far behind schedule. It is only
- present when both options <option>--rate</> and <option>--latency-limit</>
- are used.
+ When both options <option>--rate</> and <option>--latency-limit</> are used,
+ the skipped transactions will use <literal>skipped</> in place of latency.
</para>
<para>
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers