From 2850896961994aa0993b9e2ed79a209750181b8a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 27 Jan 2014 00:05:49 -0500 Subject: [PATCH] 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. --- doc/src/sgml/config.sgml | 28 +++-- src/backend/optimizer/path/costsize.c | 106 ++++++++++-------- src/backend/postmaster/postmaster.c | 1 - src/backend/utils/misc/guc.c | 1 + src/backend/utils/misc/postgresql.conf.sample | 2 +- src/include/optimizer/cost.h | 2 - src/include/utils/guc.h | 6 +- 7 files changed, 78 insertions(+), 68 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index be5b89a112..14ed6c7a53 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2974,17 +2974,9 @@ include 'filename' 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 , but not less than the size of one - shared buffer page, typically 8kB. This value - can be set manually if the automatic choice is too large or too - small. - - - - 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 PostgreSQL's shared buffers and the @@ -2996,10 +2988,16 @@ include 'filename' memory allocated by PostgreSQL, 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. + + + + If 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 . + For recommended settings of shared_buffers, this should + give reasonable results if this database cluster can use most of the + memory on the server. diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 8492eedae1..9bca96849d 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -71,6 +71,7 @@ #ifdef _MSC_VER #include /* for _isnan */ #endif +#include #include #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 23f4ada7b9..b807b064be 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 59f395474a..2cc8f90e6d 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 27791cc40e..7ad6b7cb45 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 e1b7a0b8bc..ec1605d1c9 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 3adcc99860..be68f35d37 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 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); +/* 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); + #endif /* GUC_H */