"Brendan Jurd" <[EMAIL PROTECTED]> wrote: > Josh assigned your patch to me for an initial review. Here's what I > have so far.
Thank your for reviewing! > The -T option seems to work as advertised, and I wasn't able to detect > any performance degradation (or a significant variation of any kind) > using the -T option versus the -t option. Good news. > Unfortunately, I don't have access to a Windows build environment, so > I wasn't able to investigate the portability concerns raised in the > email thread. I found I have a mistake in the usage of BOOL and BOOLEAN types. I'll fix it in the next patch. > I do have a few minor suggestions: > > * The error message given for -T <= 0 is "invalid number of > duration(-T): %d". The grammar isn't quite right there. I would go > with "invalid run duration (-T): %d", or perhaps just "invalid > duration (-T): %d". "invalid duration (-T): %d" looks good for me. > * If the -T and -t options are supposed to be mutually incompatible, > then there should be an error if the user tries to specify both > options. Currently, if I specify both options, the -t option is > ignored in favour of -T. An error along the lines of "Specify either > a number of transactions (-t) or a duration (-T), not both." would be > nice. Ok, I'll add the error protection. > * It seems like the code to stop pgbench when the timer has expired > has some unnecessary duplication. Ah, I forgot to clean up the codes. I'll fix it. (There was another logic here in the first patch, but not needed in the version.) > * The documentation should mention the new -T option in the following > paragraph: > In the first place, never believe any test that runs for only a few > seconds. Use the -t or -T option to make the run last at least a few > minutes, so as to average out noise. I'll do it. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
