Hello I did review of pgbench patch http://archives.postgresql.org/pgsql-hackers/2012-09/msg00737.php
* this patch is cleanly patched * I had a problem with doc make[1]: Entering directory `/home/pavel/src/postgresql/doc/src/sgml' openjade -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . -c /usr/share/sgml/docbook/dsssl-stylesheets/catalog -d stylesheet.dsl -t sgml -i output-html -V html-index postgres.sgml openjade:pgbench.sgml:767:8:E: document type does not allow element "TITLE" here; missing one of "ABSTRACT", "AUTHORBLURB", "MSGSET", "CALLOUTLIST", "ITEMIZEDLIST", "ORDEREDLIST", "SEGMENTEDLIST", "VARIABLELIST", "CAUTION", "IMPORTANT", "NOTE", "TIP", "WARNING", "FORMALPARA", "BLOCKQUOTE", "EQUATION", "EXAMPLE", "FIGURE", "TABLE", "PROCEDURE", "SIDEBAR", "QANDASET", "REFSECT3" start-tag make[1]: *** [HTML.index] Error 1 make[1]: *** Deleting file `HTML.index' make[1]: Leaving directory `/home/pavel/src/postgresql/doc/src/sgml * pgbench is compiled without warnings * there is a few issues in source code + + /* should we aggregate the results or not? */ + if (use_log_agg) + { + + /* are we still in the same interval? if yes, accumulate the + * values (print them otherwise) */ + if (agg->start_time + use_log_agg >= INSTR_TIME_GET_DOUBLE(now)) + { + * a real time range for aggregation is dynamic (pgbench is not realtime application), so I am not sure if following constraint has sense + if ((duration > 0) && (use_log_agg > 0) && (duration % use_log_agg != 0)) { + fprintf(stderr, "duration (%d) must be a multiple of aggregation interval (%d)\n", duration, use_log_agg); + exit(1); + } * it doesn't flush last aggregated data (it is mentioned by Tomas: "Also, I've realized the last interval may not be logged at all - I'll take a look into this in the next version of the patch."). And it can be significant for higher values of "use_log_agg" * a name of variable "use_log_agg" is not good - maybe "log_agg_interval" ? Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers