Code review for auto-tuned effective_cache_size.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 27 Jan 2014 05:05:49 +0000 (00:05 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 27 Jan 2014 05:05:56 +0000 (00:05 -0500)
Fix integer overflow issue noted by Magnus Hagander, as well as a bunch
of other infelicities in commit ee1e5662d8d8330726eaef7d3110cb7add24d058
and its unreasonably large number of followups.

doc/src/sgml/config.sgml
src/backend/optimizer/path/costsize.c
src/backend/postmaster/postmaster.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/postgresql.conf.sample
src/include/optimizer/cost.h
src/include/utils/guc.h

index be5b89a1121e9290f5fcb47c138273986572cfc9..14ed6c7a53bab78481e1e7f9f0899f1dcfe98766 100644 (file)
@@ -2974,17 +2974,9 @@ include 'filename'
       <listitem>
        <para>
         Sets the planner's assumption about the effective size of the
-        disk cache that is available to a single query.  The default
-        setting of -1 selects a size equal to four times the size of <xref
-        linkend="guc-shared-buffers">, but not less than the size of one
-        shared buffer page, typically <literal>8kB</literal>.  This value
-        can be set manually if the automatic choice is too large or too
-        small.
-       </para>
-
-       <para>
-        This value is factored into estimates of the cost of using an index;
-        a higher value makes it more likely index scans will be used, a
+        disk cache that is available to a single query.  This is
+        factored into estimates of the cost of using an index; a
+        higher value makes it more likely index scans will be used, a
         lower value makes it more likely sequential scans will be
         used. When setting this parameter you should consider both
         <productname>PostgreSQL</productname>'s shared buffers and the
@@ -2996,10 +2988,16 @@ include 'filename'
         memory allocated by <productname>PostgreSQL</productname>, nor
         does it reserve kernel disk cache; it is used only for estimation
         purposes.  The system also does not assume data remains in
-        the disk cache between queries.  The auto-tuning
-        selected by the default setting of -1 should give reasonable
-        results if this database cluster can utilize most of the memory
-        on this server.
+        the disk cache between queries.
+       </para>
+
+       <para>
+        If <varname>effective_cache_size</> is set to -1, which is the
+        default, the value is replaced by an automatically selected value,
+        currently four times the size of <xref linkend="guc-shared-buffers">.
+        For recommended settings of <varname>shared_buffers</>, this should
+        give reasonable results if this database cluster can use most of the
+        memory on the server.
        </para>
       </listitem>
      </varlistentry>
index 8492eedae1bc8bfc02fffaa9afb09916498c5fde..9bca96849d31f4ff7ce0f6d37849ac276ef6823a 100644 (file)
@@ -71,6 +71,7 @@
 #ifdef _MSC_VER
 #include <float.h>             /* for _isnan */
 #endif
+#include <limits.h>
 #include <math.h>
 
 #include "access/htup_details.h"
 
 #define LOG2(x)  (log(x) / 0.693147180559945)
 
+
 double     seq_page_cost = DEFAULT_SEQ_PAGE_COST;
 double     random_page_cost = DEFAULT_RANDOM_PAGE_COST;
 double     cpu_tuple_cost = DEFAULT_CPU_TUPLE_COST;
 double     cpu_index_tuple_cost = DEFAULT_CPU_INDEX_TUPLE_COST;
 double     cpu_operator_cost = DEFAULT_CPU_OPERATOR_COST;
 
-int            effective_cache_size = -1;
+int            effective_cache_size = -1;  /* will get replaced */
 
 Cost       disable_cost = 1.0e10;
 
@@ -456,52 +458,6 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
    path->path.total_cost = startup_cost + run_cost;
 }
 
-void
-set_default_effective_cache_size(void)
-{
-   /*
-    * If the value of effective_cache_size is -1, use the preferred
-    * auto-tune value.
-    */
-   if (effective_cache_size == -1)
-   {
-       char        buf[32];
-
-       snprintf(buf, sizeof(buf), "%d", NBuffers * DEFAULT_EFFECTIVE_CACHE_SIZE_MULTI);
-       SetConfigOption("effective_cache_size", buf, PGC_POSTMASTER, PGC_S_OVERRIDE);
-   }
-   Assert(effective_cache_size > 0);
-}
-
-/*
- * GUC check_hook for effective_cache_size
- */
-bool
-check_effective_cache_size(int *newval, void **extra, GucSource source)
-{
-   /*
-    * -1 indicates a request for auto-tune.
-    */
-   if (*newval == -1)
-   {
-       /*
-        * If we haven't yet changed the boot_val default of -1, just let it
-        * be.  We'll fix it later.
-        */
-       if (effective_cache_size == -1)
-           return true;
-
-       /* Otherwise, substitute the auto-tune value */
-       *newval = NBuffers * DEFAULT_EFFECTIVE_CACHE_SIZE_MULTI;
-   }
-
-   /* set minimum? */
-   if (*newval < 1)
-       *newval = 1;
-
-   return true;
-}
-
 /*
  * index_pages_fetched
  *   Estimate the number of pages actually fetched after accounting for
@@ -4137,3 +4093,59 @@ page_size(double tuples, int width)
 {
    return ceil(relation_byte_size(tuples, width) / BLCKSZ);
 }
+
+/*
+ * GUC check_hook for effective_cache_size
+ */
+bool
+check_effective_cache_size(int *newval, void **extra, GucSource source)
+{
+   /*
+    * -1 is the documented way of requesting auto-tune, but we also treat
+    * zero as meaning that, since we don't consider zero a valid setting.
+    */
+   if (*newval <= 0)
+   {
+       /*
+        * If we haven't yet changed the initial default of -1, just let it
+        * be.  We'll fix it later on during GUC initialization, when
+        * set_default_effective_cache_size is called.  (If we try to do it
+        * immediately, we may not be looking at the final value of NBuffers.)
+        */
+       if (effective_cache_size == -1)
+           return true;
+
+       /*
+        * Otherwise, substitute the auto-tune value, being wary of overflow.
+        */
+       if (NBuffers < INT_MAX / 4)
+           *newval = NBuffers * 4;
+       else
+           *newval = INT_MAX;
+   }
+
+   Assert(*newval > 0);
+
+   return true;
+}
+
+/*
+ * initialize effective_cache_size at the end of GUC startup
+ */
+void
+set_default_effective_cache_size(void)
+{
+   /*
+    * If the value of effective_cache_size is still -1 (or zero), replace it
+    * with the auto-tune value.
+    */
+   if (effective_cache_size <= 0)
+   {
+       /* disable the short-circuit in check_effective_cache_size */
+       effective_cache_size = 0;
+       /* and let check_effective_cache_size() compute the setting */
+       SetConfigOption("effective_cache_size", "-1",
+                       PGC_POSTMASTER, PGC_S_OVERRIDE);
+   }
+   Assert(effective_cache_size > 0);
+}
index 23f4ada7b95b2b326be7c8e0c768135f9951b1e5..b807b064be3ae4c00febfe9b5524acb28b77239c 100644 (file)
 #include "utils/builtins.h"
 #include "utils/datetime.h"
 #include "utils/dynamic_loader.h"
-#include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
index 59f395474a63e73205f0521e93edf9597972f394..2cc8f90e6d4b3562171198607abcd3738b12b121 100644 (file)
@@ -4305,6 +4305,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
     */
    pg_timezone_abbrev_initialize();
 
+   /* Also install the correct value for effective_cache_size */
    set_default_effective_cache_size();
 
    /*
index 27791cc40ec46a6cb22696da200cd8f77d405f52..7ad6b7cb4578c458cbbf429a7c2942300f416da2 100644 (file)
 #cpu_tuple_cost = 0.01         # same scale as above
 #cpu_index_tuple_cost = 0.005      # same scale as above
 #cpu_operator_cost = 0.0025        # same scale as above
-#effective_cache_size = -1
+#effective_cache_size = -1     # -1 selects auto-tuned default
 
 # - Genetic Query Optimizer -
 
index e1b7a0b8bc549424b3b1315192eb9e97653895dd..ec1605d1c9d73156034c3a8b1730dd8a4c0d738a 100644 (file)
@@ -27,8 +27,6 @@
 #define DEFAULT_CPU_INDEX_TUPLE_COST 0.005
 #define DEFAULT_CPU_OPERATOR_COST  0.0025
 
-#define DEFAULT_EFFECTIVE_CACHE_SIZE_MULTI 4
-
 typedef enum
 {
    CONSTRAINT_EXCLUSION_OFF,   /* do not use c_e */
index 3adcc99860078fb4418510050eebe0654a9c7df9..be68f35d37229417ba5b7767a65f8274bc9fd3de 100644 (file)
@@ -387,8 +387,10 @@ extern void assign_search_path(const char *newval, void *extra);
 
 /* in access/transam/xlog.c */
 extern bool check_wal_buffers(int *newval, void **extra, GucSource source);
+extern void assign_xlog_sync_method(int new_sync_method, void *extra);
+
+/* in optimizer/path/costsize.c */
 extern bool check_effective_cache_size(int *newval, void **extra, GucSource source);
 extern void set_default_effective_cache_size(void);
-extern void assign_xlog_sync_method(int new_sync_method, void *extra);
 
 #endif   /* GUC_H */