Re: [Suspect SPAM] Re: Should we increase the defaultvacuum_cost_limit? - Mailing list pgsql-hackers

From Kyotaro HORIGUCHI
Subject Re: [Suspect SPAM] Re: Should we increase the defaultvacuum_cost_limit?
Date
Msg-id 20190312.123128.04147221.horiguchi.kyotaro@lab.ntt.co.jp
Whole thread Raw
In response to Re: Should we increase the default vacuum_cost_limit?  (Julien Rouhaud <rjuju123@gmail.com>)
List pgsql-hackers
At Mon, 11 Mar 2019 13:57:21 +0100, Julien Rouhaud <rjuju123@gmail.com> wrote in
<CAOBaU_a2tLyonOMJ62=SiDmo84Xo1fy81YA8K=B+=OtTc3sYSQ@mail.gmail.com>
> On Mon, Mar 11, 2019 at 10:03 AM David Rowley
> <david.rowley@2ndquadrant.com> wrote:
> >
> > On Mon, 11 Mar 2019 at 09:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > The second patch is a delta that rounds off to the next smaller unit
> > > if there is one, producing a less noisy result:
> > >
> > > regression=# set work_mem = '30.1GB';
> > > SET
> > > regression=# show work_mem;
> > >  work_mem
> > > ----------
> > >  30822MB
> > > (1 row)
> > >
> > > I'm not sure if that's a good idea or just overthinking the problem.
> > > Thoughts?
> >
> > I don't think you're over thinking it.  I often have to look at such
> > settings and I'm probably not unique in when I glance at 30822MB I can
> > see that's roughly 30GB, whereas when I look at 31562138kB, I'm either
> > counting digits or reaching for a calculator.  This is going to reduce
> > the time it takes for a human to process the pg_settings output, so I
> > think it's a good idea.
> 
> Definitely, rounding up will spare people from wasting time to check
> what's the actual value.

+1. I don't think it overthinking, too.

Anyone who specifies memory size in GB won't care under-MB
fraction. I don't think '0.01GB' is a sane setting but it being
10MB doesn't matter.  However, I don't think that '0.1d' becoming
'2h' is reasonable. "10 times per day" is "rounded" to "12 times
per day" by that.

Is it worth showing values with at most two or three fraction
digits instead of rounding the value on setting? In the attached
PoC patch - instead of the 'roundoff-fractions-harder' patch -
shows values in the shortest exact representation.

work_mem:
   31562138  => '30.1 GB'
   31562137  => '31562137 kB'
   '0.1GB'   => '0.1 GB'
   '0.01GB'  => '0.01 GB'
   '0.001GB' => '1049 kB'

lock_timeout:
   '0.1h'    => '6 min'
   '90 min'  => '90 min'
   '120 min' => '2 h'
   '0.1 d'   => '0.1 d'

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d374f5372c..3ca2d6dec4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6011,7 +6011,7 @@ convert_to_base_unit(double value, const char *unit,
  */
 static void
 convert_int_from_base_unit(int64 base_value, int base_unit,
-                           int64 *value, const char **unit)
+                           double *value, const char **unit, int *digits)
 {
     const unit_conversion *table;
     int            i;
@@ -6027,6 +6027,9 @@ convert_int_from_base_unit(int64 base_value, int base_unit,
     {
         if (base_unit == table[i].base_unit)
         {
+            const double frac_digits = 2;
+            double rval;
+
             /*
              * Accept the first conversion that divides the value evenly.  We
              * assume that the conversions for each base unit are ordered from
@@ -6035,7 +6038,42 @@ convert_int_from_base_unit(int64 base_value, int base_unit,
             if (table[i].multiplier <= 1.0 ||
                 base_value % (int64) table[i].multiplier == 0)
             {
-                *value = (int64) rint(base_value / table[i].multiplier);
+                *digits = 0;
+                *value = rint(base_value / table[i].multiplier);
+                *unit = table[i].unit;
+                break;
+            }
+
+            /*
+             * If base_value is exactly represented by a number with at most
+             * two fraction digits, we prefer it than lower units.
+             */
+            rval = (double)base_value / table[i].multiplier;
+            rval = rint(rval * pow(10, frac_digits)) *
+                pow(10, -frac_digits);
+
+            /*
+             * Acceptable range for rval is quite arbitrary.
+             */
+            if ((rval >= 1.0 ||
+                 (table[i + 1].unit &&
+                  table[i].multiplier / table[i + 1].multiplier < 1000) &&
+                (int64)rint(rval * table[i].multiplier) == base_value)
+            {
+                int frac;
+
+                /* count required fraction digits */
+                for (frac = 1 ; frac < frac_digits ; frac++)
+                {
+                    if (rval * 10.0 - floor(rval * 10.0) < 0.1)
+                    {
+                        *digits = frac;
+                        break;
+                    }
+                }
+                if (frac == frac_digits)
+                    *digits = frac_digits;
+                *value = rval;
                 *unit = table[i].unit;
                 break;
             }
@@ -9359,18 +9397,19 @@ _ShowOption(struct config_generic *record, bool use_units)
                      * Use int64 arithmetic to avoid overflows in units
                      * conversion.
                      */
-                    int64        result = *conf->variable;
+                    double        result = *conf->variable;
                     const char *unit;
+                    int            digits = 0;
 
                     if (use_units && result > 0 && (record->flags & GUC_UNIT))
-                        convert_int_from_base_unit(result,
+                        convert_int_from_base_unit(*conf->variable,
                                                    record->flags & GUC_UNIT,
-                                                   &result, &unit);
+                                                   &result, &unit, &digits);
                     else
                         unit = "";
 
-                    snprintf(buffer, sizeof(buffer), INT64_FORMAT "%s",
-                             result, unit);
+                    snprintf(buffer, sizeof(buffer), "%.*f %s",
+                             digits, result, unit);
                     val = buffer;
                 }
             }

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Offline enabling/disabling of data checksums
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: [Suspect SPAM] Re: Should we increase the defaultvacuum_cost_limit?