Code review for auto-tuned effective_cache_size.

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.

6 files changed, 64 insertions, 52 deletions

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 8492eedae1b..9bca96849d3 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -71,6 +71,7 @@
 #ifdef _MSC_VER
 #include <float.h>				/* for _isnan */
 #endif
+#include <limits.h>
 #include <math.h>
 
 #include "access/htup_details.h"
@@ -96,13 +97,14 @@
 
 #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);
+}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 23f4ada7b95..b807b064be3 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -118,7 +118,6 @@
 #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"
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 59f395474a6..2cc8f90e6d4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -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();
 
 	/*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 27791cc40ec..7ad6b7cb457 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -279,7 +279,7 @@
 #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 -
 
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index e1b7a0b8bc5..ec1605d1c9d 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -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 */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 3adcc998600..be68f35d372 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -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 */