On Tue, Nov 30, 2010 at 12:15 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Tue, Nov 30, 2010 at 8:38 AM, Greg Smith <greg@2ndquadrant.com> wrote:
>
>
> Hi Greg,
>
> Thanks for the update.
>
>
>
>> This might be ready for some proper review now. I know there's at least one
>> blatant bug still in here I haven't found yet, related to how the averages
>> are computed.
>
> Yes, the blatant bug:
>
> average_sync_time = CheckpointStats.ckpt_longest_sync /
> CheckpointStats.ckpt_sync_rels;
>
> That should clearly be ckpt_agg_sync_time, not ckpt_longest_sync.
I've attached a tiny patch to apply over yours, to deal with this and
with the case where no files are synced.
Combining this instrumentation patch with the backend sync one already
committed, the net result under log_min_messages=debug1is somewhat
undesirable in that I can now see the individual sync times for the
syncs done by the checkpoint writer, but I do not get times for the
syncs done by the backend (I only get informed of their existence).
I don't know what I would propose to fix this. Having the reportage
of sync time of backend syncs be controlled by log_checkpoints seems
somewhat perverse, but the only alternative I see is to have
log_min_messages=debug1 always report times for the backend syncs. Or
to have them go unreported altogether.
Cheers,
Jeff