Re: Count backend self-sync calls

Robert Haas wrote:
> I think this one could be removed:
>
> +    if (n > 0)
> +        elog(DEBUG1,"Absorbing %d fsync requests",n);
>   

Right; that one is just there to let you know the patch is working, and 
how much the background writer does for you per pass, mainly for the 
purpose of reviewing the patch.


> But if this is generating a lot of log data or adding a lot of
> overhead, then you have bigger problems anyway:
>
> +        elog(DEBUG1, "Unable to forward fsync request, executing directly");
>   

The argument against this log line even existing is that if the field is 
added to pg_stat_bgwriter, that's probably how you want to monitor this 
data anyway.  I don't think there's actually much value to it, which is 
one reason I didn't worry too much about matching style guidelines, 
translation, etc.  You've found the two things that I think both need to 
go away before commit, but I left them in because I think they're 
valuable for testing the patch itself does what it's supposed to.

> Also, how about referring to this as buffers_backend_fsync
> consistently throughout, instead of dropping the "f" in some places?
>   

I started out touching code that called it just "sync", but then crossed 
to other code that called it "fsync", and made the external UI use that 
name.  No objections to sorting that out within my patch so it's consistent.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services and Support        www.2ndQuadrant.us