On Jul29, 2010, at 00:48 , Greg Smith wrote: > Finally got around to taking a longer look at your patch, sorry about the > delay here. Patch itself seems to work on simple tests anyway (more on the > one suspect bit below). You didn't show what the output looks like, so let's > start with that because it is both kind of neat and not what I expected from > your description. Here's the sort of extra stuff added to the end of the > standard output when you toggle this feature on: > > <snipped output> > > From the way you described the patch, I had thought that you were just > putting this information into the log files or something like that. In fact, > it's not in the log files; it just shows up in this summary at the end. I'm > not sure if that's best or not. Some might want to see how the per-statement > variation varies over time. Sort of torn on whether the summary alone is > enough detail or not. Let me play with this some more and get back to you on > that.
I think the two features are pretty much orthogonal, even though they'd make use of the same per-statement instrumentation machinery. I created the patch to tune the wal_writer for the synchronous_commit=off case - the idea being that the COMMIT should be virtually instantaneous if the wal_writer writes old wal buffers out fast enough. I haven't yet used pgbench's log output feature, so I can't judge how useful the additional of per-statement data to that log is, and what the format should be. However, if you think it's useful and can come up with a sensible format, I'd be happy to add that feature to the patch. > Now onto the nitpicking. With the standard Ubuntu compiler warnings on I get > this: > > pgbench.c:1588: warning: ‘i’ may be used uninitialized in this function > > If you didn't see that, you may want to double-check how verbose you have > your compiler setup to be; maybe you just missed it (which is of course what > reviewers are here for). Regardless, the troublesome bit is this: > > int i; > > commands = process_commands(&buf[i]); Fixed. That was a leftover of the trimming and comment skipping logic, which my patch moves to process_command. Updated patch is attached. Thanks for your extensive review & best regards, Florian Pflug
pgbench_statementlatency.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
