On 09/26/2011 05:58 AM, Shigeru Hanada wrote:
> * Local variables added by the patch (secs, usecs, write_rate and
> endtime) can be moved into narrower scope.
> * Initializing starttime to zero seems unnecessary.
>
Setting starttime to 0 is already in the code; the change made to that
line was to add endtime, which is not initialized. You may be right that
initializing it isn't necessary, but I'm sure not going to touch that
part of the working code.
You're right about the the local variables; they were placed to look
like the surrounding code rather than to be as local as possible. I'm
not sure if all the PostgreSQL code is completely consistent here; a
quick survey shows examples of both "put all the variables at the top"
and "make variables as local to blocks as possible" styles. I don't know
that it really makes any difference with modern compilers, either. I'm
sure someone else will have a stronger opinion on this subject now that
I've trolled the list for one by writing this.
> In addition, documents about how to use the statistics would be
> necessary, maybe in "23.1.5. The Autovacuum Daemon".
> I'll set the status of this patch to waiting-on-author. Would you rebase
> the patch and post it again?
>
I didn't do that because there's not really much documentation at this
level of detail yet--how to interpret all the information in the logs.
That's an open-ended bit of work; there's a lot more that could be
written on this topic than the docs have right now. It's probably worth
pointing out that this specific info is now in the logs, though, given
that people might not notice it otherwise. I'll see what I can do about
that.
As a general FYI, rebasing is normally requested only when the existing
patch doesn't apply anymore. If "patch" or "git apply" can consume it,
complaints about code shifting around isn't by itself enough reason to
ask for an updated patch.
--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
