Re: Backup throttling

Antonin Houska wrote:

> +       <para>
> +        Suffixes <literal>k</literal> (kilobytes) and <literal>m</literal>
> +        (megabytes) are accepted. For example: <literal>10m</literal>
> +       </para>

"m" is for meters, or milli.  Please use "M" here.

> +static uint32
> +parse_max_rate(char *src)
> +{
> +	int			factor;
> +	char	   *after_num;
> +	int64		result;
> +	int	errno_copy;
> +
> +	result = strtol(src, &after_num, 0);
> +	errno_copy = errno;
> +	if (src == after_num)
> +	{
> +		fprintf(stderr, _("%s: transfer rate %s is not a valid integer value\n"), progname, src);
> +		exit(1);
> +	}

Please add quotes to the invalid value.

> +
> +	/*
> +	 * Evaluate (optional) suffix.
> +	 *
> +	 * after_num should now be right behind the numeric value.
> +	 */
> +	factor = 1;
> +	switch (tolower(*after_num))
> +	{
> +			/*
> +			 * Only the following suffixes are allowed. It's not too useful to
> +			 * restrict the rate to gigabytes: such a rate will probably bring
> +			 * significant impact on the master anyway, so the throttling
> +			 * won't help much.
> +			 */
> +		case 'g':
> +			factor <<= 10;

I don't understand why you allow a 'g' here, given the comment above ...
but in any case it should be G.

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