Re: [PATCH] pg_basebackup: progress report max once per second

On Mon, Feb 3, 2014 at 4:14 AM, Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com>wrote:

> On Sat, Feb 1, 2014 at 8:29 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
> > 31.01.2014 10:59, Sawada Masahiko kirjoitti:
> >
> > I think the idea in the new progress_report() call (with force == true)
> is
> > to make sure that there is at least one progress_report call that
> actually
> > writes the progress report. Otherwise the final report may go missing
> if it
> > gets suppressed by the time-based check. The force argument as used in
> the
> > new call skips that check.
> >
>
> I understood.
>
> I have two concerns as follows.
> - I think that there is possible that progress_report() is called
> frequently ( less than 1 second).
> That is, progress_report() is called with force == true after
> progress_report was called with force == false and execute this
> function.
> - progress_report() is called even if -P option is disabled. I'm
> concerned about that is cause of performance degradation.
>

I looked over the latest version, and the only real problem I see here is
your second point, which is the calling with -P not specified. I doubt it's
going to be much, but in theory I guess the call to time(NULL) many times
could have an effect. I've fixed that by just moving it to after a check
for showprogress.

As for the first one - I believe that's the point. progress_report should
be called with force==true after it was called with it false, that's the
intended design.

I've applied the patch, with that minor adjustment and an added comment.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/