Re: Backup throttling

Antonin Houska escribió:

> > Why did you choose "bytes per second" as a valid rate which we can specify?
> > Since the minimum rate is 32kB, isn't it better to use "KB per second" for that?
> > If we do that, we can easily increase the maximum rate from 1GB to very large
> > number in the future if required.
> 
> The attached version addresses all the comments above.

I pushed this patch with a few further tweaks.  In your changes to
address the above point, you made the suffix mandatory in the
pg_basebackup -r option.  This seemed a strange restriction, so I
removed it.  It seems more user-friendly to me to accept the value as
being expressed in kilobytes per second without requiring the suffix to
be there; the 'k' suffix is then also accepted and has no effect.  I
amended the docs to say that also.

If you or others feel strongly about this, we can still tweak it, of
course.

I also moved the min/max #defines to replication/basebackup.h, and
included that file in pg_basebackup.c.  This avoids the duplicated
values.  That file is okay to be included there.

> > If WL_POSTMASTER_DEATH is triggered, we should exit immediately like
> > other process does? This is not a problem of this patch. This problem exists
> > also in current master. But ISTM it's better to solve that together. Thought?
> 
> Once we're careful about not missing signals, I think PM death should be
> noticed too. The backup functionality itself would probably manage to
> finish without postmaster, however it's executed under walsender process.
> 
> Question is where !PostmasterIsAlive() check should be added. I think it
> should go to the main loop of perform_base_backup(), but that's probably
> not in the scope of this patch.

Feel free to submit patches about this.

Thanks for your patch, and the numerous reviewers who took part.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services