Re: Should we increase the default vacuum_cost_limit? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Should we increase the default vacuum_cost_limit?
Date
Msg-id 1798.1552165479@sss.pgh.pa.us
Whole thread Raw
In response to Re: Should we increase the default vacuum_cost_limit?  (Julien Rouhaud <rjuju123@gmail.com>)
Responses Re: Should we increase the default vacuum_cost_limit?
Re: Should we increase the default vacuum_cost_limit?
List pgsql-hackers
Julien Rouhaud <rjuju123@gmail.com> writes:
> On Sat, Mar 9, 2019 at 7:58 PM Andrew Dunstan
> <andrew.dunstan@2ndquadrant.com> wrote:
>> On 3/9/19 12:55 PM, Tom Lane wrote:
>>> The idea of converting vacuum_cost_delay into a float variable, while
>>> keeping its native unit as ms, seems probably more feasible from a
>>> compatibility standpoint.  There are two sub-possibilities:
>>> ...
>>> 2. Add support for units for float variables, too.  I don't think
>>> this'd be a huge amount of work, and we'd surely have other uses
>>> for it in the long run.
>>> ...
>>> I'm inclined to go look into #2.  Anybody think this is a bad idea?

>> Sounds good to me, seems much more likely to be future-proof.

> Agreed.

I tried this, and it seems to work pretty well.  The first of the two
attached patches just teaches guc.c to support units for float values,
incidentally allowing "us" as an input unit for time-based GUCs.
The second converts [autovacuum_]cost_delay to float GUCs, and changes
the default value for autovacuum_cost_delay from 20ms to 2ms.
We'd want to revert the previous patch that changed the default value
of vacuum_cost_limit, else this means a 100x not 10x change in the
default autovac speed ... but I've not included that in this patch.

Some notes:

1. I hadn't quite absorbed until doing this that we'd need a catversion
bump because of format change in StdRdOptions.  Since this isn't proposed
for back-patching, that doesn't seem problematic.

2. It's always bugged me that we don't allow fractional unit
specifications, say "0.1GB", even for GUCs that are integers underneath.
That would be a simple additional change on top of this, but I didn't
do it here.

3. I noticed that parse_real doesn't reject infinity or NaN values
for float GUCs.  This seems like a bug, maybe even a back-patchable one;
I doubt the planner will react sanely to SET seq_page_cost TO 'NaN'
for instance.  I didn't do anything about that here either.

4. I've not done anything here about increasing the max value of
[autovacuum_]vacuum_cost_limit.  I have no objection to kicking that
up 10x or so if somebody wants to do the work, but I'm not sure it's
very useful given this patch.

Comments?

            regards, tom lane

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index cdf1f4a..8ca36ab 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1198,7 +1198,7 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
             {
                 relopt_real *optreal = (relopt_real *) option->gen;

-                parsed = parse_real(value, &option->values.real_val);
+                parsed = parse_real(value, &option->values.real_val, 0, NULL);
                 if (validate && !parsed)
                     ereport(ERROR,
                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index bb6052a..5226733 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -755,13 +755,13 @@ const char *const config_type_names[] =
  * For each supported conversion from one unit to another, we have an entry
  * in the table.
  *
- * To keep things simple, and to avoid intermediate-value overflows,
+ * To keep things simple, and to avoid possible roundoff error,
  * conversions are never chained.  There needs to be a direct conversion
  * between all units (of the same type).
  *
- * The conversions from each base unit must be kept in order from greatest
- * to smallest unit; convert_from_base_unit() relies on that.  (The order of
- * the base units does not matter.)
+ * The conversions for each base unit must be kept in order from greatest to
+ * smallest human-friendly unit; convert_xxx_from_base_unit() rely on that.
+ * (The order of the base-unit groups does not matter.)
  */
 #define MAX_UNIT_LEN        3    /* length of longest recognized unit string */

@@ -770,9 +770,7 @@ typedef struct
     char        unit[MAX_UNIT_LEN + 1]; /* unit, as a string, like "kB" or
                                          * "min" */
     int            base_unit;        /* GUC_UNIT_XXX */
-    int64        multiplier;        /* If positive, multiply the value with this
-                                 * for unit -> base_unit conversion.  If
-                                 * negative, divide (with the absolute value) */
+    double        multiplier;        /* Factor for converting unit -> base_unit */
 } unit_conversion;

 /* Ensure that the constants in the tables don't overflow or underflow */
@@ -787,45 +785,40 @@ static const char *memory_units_hint = gettext_noop("Valid units for this parame

 static const unit_conversion memory_unit_conversion_table[] =
 {
-    /*
-     * TB -> bytes conversion always overflows 32-bit integer, so this always
-     * produces an error.  Include it nevertheless for completeness, and so
-     * that you get an "out of range" error, rather than "invalid unit".
-     */
-    {"TB", GUC_UNIT_BYTE, INT64CONST(1024) * 1024 * 1024 * 1024},
-    {"GB", GUC_UNIT_BYTE, 1024 * 1024 * 1024},
-    {"MB", GUC_UNIT_BYTE, 1024 * 1024},
-    {"kB", GUC_UNIT_BYTE, 1024},
-    {"B", GUC_UNIT_BYTE, 1},
-
-    {"TB", GUC_UNIT_KB, 1024 * 1024 * 1024},
-    {"GB", GUC_UNIT_KB, 1024 * 1024},
-    {"MB", GUC_UNIT_KB, 1024},
-    {"kB", GUC_UNIT_KB, 1},
-    {"B", GUC_UNIT_KB, -1024},
-
-    {"TB", GUC_UNIT_MB, 1024 * 1024},
-    {"GB", GUC_UNIT_MB, 1024},
-    {"MB", GUC_UNIT_MB, 1},
-    {"kB", GUC_UNIT_MB, -1024},
-    {"B", GUC_UNIT_MB, -(1024 * 1024)},
-
-    {"TB", GUC_UNIT_BLOCKS, (1024 * 1024 * 1024) / (BLCKSZ / 1024)},
-    {"GB", GUC_UNIT_BLOCKS, (1024 * 1024) / (BLCKSZ / 1024)},
-    {"MB", GUC_UNIT_BLOCKS, 1024 / (BLCKSZ / 1024)},
-    {"kB", GUC_UNIT_BLOCKS, -(BLCKSZ / 1024)},
-    {"B", GUC_UNIT_BLOCKS, -BLCKSZ},
-
-    {"TB", GUC_UNIT_XBLOCKS, (1024 * 1024 * 1024) / (XLOG_BLCKSZ / 1024)},
-    {"GB", GUC_UNIT_XBLOCKS, (1024 * 1024) / (XLOG_BLCKSZ / 1024)},
-    {"MB", GUC_UNIT_XBLOCKS, 1024 / (XLOG_BLCKSZ / 1024)},
-    {"kB", GUC_UNIT_XBLOCKS, -(XLOG_BLCKSZ / 1024)},
-    {"B", GUC_UNIT_XBLOCKS, -XLOG_BLCKSZ},
+    {"TB", GUC_UNIT_BYTE, 1024.0 * 1024.0 * 1024.0 * 1024.0},
+    {"GB", GUC_UNIT_BYTE, 1024.0 * 1024.0 * 1024.0},
+    {"MB", GUC_UNIT_BYTE, 1024.0 * 1024.0},
+    {"kB", GUC_UNIT_BYTE, 1024.0},
+    {"B", GUC_UNIT_BYTE, 1.0},
+
+    {"TB", GUC_UNIT_KB, 1024.0 * 1024.0 * 1024.0},
+    {"GB", GUC_UNIT_KB, 1024.0 * 1024.0},
+    {"MB", GUC_UNIT_KB, 1024.0},
+    {"kB", GUC_UNIT_KB, 1.0},
+    {"B", GUC_UNIT_KB, 1.0 / 1024.0},
+
+    {"TB", GUC_UNIT_MB, 1024.0 * 1024.0},
+    {"GB", GUC_UNIT_MB, 1024.0},
+    {"MB", GUC_UNIT_MB, 1.0},
+    {"kB", GUC_UNIT_MB, 1.0 / 1024.0},
+    {"B", GUC_UNIT_MB, 1.0 / (1024.0 * 1024.0)},
+
+    {"TB", GUC_UNIT_BLOCKS, (1024.0 * 1024.0 * 1024.0) / (BLCKSZ / 1024)},
+    {"GB", GUC_UNIT_BLOCKS, (1024.0 * 1024.0) / (BLCKSZ / 1024)},
+    {"MB", GUC_UNIT_BLOCKS, 1024.0 / (BLCKSZ / 1024)},
+    {"kB", GUC_UNIT_BLOCKS, 1.0 / (BLCKSZ / 1024)},
+    {"B", GUC_UNIT_BLOCKS, 1.0 / BLCKSZ},
+
+    {"TB", GUC_UNIT_XBLOCKS, (1024.0 * 1024.0 * 1024.0) / (XLOG_BLCKSZ / 1024)},
+    {"GB", GUC_UNIT_XBLOCKS, (1024.0 * 1024.0) / (XLOG_BLCKSZ / 1024)},
+    {"MB", GUC_UNIT_XBLOCKS, 1024.0 / (XLOG_BLCKSZ / 1024)},
+    {"kB", GUC_UNIT_XBLOCKS, 1.0 / (XLOG_BLCKSZ / 1024)},
+    {"B", GUC_UNIT_XBLOCKS, 1.0 / XLOG_BLCKSZ},

     {""}                        /* end of table marker */
 };

-static const char *time_units_hint = gettext_noop("Valid units for this parameter are \"ms\", \"s\", \"min\", \"h\",
and\"d\"."); 
+static const char *time_units_hint = gettext_noop("Valid units for this parameter are \"us\", \"ms\", \"s\", \"min\",
\"h\",and \"d\"."); 

 static const unit_conversion time_unit_conversion_table[] =
 {
@@ -834,18 +827,21 @@ static const unit_conversion time_unit_conversion_table[] =
     {"min", GUC_UNIT_MS, 1000 * 60},
     {"s", GUC_UNIT_MS, 1000},
     {"ms", GUC_UNIT_MS, 1},
+    {"us", GUC_UNIT_MS, 1.0 / 1000},

     {"d", GUC_UNIT_S, 60 * 60 * 24},
     {"h", GUC_UNIT_S, 60 * 60},
     {"min", GUC_UNIT_S, 60},
     {"s", GUC_UNIT_S, 1},
-    {"ms", GUC_UNIT_S, -1000},
+    {"ms", GUC_UNIT_S, 1.0 / 1000},
+    {"us", GUC_UNIT_S, 1.0 / (1000 * 1000)},

     {"d", GUC_UNIT_MIN, 60 * 24},
     {"h", GUC_UNIT_MIN, 60},
     {"min", GUC_UNIT_MIN, 1},
-    {"s", GUC_UNIT_MIN, -60},
-    {"ms", GUC_UNIT_MIN, -1000 * 60},
+    {"s", GUC_UNIT_MIN, 1.0 / 60},
+    {"ms", GUC_UNIT_MIN, 1.0 / (1000 * 60)},
+    {"us", GUC_UNIT_MIN, 1.0 / (1000 * 1000 * 60)},

     {""}                        /* end of table marker */
 };
@@ -5960,17 +5956,35 @@ ReportGUCOption(struct config_generic *record)
 /*
  * Convert a value from one of the human-friendly units ("kB", "min" etc.)
  * to the given base unit.  'value' and 'unit' are the input value and unit
- * to convert from.  The converted value is stored in *base_value.
+ * to convert from (there can be trailing spaces in the unit string).
+ * The converted value is stored in *base_value.
+ * It's caller's responsibility to round off the converted value as necessary
+ * and check for out-of-range.
  *
  * Returns true on success, false if the input unit is not recognized.
  */
 static bool
-convert_to_base_unit(int64 value, const char *unit,
-                     int base_unit, int64 *base_value)
+convert_to_base_unit(double value, const char *unit,
+                     int base_unit, double *base_value)
 {
+    char        unitstr[MAX_UNIT_LEN + 1];
+    int            unitlen;
     const unit_conversion *table;
     int            i;

+    /* extract unit string to compare to table entries */
+    unitlen = 0;
+    while (*unit != '\0' && !isspace((unsigned char) *unit) &&
+           unitlen < MAX_UNIT_LEN)
+        unitstr[unitlen++] = *(unit++);
+    unitstr[unitlen] = '\0';
+    /* allow whitespace after unit */
+    while (isspace((unsigned char) *unit))
+        unit++;
+    if (*unit != '\0')
+        return false;            /* unit too long, or garbage after it */
+
+    /* now search the appropriate table */
     if (base_unit & GUC_UNIT_MEMORY)
         table = memory_unit_conversion_table;
     else
@@ -5979,12 +5993,9 @@ convert_to_base_unit(int64 value, const char *unit,
     for (i = 0; *table[i].unit; i++)
     {
         if (base_unit == table[i].base_unit &&
-            strcmp(unit, table[i].unit) == 0)
+            strcmp(unitstr, table[i].unit) == 0)
         {
-            if (table[i].multiplier < 0)
-                *base_value = value / (-table[i].multiplier);
-            else
-                *base_value = value * table[i].multiplier;
+            *base_value = value * table[i].multiplier;
             return true;
         }
     }
@@ -5992,14 +6003,15 @@ convert_to_base_unit(int64 value, const char *unit,
 }

 /*
- * Convert a value in some base unit to a human-friendly unit.  The output
- * unit is chosen so that it's the greatest unit that can represent the value
- * without loss.  For example, if the base unit is GUC_UNIT_KB, 1024 is
- * converted to 1 MB, but 1025 is represented as 1025 kB.
+ * Convert an integer value in some base unit to a human-friendly unit.
+ *
+ * The output unit is chosen so that it's the greatest unit that can represent
+ * the value without loss.  For example, if the base unit is GUC_UNIT_KB, 1024
+ * is converted to 1 MB, but 1025 is represented as 1025 kB.
  */
 static void
-convert_from_base_unit(int64 base_value, int base_unit,
-                       int64 *value, const char **unit)
+convert_int_from_base_unit(int64 base_value, int base_unit,
+                           int64 *value, const char **unit)
 {
     const unit_conversion *table;
     int            i;
@@ -6020,15 +6032,15 @@ convert_from_base_unit(int64 base_value, int base_unit,
              * assume that the conversions for each base unit are ordered from
              * greatest unit to the smallest!
              */
-            if (table[i].multiplier < 0)
+            if (table[i].multiplier <= 1.0)
             {
-                *value = base_value * (-table[i].multiplier);
+                *value = rint(base_value / table[i].multiplier);
                 *unit = table[i].unit;
                 break;
             }
-            else if (base_value % table[i].multiplier == 0)
+            else if (base_value % (int64) table[i].multiplier == 0)
             {
-                *value = base_value / table[i].multiplier;
+                *value = rint(base_value / table[i].multiplier);
                 *unit = table[i].unit;
                 break;
             }
@@ -6038,6 +6050,44 @@ convert_from_base_unit(int64 base_value, int base_unit,
     Assert(*unit != NULL);
 }

+/*
+ * Convert a floating-point value in some base unit to a human-friendly unit.
+ *
+ * Same as above, except we have to do the math a bit differently, and
+ * there's a possibility that we don't find any exact divisor.
+ */
+static void
+convert_real_from_base_unit(double base_value, int base_unit,
+                            double *value, const char **unit)
+{
+    const unit_conversion *table;
+    int            i;
+
+    *unit = NULL;
+
+    if (base_unit & GUC_UNIT_MEMORY)
+        table = memory_unit_conversion_table;
+    else
+        table = time_unit_conversion_table;
+
+    for (i = 0; *table[i].unit; i++)
+    {
+        if (base_unit == table[i].base_unit)
+        {
+            /*
+             * Accept the first conversion that divides the value evenly; or
+             * if there is none, use the smallest (last) target unit.
+             */
+            *value = base_value / table[i].multiplier;
+            *unit = table[i].unit;
+            if (*value == rint(*value))
+                break;
+        }
+    }
+
+    Assert(*unit != NULL);
+}
+

 /*
  * Try to parse value as an integer.  The accepted formats are the
@@ -6082,26 +6132,14 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
     /* Handle possible unit */
     if (*endptr != '\0')
     {
-        char        unit[MAX_UNIT_LEN + 1];
-        int            unitlen;
-        bool        converted = false;
+        double        cval;

         if ((flags & GUC_UNIT) == 0)
             return false;        /* this setting does not accept a unit */

-        unitlen = 0;
-        while (*endptr != '\0' && !isspace((unsigned char) *endptr) &&
-               unitlen < MAX_UNIT_LEN)
-            unit[unitlen++] = *(endptr++);
-        unit[unitlen] = '\0';
-        /* allow whitespace after unit */
-        while (isspace((unsigned char) *endptr))
-            endptr++;
-
-        if (*endptr == '\0')
-            converted = convert_to_base_unit(val, unit, (flags & GUC_UNIT),
-                                             &val);
-        if (!converted)
+        if (!convert_to_base_unit((double) val,
+                                  endptr, (flags & GUC_UNIT),
+                                  &cval))
         {
             /* invalid unit, or garbage after the unit; set hint and fail. */
             if (hintmsg)
@@ -6114,13 +6152,15 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
             return false;
         }

-        /* Check for overflow due to units conversion */
-        if (val != (int64) ((int32) val))
+        /* Round to int, then check for overflow due to units conversion */
+        cval = rint(cval);
+        if (cval > INT_MAX || cval < INT_MIN)
         {
             if (hintmsg)
                 *hintmsg = gettext_noop("Value exceeds integer range.");
             return false;
         }
+        val = (int64) cval;
     }

     if (result)
@@ -6136,24 +6176,47 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
  * If okay and result is not NULL, return the value in *result.
  */
 bool
-parse_real(const char *value, double *result)
+parse_real(const char *value, double *result, int flags, const char **hintmsg)
 {
     double        val;
     char       *endptr;

+    /* To suppress compiler warnings, always set output params */
     if (result)
-        *result = 0;            /* suppress compiler warning */
+        *result = 0;
+    if (hintmsg)
+        *hintmsg = NULL;

     errno = 0;
     val = strtod(value, &endptr);
     if (endptr == value || errno == ERANGE)
         return false;

-    /* allow whitespace after number */
+    /* allow whitespace between number and unit */
     while (isspace((unsigned char) *endptr))
         endptr++;
+
+    /* Handle possible unit */
     if (*endptr != '\0')
-        return false;
+    {
+        if ((flags & GUC_UNIT) == 0)
+            return false;        /* this setting does not accept a unit */
+
+        if (!convert_to_base_unit(val,
+                                  endptr, (flags & GUC_UNIT),
+                                  &val))
+        {
+            /* invalid unit, or garbage after the unit; set hint and fail. */
+            if (hintmsg)
+            {
+                if (flags & GUC_UNIT_MEMORY)
+                    *hintmsg = memory_units_hint;
+                else
+                    *hintmsg = time_units_hint;
+            }
+            return false;
+        }
+    }

     if (result)
         *result = val;
@@ -6336,13 +6399,16 @@ parse_and_validate_value(struct config_generic *record,
         case PGC_REAL:
             {
                 struct config_real *conf = (struct config_real *) record;
+                const char *hintmsg;

-                if (!parse_real(value, &newval->realval))
+                if (!parse_real(value, &newval->realval,
+                                conf->gen.flags, &hintmsg))
                 {
                     ereport(elevel,
                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                             errmsg("parameter \"%s\" requires a numeric value",
-                                    name)));
+                             errmsg("invalid value for parameter \"%s\": \"%s\"",
+                                    name, value),
+                             hintmsg ? errhint("%s", _(hintmsg)) : 0));
                     return false;
                 }

@@ -8687,48 +8753,44 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow)
     values[1] = _ShowOption(conf, false);

     /* unit */
-    if (conf->vartype == PGC_INT)
+    switch (conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME))
     {
-        switch (conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME))
-        {
-            case GUC_UNIT_BYTE:
-                values[2] = "B";
-                break;
-            case GUC_UNIT_KB:
-                values[2] = "kB";
-                break;
-            case GUC_UNIT_MB:
-                values[2] = "MB";
-                break;
-            case GUC_UNIT_BLOCKS:
-                snprintf(buffer, sizeof(buffer), "%dkB", BLCKSZ / 1024);
-                values[2] = pstrdup(buffer);
-                break;
-            case GUC_UNIT_XBLOCKS:
-                snprintf(buffer, sizeof(buffer), "%dkB", XLOG_BLCKSZ / 1024);
-                values[2] = pstrdup(buffer);
-                break;
-            case GUC_UNIT_MS:
-                values[2] = "ms";
-                break;
-            case GUC_UNIT_S:
-                values[2] = "s";
-                break;
-            case GUC_UNIT_MIN:
-                values[2] = "min";
-                break;
-            case 0:
-                values[2] = NULL;
-                break;
-            default:
-                elog(ERROR, "unrecognized GUC units value: %d",
-                     conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME));
-                values[2] = NULL;
-                break;
-        }
+        case 0:
+            /* no unit specified */
+            values[2] = NULL;
+            break;
+        case GUC_UNIT_BYTE:
+            values[2] = "B";
+            break;
+        case GUC_UNIT_KB:
+            values[2] = "kB";
+            break;
+        case GUC_UNIT_MB:
+            values[2] = "MB";
+            break;
+        case GUC_UNIT_BLOCKS:
+            snprintf(buffer, sizeof(buffer), "%dkB", BLCKSZ / 1024);
+            values[2] = pstrdup(buffer);
+            break;
+        case GUC_UNIT_XBLOCKS:
+            snprintf(buffer, sizeof(buffer), "%dkB", XLOG_BLCKSZ / 1024);
+            values[2] = pstrdup(buffer);
+            break;
+        case GUC_UNIT_MS:
+            values[2] = "ms";
+            break;
+        case GUC_UNIT_S:
+            values[2] = "s";
+            break;
+        case GUC_UNIT_MIN:
+            values[2] = "min";
+            break;
+        default:
+            elog(ERROR, "unrecognized GUC units value: %d",
+                 conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME));
+            values[2] = NULL;
+            break;
     }
-    else
-        values[2] = NULL;

     /* group */
     values[3] = _(config_group_names[conf->group]);
@@ -9257,10 +9319,9 @@ _ShowOption(struct config_generic *record, bool use_units)
                     const char *unit;

                     if (use_units && result > 0 && (record->flags & GUC_UNIT))
-                    {
-                        convert_from_base_unit(result, record->flags & GUC_UNIT,
-                                               &result, &unit);
-                    }
+                        convert_int_from_base_unit(result,
+                                                   record->flags & GUC_UNIT,
+                                                   &result, &unit);
                     else
                         unit = "";

@@ -9279,8 +9340,18 @@ _ShowOption(struct config_generic *record, bool use_units)
                     val = conf->show_hook();
                 else
                 {
-                    snprintf(buffer, sizeof(buffer), "%g",
-                             *conf->variable);
+                    double        result = *conf->variable;
+                    const char *unit;
+
+                    if (use_units && result > 0 && (record->flags & GUC_UNIT))
+                        convert_real_from_base_unit(result,
+                                                    record->flags & GUC_UNIT,
+                                                    &result, &unit);
+                    else
+                        unit = "";
+
+                    snprintf(buffer, sizeof(buffer), "%g%s",
+                             result, unit);
                     val = buffer;
                 }
             }
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07e7b9..2712a77 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -361,7 +361,8 @@ extern void BeginReportingGUCOptions(void);
 extern void ParseLongOption(const char *string, char **name, char **value);
 extern bool parse_int(const char *value, int *result, int flags,
           const char **hintmsg);
-extern bool parse_real(const char *value, double *result);
+extern bool parse_real(const char *value, double *result, int flags,
+           const char **hintmsg);
 extern int set_config_option(const char *name, const char *value,
                   GucContext context, GucSource source,
                   GucAction action, bool changeVal, int elevel,
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7bbe8f5..9618aca 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1845,7 +1845,7 @@ include_dir 'conf.d'

      <variablelist>
       <varlistentry id="guc-vacuum-cost-delay" xreflabel="vacuum_cost_delay">
-       <term><varname>vacuum_cost_delay</varname> (<type>integer</type>)
+       <term><varname>vacuum_cost_delay</varname> (<type>floating point</type>)
        <indexterm>
         <primary><varname>vacuum_cost_delay</varname> configuration parameter</primary>
        </indexterm>
@@ -1856,18 +1856,19 @@ include_dir 'conf.d'
          when the cost limit has been exceeded.
          The default value is zero, which disables the cost-based vacuum
          delay feature.  Positive values enable cost-based vacuuming.
-         Note that on many systems, the effective resolution
-         of sleep delays is 10 milliseconds; setting
-         <varname>vacuum_cost_delay</varname> to a value that is
-         not a multiple of 10 might have the same results as setting it
-         to the next higher multiple of 10.
         </para>

         <para>
          When using cost-based vacuuming, appropriate values for
          <varname>vacuum_cost_delay</varname> are usually quite small, perhaps
-         10 or 20 milliseconds.  Adjusting vacuum's resource consumption
-         is best done by changing the other vacuum cost parameters.
+         less than 1 millisecond.  While <varname>vacuum_cost_delay</varname>
+         can be set to fractional-millisecond values, such delays may not be
+         measured accurately on older platforms.  On such platforms,
+         increasing <command>VACUUM</command>'s throttled resource consumption
+         above what you get at 1ms will require changing the other vacuum cost
+         parameters.  You should, nonetheless,
+         keep <varname>vacuum_cost_delay</varname> as small as your platform
+         will consistently measure; large delays are not helpful.
         </para>
        </listitem>
       </varlistentry>
@@ -7020,7 +7021,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
      </varlistentry>

      <varlistentry id="guc-autovacuum-vacuum-cost-delay" xreflabel="autovacuum_vacuum_cost_delay">
-      <term><varname>autovacuum_vacuum_cost_delay</varname> (<type>integer</type>)
+      <term><varname>autovacuum_vacuum_cost_delay</varname> (<type>floating point</type>)
       <indexterm>
        <primary><varname>autovacuum_vacuum_cost_delay</varname> configuration parameter</primary>
       </indexterm>
@@ -7030,7 +7031,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         Specifies the cost delay value that will be used in automatic
         <command>VACUUM</command> operations.  If -1 is specified, the regular
         <xref linkend="guc-vacuum-cost-delay"/> value will be used.
-        The default value is 20 milliseconds.
+        The default value is 2 milliseconds.
         This parameter can only be set in the <filename>postgresql.conf</filename>
         file or on the server command line;
         but the setting can be overridden for individual tables by
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 22dbc07..e94fe2c 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1385,7 +1385,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
    </varlistentry>

    <varlistentry>
-    <term><literal>autovacuum_vacuum_cost_delay</literal>, <literal>toast.autovacuum_vacuum_cost_delay</literal>
(<type>integer</type>)</term>
+    <term><literal>autovacuum_vacuum_cost_delay</literal>, <literal>toast.autovacuum_vacuum_cost_delay</literal>
(<type>floatingpoint</type>)</term> 
     <listitem>
      <para>
       Per-table value for <xref linkend="guc-autovacuum-vacuum-cost-delay"/>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8ca36ab..3b0b138 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1359,8 +1359,6 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
         offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
         {"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
         offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-        {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT,
-        offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
         {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
         offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
         {"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
@@ -1379,6 +1377,8 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
         offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
         {"toast_tuple_target", RELOPT_TYPE_INT,
         offsetof(StdRdOptions, toast_tuple_target)},
+        {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
+        offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
         {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
         offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
         {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index e91df21..da13a5a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1834,13 +1834,13 @@ vacuum_delay_point(void)
     if (VacuumCostActive && !InterruptPending &&
         VacuumCostBalance >= VacuumCostLimit)
     {
-        int            msec;
+        double        msec;

         msec = VacuumCostDelay * VacuumCostBalance / VacuumCostLimit;
         if (msec > VacuumCostDelay * 4)
             msec = VacuumCostDelay * 4;

-        pg_usleep(msec * 1000L);
+        pg_usleep((long) (msec * 1000));

         VacuumCostBalance = 0;

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 347f91e..e9fe0a6 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -120,7 +120,7 @@ double        autovacuum_anl_scale;
 int            autovacuum_freeze_max_age;
 int            autovacuum_multixact_freeze_max_age;

-int            autovacuum_vac_cost_delay;
+double        autovacuum_vac_cost_delay;
 int            autovacuum_vac_cost_limit;

 int            Log_autovacuum_min_duration = -1;
@@ -189,7 +189,7 @@ typedef struct autovac_table
     Oid            at_relid;
     int            at_vacoptions;    /* bitmask of VacuumOption */
     VacuumParams at_params;
-    int            at_vacuum_cost_delay;
+    double        at_vacuum_cost_delay;
     int            at_vacuum_cost_limit;
     bool        at_dobalance;
     bool        at_sharedrel;
@@ -225,7 +225,7 @@ typedef struct WorkerInfoData
     TimestampTz wi_launchtime;
     bool        wi_dobalance;
     bool        wi_sharedrel;
-    int            wi_cost_delay;
+    double        wi_cost_delay;
     int            wi_cost_limit;
     int            wi_cost_limit_base;
 } WorkerInfoData;
@@ -1785,7 +1785,7 @@ autovac_balance_cost(void)
      */
     int            vac_cost_limit = (autovacuum_vac_cost_limit > 0 ?
                                   autovacuum_vac_cost_limit : VacuumCostLimit);
-    int            vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ?
+    double        vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ?
                                   autovacuum_vac_cost_delay : VacuumCostDelay);
     double        cost_total;
     double        cost_avail;
@@ -1840,7 +1840,7 @@ autovac_balance_cost(void)
         }

         if (worker->wi_proc != NULL)
-            elog(DEBUG2, "autovac_balance_cost(pid=%u db=%u, rel=%u, dobalance=%s cost_limit=%d, cost_limit_base=%d,
cost_delay=%d)",
+            elog(DEBUG2, "autovac_balance_cost(pid=%u db=%u, rel=%u, dobalance=%s cost_limit=%d, cost_limit_base=%d,
cost_delay=%g)",
                  worker->wi_proc->pid, worker->wi_dboid, worker->wi_tableoid,
                  worker->wi_dobalance ? "yes" : "no",
                  worker->wi_cost_limit, worker->wi_cost_limit_base,
@@ -2302,7 +2302,7 @@ do_autovacuum(void)
         autovac_table *tab;
         bool        isshared;
         bool        skipit;
-        int            stdVacuumCostDelay;
+        double        stdVacuumCostDelay;
         int            stdVacuumCostLimit;
         dlist_iter    iter;

@@ -2831,7 +2831,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
         int            multixact_freeze_min_age;
         int            multixact_freeze_table_age;
         int            vac_cost_limit;
-        int            vac_cost_delay;
+        double        vac_cost_delay;
         int            log_min_duration;

         /*
@@ -2993,7 +2993,7 @@ relation_needs_vacanalyze(Oid relid,
      * table), or the autovacuum GUC variables.
      */

-    /* -1 in autovac setting means use plain vacuum_cost_delay */
+    /* -1 in autovac setting means use plain vacuum_scale_factor */
     vac_scale_factor = (relopts && relopts->vacuum_scale_factor >= 0)
         ? relopts->vacuum_scale_factor
         : autovacuum_vac_scale;
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index a6ce184..6d1e94f 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -138,7 +138,7 @@ int            VacuumCostPageHit = 1;    /* GUC parameters for vacuum */
 int            VacuumCostPageMiss = 10;
 int            VacuumCostPageDirty = 20;
 int            VacuumCostLimit = 2000;
-int            VacuumCostDelay = 0;
+double        VacuumCostDelay = 0;

 int            VacuumPageHit = 0;
 int            VacuumPageMiss = 0;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5226733..d219b7d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2270,28 +2270,6 @@ static struct config_int ConfigureNamesInt[] =
     },

     {
-        {"vacuum_cost_delay", PGC_USERSET, RESOURCES_VACUUM_DELAY,
-            gettext_noop("Vacuum cost delay in milliseconds."),
-            NULL,
-            GUC_UNIT_MS
-        },
-        &VacuumCostDelay,
-        0, 0, 100,
-        NULL, NULL, NULL
-    },
-
-    {
-        {"autovacuum_vacuum_cost_delay", PGC_SIGHUP, AUTOVACUUM,
-            gettext_noop("Vacuum cost delay in milliseconds, for autovacuum."),
-            NULL,
-            GUC_UNIT_MS
-        },
-        &autovacuum_vac_cost_delay,
-        20, -1, 100,
-        NULL, NULL, NULL
-    },
-
-    {
         {"autovacuum_vacuum_cost_limit", PGC_SIGHUP, AUTOVACUUM,
             gettext_noop("Vacuum cost amount available before napping, for autovacuum."),
             NULL
@@ -3317,6 +3295,28 @@ static struct config_real ConfigureNamesReal[] =
     },

     {
+        {"vacuum_cost_delay", PGC_USERSET, RESOURCES_VACUUM_DELAY,
+            gettext_noop("Vacuum cost delay in milliseconds."),
+            NULL,
+            GUC_UNIT_MS
+        },
+        &VacuumCostDelay,
+        0, 0, 100,
+        NULL, NULL, NULL
+    },
+
+    {
+        {"autovacuum_vacuum_cost_delay", PGC_SIGHUP, AUTOVACUUM,
+            gettext_noop("Vacuum cost delay in milliseconds, for autovacuum."),
+            NULL,
+            GUC_UNIT_MS
+        },
+        &autovacuum_vac_cost_delay,
+        2, -1, 100,
+        NULL, NULL, NULL
+    },
+
+    {
         {"autovacuum_vacuum_scale_factor", PGC_SIGHUP, AUTOVACUUM,
             gettext_noop("Number of tuple updates or deletes prior to vacuum as a fraction of reltuples."),
             NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 99f1666..acd7daf 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -155,7 +155,7 @@

 # - Cost-Based Vacuum Delay -

-#vacuum_cost_delay = 0            # 0-100 milliseconds
+#vacuum_cost_delay = 0            # 0-100 milliseconds (0 disables)
 #vacuum_cost_page_hit = 1        # 0-10000 credits
 #vacuum_cost_page_miss = 10        # 0-10000 credits
 #vacuum_cost_page_dirty = 20        # 0-10000 credits
@@ -589,7 +589,7 @@
 #autovacuum_multixact_freeze_max_age = 400000000    # maximum multixact age
                     # before forced vacuum
                     # (change requires restart)
-#autovacuum_vacuum_cost_delay = 20ms    # default vacuum cost delay for
+#autovacuum_vacuum_cost_delay = 2ms    # default vacuum cost delay for
                     # autovacuum, in milliseconds;
                     # -1 means use vacuum_cost_delay
 #autovacuum_vacuum_cost_limit = -1    # default vacuum cost limit for
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index c9e3500..b677c7e 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -250,7 +250,7 @@ extern int    VacuumCostPageHit;
 extern int    VacuumCostPageMiss;
 extern int    VacuumCostPageDirty;
 extern int    VacuumCostLimit;
-extern int    VacuumCostDelay;
+extern double VacuumCostDelay;

 extern int    VacuumPageHit;
 extern int    VacuumPageMiss;
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index 79e99f0..722ef1c 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -37,7 +37,7 @@ extern int    autovacuum_anl_thresh;
 extern double autovacuum_anl_scale;
 extern int    autovacuum_freeze_max_age;
 extern int    autovacuum_multixact_freeze_max_age;
-extern int    autovacuum_vac_cost_delay;
+extern double autovacuum_vac_cost_delay;
 extern int    autovacuum_vac_cost_limit;

 /* autovacuum launcher PID, only valid when worker is shutting down */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 9d805ca..5402851 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -243,7 +243,6 @@ typedef struct AutoVacOpts
     bool        enabled;
     int            vacuum_threshold;
     int            analyze_threshold;
-    int            vacuum_cost_delay;
     int            vacuum_cost_limit;
     int            freeze_min_age;
     int            freeze_max_age;
@@ -252,6 +251,7 @@ typedef struct AutoVacOpts
     int            multixact_freeze_max_age;
     int            multixact_freeze_table_age;
     int            log_min_duration;
+    float8        vacuum_cost_delay;
     float8        vacuum_scale_factor;
     float8        analyze_scale_factor;
 } AutoVacOpts;
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index b0d7351..aa5fe58 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -149,11 +149,11 @@ SELECT '2006-08-13 12:34:56'::timestamptz;
 (1 row)

 SAVEPOINT first_sp;
-SET vacuum_cost_delay TO 80;
+SET vacuum_cost_delay TO 80.1;
 SHOW vacuum_cost_delay;
  vacuum_cost_delay
 -------------------
- 80ms
+ 80100us
 (1 row)

 SET datestyle = 'German, DMY';
@@ -183,7 +183,7 @@ SELECT '2006-08-13 12:34:56'::timestamptz;
 (1 row)

 SAVEPOINT second_sp;
-SET vacuum_cost_delay TO 90;
+SET vacuum_cost_delay TO '900us';
 SET datestyle = 'SQL, YMD';
 SHOW datestyle;
  DateStyle
@@ -222,7 +222,7 @@ ROLLBACK TO third_sp;
 SHOW vacuum_cost_delay;
  vacuum_cost_delay
 -------------------
- 90ms
+ 900us
 (1 row)

 SHOW datestyle;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 3b854ac..4fd8dc3 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -47,7 +47,7 @@ SET datestyle = 'MDY';
 SHOW datestyle;
 SELECT '2006-08-13 12:34:56'::timestamptz;
 SAVEPOINT first_sp;
-SET vacuum_cost_delay TO 80;
+SET vacuum_cost_delay TO 80.1;
 SHOW vacuum_cost_delay;
 SET datestyle = 'German, DMY';
 SHOW datestyle;
@@ -56,7 +56,7 @@ ROLLBACK TO first_sp;
 SHOW datestyle;
 SELECT '2006-08-13 12:34:56'::timestamptz;
 SAVEPOINT second_sp;
-SET vacuum_cost_delay TO 90;
+SET vacuum_cost_delay TO '900us';
 SET datestyle = 'SQL, YMD';
 SHOW datestyle;
 SELECT '2006-08-13 12:34:56'::timestamptz;

pgsql-hackers by date:

Previous
From: "Karl O. Pinc"
Date:
Subject: Re: Patch to document base64 encoding
Next
From: Tom Lane
Date:
Subject: Re: Should we increase the default vacuum_cost_limit?