Hi Fabien,
On Sun, Aug 24, 2014 at 9:16 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> Find attached a new version:
> - fix dropped percent computation in the final report
> - simplify progress report code
I have reviewed this patch.
Is the patch in a patch format which has context? Yes.
Does it apply cleanly to the current git master? Yes.
Does it include reasonable tests, necessary doc patches, etc? Yes.
Does the patch actually implement that? Yes.
Do we want that? I think we do, yes.
Do we already have it? No.
Are there dangers? None that I can see.
Does the feature work as advertised? Almost, see below.
Are there corner cases the author has failed to consider? None that I can see.
Are there any assertion failures or crashes? No.
I can't make the -L option work at all. If I do this: ./pgbench -R 100 -L 1
I get: pgbench: invalid option -- L
Which appears to be caused by the fact that the call to getopt_long()
has not been updated to reflect the new parameter.
Also this part:
+ " -L, --limit=NUM under throttling (--rate), skip
transactions that\n"
+ " far behind schedule in ms
(default: do not skip)\n"
I would suggest rewording this to something like "skip transactions
that are more than NUM milliseconds behind schedule (default: do not
skip)".
Marking Waiting for Author until these small issues have been fixed.
Thanks,
♜