Re: Backup throttling

On Wed, 31 Jul 2013 22:50:19 +0200
Antonin Houska <antonin(dot)houska(at)gmail(dot)com> wrote:

> On 07/31/2013 07:13 AM, Gibheer wrote:
> > Hi,
> >
> > That is a really nice feature.
> I don't pretend it's my idea, I just coded it. My boss proposed the
> feature as such :-)
> > I took a first look at your patch and some empty lines you added
> > (e.g. line 60 your patch). Can you remove them?
> Sure, will do in the next version.
> > Why did you move localGetCurrentTimestamp() into streamutil.c? Is
> > sys/time.h still needed in receivelog.c after the move?
> Because both receivelog.c and pg_basebackup.c need it now. I thought
> I could move localTimestampDifference() and
> localTimestampDifferenceExceeds() as well for the sake of consistency
> (these are actually utilities too) but I didn't get convinced enough
> that the feature alone justifies such a change.
>
> As mentioned in
> http://www.postgresql.org/message-id/20130731173624.GX14652@eldon.alvh.no-ip.org
> these functions ideally shouldn't have separate implementation at
> all. However the problem is that pg_basebackup is not linked to the
> backend.
>
> You're right about sys/time.h, it's included via via streamutil.h.
> I'll fix that too.
> > I will try your patch later today to see, if it works.
> >
> Whenever you have time. Thanks!
>
> // Tony

Hi,

I got around to test your patch and it works. I've build a small script
for others to test it easily. You can tell it with ROOTDIR and BASEDIR
environment variables where to look for the binaries and where to put
the test directories.

There is only one small thing I hit, namely the error message when the
limit is too small. It was like

transfer rate out of range '10k'

but it does not mention what the actual range is.

Maybe a message like

transfer rate of 10k is too small. Lower limit is 32k.

or

transfer rate has to be between 32k and 1GB, was 10k.

would be better as they mention what the actual limit is. That avoids
that people have to look up the limits in the manual.
We should also add the current limits to the documentation.

regards,

Stefan Radomski