From fc472c5b473e3495e92ec9cf021b6e6393579ca5 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 7 Mar 2020 15:04:34 +1300 Subject: [PATCH 1/2] Simplify the effective_io_concurrency setting. The effective_io_concurrency GUC and tablespace option were previously not used directly to control the number of prefetches we'd issue. Instead, they were passed through a formula based on a theory about RAID spindles and probabilities. Today, a lot of people are familiar with the concept of I/O request queues and have never seen a spinning disk, so let's switch to something a little easier to justify. If users were using a setting higher than the default value 1, then they will need to increase the value for the same effect. The query to compute the new setting corresponding to the old setting OLD is: select round(sum(OLD / n::float)) as new_setting from generate_series(1, OLD) s(n); Discussion: https://postgr.es/m/CA%2BhUKGJUw08dPs_3EUcdO6M90GnjofPYrWp4YSLaBkgYwS-AqA%40mail.gmail.com --- src/backend/executor/nodeBitmapHeapscan.c | 18 +----- src/backend/storage/buffer/bufmgr.c | 74 +++-------------------- src/backend/utils/misc/guc.c | 29 +-------- src/include/storage/bufmgr.h | 1 - 4 files changed, 13 insertions(+), 109 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index ae8a11da30..726d3a2d9a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -707,7 +707,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) { BitmapHeapScanState *scanstate; Relation currentRelation; - int io_concurrency; /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); @@ -737,8 +736,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->prefetch_iterator = NULL; scanstate->prefetch_pages = 0; scanstate->prefetch_target = 0; - /* may be updated below */ - scanstate->prefetch_maximum = target_prefetch_pages; scanstate->pscan_len = 0; scanstate->initialized = false; scanstate->shared_tbmiterator = NULL; @@ -794,20 +791,11 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) ExecInitQual(node->bitmapqualorig, (PlanState *) scanstate); /* - * Determine the maximum for prefetch_target. If the tablespace has a - * specific IO concurrency set, use that to compute the corresponding - * maximum value; otherwise, we already initialized to the value computed - * by the GUC machinery. + * Maximum number of prefetches for the tablespace if configured, otherwise + * the current value of the effective_io_concurrency GUC. */ - io_concurrency = + scanstate->prefetch_maximum = get_tablespace_io_concurrency(currentRelation->rd_rel->reltablespace); - if (io_concurrency != effective_io_concurrency) - { - double maximum; - - if (ComputeIoConcurrency(io_concurrency, &maximum)) - scanstate->prefetch_maximum = rint(maximum); - } scanstate->ss.ss_currentRelation = currentRelation; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 5880054245..7a7748b695 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -110,6 +110,13 @@ bool zero_damaged_pages = false; int bgwriter_lru_maxpages = 100; double bgwriter_lru_multiplier = 2.0; bool track_io_timing = false; + +/* + * How many buffers PrefetchBuffer callers should try to stay ahead of their + * ReadBuffer calls by. Zero means "never prefetch". This value is only used + * for buffers not belonging to tablespaces that have their + * effective_io_concurrency parameter set. + */ int effective_io_concurrency = 0; /* @@ -120,15 +127,6 @@ int checkpoint_flush_after = 0; int bgwriter_flush_after = 0; int backend_flush_after = 0; -/* - * How many buffers PrefetchBuffer callers should try to stay ahead of their - * ReadBuffer calls by. This is maintained by the assign hook for - * effective_io_concurrency. Zero means "never prefetch". This value is - * only used for buffers not belonging to tablespaces that have their - * effective_io_concurrency parameter set. - */ -int target_prefetch_pages = 0; - /* local state for StartBufferIO and related functions */ static BufferDesc *InProgressBuf = NULL; static bool IsForInput; @@ -461,64 +459,6 @@ static int ckpt_buforder_comparator(const void *pa, const void *pb); static int ts_ckpt_progress_comparator(Datum a, Datum b, void *arg); -/* - * ComputeIoConcurrency -- get the number of pages to prefetch for a given - * number of spindles. - */ -bool -ComputeIoConcurrency(int io_concurrency, double *target) -{ - double new_prefetch_pages = 0.0; - int i; - - /* - * Make sure the io_concurrency value is within valid range; it may have - * been forced with a manual pg_tablespace update. - */ - io_concurrency = Min(Max(io_concurrency, 0), MAX_IO_CONCURRENCY); - - /*---------- - * The user-visible GUC parameter is the number of drives (spindles), - * which we need to translate to a number-of-pages-to-prefetch target. - * The target value is stashed in *extra and then assigned to the actual - * variable by assign_effective_io_concurrency. - * - * The expected number of prefetch pages needed to keep N drives busy is: - * - * drives | I/O requests - * -------+---------------- - * 1 | 1 - * 2 | 2/1 + 2/2 = 3 - * 3 | 3/1 + 3/2 + 3/3 = 5 1/2 - * 4 | 4/1 + 4/2 + 4/3 + 4/4 = 8 1/3 - * n | n * H(n) - * - * This is called the "coupon collector problem" and H(n) is called the - * harmonic series. This could be approximated by n * ln(n), but for - * reasonable numbers of drives we might as well just compute the series. - * - * Alternatively we could set the target to the number of pages necessary - * so that the expected number of active spindles is some arbitrary - * percentage of the total. This sounds the same but is actually slightly - * different. The result ends up being ln(1-P)/ln((n-1)/n) where P is - * that desired fraction. - * - * Experimental results show that both of these formulas aren't aggressive - * enough, but we don't really have any better proposals. - * - * Note that if io_concurrency = 0 (disabled), we must set target = 0. - *---------- - */ - - for (i = 1; i <= io_concurrency; i++) - new_prefetch_pages += (double) io_concurrency / (double) i; - - *target = new_prefetch_pages; - - /* This range check shouldn't fail, but let's be paranoid */ - return (new_prefetch_pages >= 0.0 && new_prefetch_pages < (double) INT_MAX); -} - /* * PrefetchBuffer -- initiate asynchronous read of a block of a relation * diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index c1fad3b350..45dbf270bf 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -195,7 +195,6 @@ static bool check_autovacuum_max_workers(int *newval, void **extra, GucSource so static bool check_max_wal_senders(int *newval, void **extra, GucSource source); static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource source); static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source); -static void assign_effective_io_concurrency(int newval, void *extra); static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); @@ -2881,7 +2880,7 @@ static struct config_int ConfigureNamesInt[] = 0, #endif 0, MAX_IO_CONCURRENCY, - check_effective_io_concurrency, assign_effective_io_concurrency, NULL + check_effective_io_concurrency, NULL, NULL }, { @@ -11456,36 +11455,14 @@ check_max_worker_processes(int *newval, void **extra, GucSource source) static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source) { -#ifdef USE_PREFETCH - double new_prefetch_pages; - - if (ComputeIoConcurrency(*newval, &new_prefetch_pages)) - { - int *myextra = (int *) guc_malloc(ERROR, sizeof(int)); - - *myextra = (int) rint(new_prefetch_pages); - *extra = (void *) myextra; - - return true; - } - else - return false; -#else +#ifndef USE_PREFETCH if (*newval != 0) { GUC_check_errdetail("effective_io_concurrency must be set to 0 on platforms that lack posix_fadvise()."); return false; } - return true; -#endif /* USE_PREFETCH */ -} - -static void -assign_effective_io_concurrency(int newval, void *extra) -{ -#ifdef USE_PREFETCH - target_prefetch_pages = *((int *) extra); #endif /* USE_PREFETCH */ + return true; } static void diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 73c7e9ba38..0b082ea79f 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -161,7 +161,6 @@ extern PGDLLIMPORT int32 *LocalRefCount; /* * prototypes for functions in bufmgr.c */ -extern bool ComputeIoConcurrency(int io_concurrency, double *target); extern void PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum); extern Buffer ReadBuffer(Relation reln, BlockNumber blockNum); -- 2.20.1