public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 4/4] add force_incremental_sort GUC 10+ messages / 5 participants [nested] [flat]
* [PATCH 4/4] add force_incremental_sort GUC @ 2019-07-09 11:36 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Tomas Vondra @ 2019-07-09 11:36 UTC (permalink / raw) --- src/backend/optimizer/path/costsize.c | 8 ++++++++ src/backend/utils/misc/guc.c | 10 ++++++++++ src/include/optimizer/cost.h | 1 + 3 files changed, 19 insertions(+) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index c6aa17ba67..ee4487b158 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -140,6 +140,8 @@ bool enable_parallel_append = true; bool enable_parallel_hash = true; bool enable_partition_pruning = true; +bool force_incremental_sort = true; + typedef struct { PlannerInfo *root; @@ -1907,6 +1909,12 @@ cost_incremental_sort(Path *path, path->rows = input_tuples; path->startup_cost = startup_cost; path->total_cost = startup_cost + run_cost; + + if (force_incremental_sort) + { + path->startup_cost = input_startup_cost; + path->total_cost = input_total_cost; + } } /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index f922ee66a0..d2cc5b56b5 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -941,6 +941,16 @@ static struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, + { + {"force_incremental_sort", PGC_USERSET, QUERY_TUNING_METHOD, + gettext_noop("Makes the incremental sort look like no-cost."), + NULL, + GUC_EXPLAIN + }, + &force_incremental_sort, + false, + NULL, NULL, NULL + }, { {"enable_incrementalsort", PGC_USERSET, QUERY_TUNING_METHOD, gettext_noop("Enables the planner's use of incremental sort steps."), diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h index b9d7a77e65..bad1d5a330 100644 --- a/src/include/optimizer/cost.h +++ b/src/include/optimizer/cost.h @@ -66,6 +66,7 @@ extern PGDLLIMPORT bool enable_parallel_append; extern PGDLLIMPORT bool enable_parallel_hash; extern PGDLLIMPORT bool enable_partition_pruning; extern PGDLLIMPORT int constraint_exclusion; +extern PGDLLIMPORT bool force_incremental_sort; extern double index_pages_fetched(double tuples_fetched, BlockNumber pages, double index_pages, PlannerInfo *root); -- 2.21.0 --z44ikx7gwyo2xwzv-- ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 1/2] Refactor checks for deleted GiST pages. @ 2019-07-22 12:57 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Heikki Linnakangas @ 2019-07-22 12:57 UTC (permalink / raw) The explicit check in gistScanPage() isn't currently really necessary, as a deleted page is always empty, so the loop would fall through without doing anything, anyway. But it's a marginal optimization, and it gives a nice place to attach a comment to explain how it works. --- src/backend/access/gist/gist.c | 40 ++++++++++++------------------- src/backend/access/gist/gistget.c | 14 +++++++++++ 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 169bf6fcfed..e9ca4b82527 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -709,14 +709,15 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, continue; } - if (stack->blkno != GIST_ROOT_BLKNO && - stack->parent->lsn < GistPageGetNSN(stack->page)) + if ((stack->blkno != GIST_ROOT_BLKNO && + stack->parent->lsn < GistPageGetNSN(stack->page)) || + GistPageIsDeleted(stack->page)) { /* - * Concurrent split detected. There's no guarantee that the - * downlink for this page is consistent with the tuple we're - * inserting anymore, so go back to parent and rechoose the best - * child. + * Concurrent split or page deletion detected. There's no + * guarantee that the downlink for this page is consistent with + * the tuple we're inserting anymore, so go back to parent and + * rechoose the best child. */ UnlockReleaseBuffer(stack->buffer); xlocked = false; @@ -735,9 +736,6 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTInsertStack *item; OffsetNumber downlinkoffnum; - /* currently, internal pages are never deleted */ - Assert(!GistPageIsDeleted(stack->page)); - downlinkoffnum = gistchoose(state.r, stack->page, itup, giststate); iid = PageGetItemId(stack->page, downlinkoffnum); idxtuple = (IndexTuple) PageGetItem(stack->page, iid); @@ -858,12 +856,13 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, * leaf/inner is enough to recognize split for root */ } - else if (GistFollowRight(stack->page) || - stack->parent->lsn < GistPageGetNSN(stack->page)) + else if ((GistFollowRight(stack->page) || + stack->parent->lsn < GistPageGetNSN(stack->page)) && + GistPageIsDeleted(stack->page)) { /* - * The page was split while we momentarily unlocked the - * page. Go back to parent. + * The page was split or deleted while we momentarily + * unlocked the page. Go back to parent. */ UnlockReleaseBuffer(stack->buffer); xlocked = false; @@ -872,18 +871,6 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, } } - /* - * The page might have been deleted after we scanned the parent - * and saw the downlink. - */ - if (GistPageIsDeleted(stack->page)) - { - UnlockReleaseBuffer(stack->buffer); - xlocked = false; - state.stack = stack = stack->parent; - continue; - } - /* now state.stack->(page, buffer and blkno) points to leaf page */ gistinserttuple(&state, stack, giststate, itup, @@ -947,6 +934,9 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum) break; } + /* currently, internal pages are never deleted */ + Assert(!GistPageIsDeleted(page)); + top->lsn = BufferGetLSNAtomic(buffer); /* diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index 46d08e06350..c5fe2ea3998 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -377,6 +377,20 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem, double *myDistances, MemoryContextSwitchTo(oldcxt); } + /* + * Check if the page was deleted after we saw the downlink. There's + * nothing of interest on a deleted page. Note that we must do this + * after checking the NSN for concurrent splits! It's possible that + * the page originally contained some tuples that are visible to use, + * but was split so that all the visible tuples were moved to another + * page, and then this page was deleted. + */ + if (GistPageIsDeleted(page)) + { + UnlockReleaseBuffer(buffer); + return; + } + so->nPageData = so->curPageData = 0; scan->xs_hitup = NULL; /* might point into pageDataCxt */ if (so->pageDataCxt) -- 2.20.1 --------------2155CE93C68FA8184E2F2BE8 Content-Type: text/x-patch; name="0002-Use-full-64-bit-XID-for-checking-if-a-deleted-GiST-p.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Use-full-64-bit-XID-for-checking-if-a-deleted-GiST-p.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make MaxBackends available in _PG_init @ 2022-01-29 04:33 Nathan Bossart <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Nathan Bossart @ 2022-01-29 04:33 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Robert Haas <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Andres Freund <[email protected]>; Bharath Rupireddy <[email protected]>; Greg Sabino Mullane <[email protected]>; Tom Lane <[email protected]>; [email protected] <[email protected]> On Sat, Jan 29, 2022 at 11:19:12AM +0900, Michael Paquier wrote: > On Thu, Jan 27, 2022 at 10:18:15AM -0800, Nathan Bossart wrote: >> Alright. I think the comment adjustments still apply, so I split those out >> to a new patch. > > Looks fine after a second look, so applied. Thanks! > As of the issues of this thread, we really have two things to think > about: > 1) How do we want to control the access of some parameters in a > context or another? One idea would be more control through GUCs, say > with a set of context-related flags that prevent the read of some > variables until they are set. We could encourage the use of > GetConfigOption() for that. For MaxBackends, we could add a read-only > GUC for this purpose. That's what Andres hinted at upthread, I > guess. > 2) How do we deal with unwanted access of shared parameters? This one > is not really controllable, is it? And we are talking about much more > than MaxBackends. This could perhaps be addressed with more > documentation in the headers for the concerned variables, as a first > step. Hm. Perhaps we should understand the full scope of the problem first. What else besides MaxBackends and the shared memory GUCs won't be properly initialized when the shared_preload_libraries' _PG_init() functions are called? MaxBackends seems to be the only one that folks have experienced problems with, which is why I initially zeroed in on it. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make MaxBackends available in _PG_init @ 2022-01-29 05:24 Michael Paquier <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Michael Paquier @ 2022-01-29 05:24 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Robert Haas <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Andres Freund <[email protected]>; Bharath Rupireddy <[email protected]>; Greg Sabino Mullane <[email protected]>; Tom Lane <[email protected]>; [email protected] <[email protected]> On Fri, Jan 28, 2022 at 08:33:42PM -0800, Nathan Bossart wrote: > Hm. Perhaps we should understand the full scope of the problem first. > What else besides MaxBackends and the shared memory GUCs won't be properly > initialized when the shared_preload_libraries' _PG_init() functions are > called? MaxBackends seems to be the only one that folks have experienced > problems with, which is why I initially zeroed in on it. Yeah, it would be good to know the scope before defining the limits of what could be done. Another thing may be the interactions with session_preload_libraries and local_preload_libraries. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make MaxBackends available in _PG_init @ 2022-01-31 07:11 Michael Paquier <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Michael Paquier @ 2022-01-31 07:11 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Robert Haas <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Andres Freund <[email protected]>; Bharath Rupireddy <[email protected]>; Greg Sabino Mullane <[email protected]>; Tom Lane <[email protected]>; [email protected] <[email protected]> On Sat, Jan 29, 2022 at 02:24:24PM +0900, Michael Paquier wrote: > Yeah, it would be good to know the scope before defining the limits > of what could be done. Another thing may be the interactions with > session_preload_libraries and local_preload_libraries. Worth noting that I have marked marked this patch as committed in the CF app, as there is nothing else to do for now. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v3 01/10] ci: Don't specify amount of memory @ 2023-08-08 00:31 Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2023-08-08 00:31 UTC (permalink / raw) The number of CPUs is the cost-determining factor. Most instance types that run tests have more memory/core than what we specified, there's no real benefit in wasting that. Discussion: https://postgr.es/m/[email protected] Backpatch: 15-, where CI support was added --- .cirrus.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 727c434de40..9e84eb95be5 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -149,7 +149,6 @@ task: image: family/pg-ci-freebsd-13 platform: freebsd cpu: $CPUS - memory: 4G disk: 50 sysinfo_script: | @@ -291,7 +290,6 @@ task: image: family/pg-ci-bullseye platform: linux cpu: $CPUS - memory: 4G ccache_cache: folder: ${CCACHE_DIR} @@ -558,7 +556,6 @@ task: image: family/pg-ci-windows-ci-vs-2019 platform: windows cpu: $CPUS - memory: 4G setup_additional_packages_script: | REM choco install -y --no-progress ... @@ -606,7 +603,6 @@ task: image: family/pg-ci-windows-ci-mingw64 platform: windows cpu: $CPUS - memory: 4G env: TEST_JOBS: 4 # higher concurrency causes occasional failures -- 2.38.0 --uh2yukyzfvojbe2k Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0002-ci-Move-execution-method-of-tasks-into-yaml-templ.patch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1 8/9] ci: Don't specify amount of memory @ 2023-08-08 00:31 Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2023-08-08 00:31 UTC (permalink / raw) The number of CPUs is the cost-determining factor. Most instance types that run tests have more memory/core than what we specified, there's no real benefit in wasting that. --- .cirrus.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index ef825485826..a4d64955489 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -150,7 +150,6 @@ task: image: family/pg-ci-freebsd-13 platform: freebsd cpu: $CPUS - memory: 4G disk: 50 sysinfo_script: | @@ -292,7 +291,6 @@ task: image: family/pg-ci-bullseye platform: linux cpu: $CPUS - memory: 4G ccache_cache: folder: ${CCACHE_DIR} @@ -554,7 +552,6 @@ task: image: family/pg-ci-windows-ci-vs-2019 platform: windows cpu: $CPUS - memory: 4G setup_additional_packages_script: | REM choco install -y --no-progress ... @@ -604,7 +601,6 @@ task: image: family/pg-ci-windows-ci-mingw64 platform: windows cpu: $CPUS - memory: 4G env: TEST_JOBS: 4 # higher concurrency causes occasional failures -- 2.38.0 --uu4yojthqnm7ulmd-- ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v3 01/10] ci: Don't specify amount of memory @ 2023-08-08 00:31 Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2023-08-08 00:31 UTC (permalink / raw) The number of CPUs is the cost-determining factor. Most instance types that run tests have more memory/core than what we specified, there's no real benefit in wasting that. Discussion: https://postgr.es/m/[email protected] Backpatch: 15-, where CI support was added --- .cirrus.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 727c434de40..9e84eb95be5 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -149,7 +149,6 @@ task: image: family/pg-ci-freebsd-13 platform: freebsd cpu: $CPUS - memory: 4G disk: 50 sysinfo_script: | @@ -291,7 +290,6 @@ task: image: family/pg-ci-bullseye platform: linux cpu: $CPUS - memory: 4G ccache_cache: folder: ${CCACHE_DIR} @@ -558,7 +556,6 @@ task: image: family/pg-ci-windows-ci-vs-2019 platform: windows cpu: $CPUS - memory: 4G setup_additional_packages_script: | REM choco install -y --no-progress ... @@ -606,7 +603,6 @@ task: image: family/pg-ci-windows-ci-mingw64 platform: windows cpu: $CPUS - memory: 4G env: TEST_JOBS: 4 # higher concurrency causes occasional failures -- 2.38.0 --uh2yukyzfvojbe2k Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0002-ci-Move-execution-method-of-tasks-into-yaml-templ.patch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1 8/9] ci: Don't specify amount of memory @ 2023-08-08 00:31 Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2023-08-08 00:31 UTC (permalink / raw) The number of CPUs is the cost-determining factor. Most instance types that run tests have more memory/core than what we specified, there's no real benefit in wasting that. --- .cirrus.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index ef825485826..a4d64955489 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -150,7 +150,6 @@ task: image: family/pg-ci-freebsd-13 platform: freebsd cpu: $CPUS - memory: 4G disk: 50 sysinfo_script: | @@ -292,7 +291,6 @@ task: image: family/pg-ci-bullseye platform: linux cpu: $CPUS - memory: 4G ccache_cache: folder: ${CCACHE_DIR} @@ -554,7 +552,6 @@ task: image: family/pg-ci-windows-ci-vs-2019 platform: windows cpu: $CPUS - memory: 4G setup_additional_packages_script: | REM choco install -y --no-progress ... @@ -604,7 +601,6 @@ task: image: family/pg-ci-windows-ci-mingw64 platform: windows cpu: $CPUS - memory: 4G env: TEST_JOBS: 4 # higher concurrency causes occasional failures -- 2.38.0 --uu4yojthqnm7ulmd-- ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1 8/9] ci: Don't specify amount of memory @ 2023-08-08 00:31 Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2023-08-08 00:31 UTC (permalink / raw) The number of CPUs is the cost-determining factor. Most instance types that run tests have more memory/core than what we specified, there's no real benefit in wasting that. --- .cirrus.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index ef825485826..a4d64955489 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -150,7 +150,6 @@ task: image: family/pg-ci-freebsd-13 platform: freebsd cpu: $CPUS - memory: 4G disk: 50 sysinfo_script: | @@ -292,7 +291,6 @@ task: image: family/pg-ci-bullseye platform: linux cpu: $CPUS - memory: 4G ccache_cache: folder: ${CCACHE_DIR} @@ -554,7 +552,6 @@ task: image: family/pg-ci-windows-ci-vs-2019 platform: windows cpu: $CPUS - memory: 4G setup_additional_packages_script: | REM choco install -y --no-progress ... @@ -604,7 +601,6 @@ task: image: family/pg-ci-windows-ci-mingw64 platform: windows cpu: $CPUS - memory: 4G env: TEST_JOBS: 4 # higher concurrency causes occasional failures -- 2.38.0 --uu4yojthqnm7ulmd-- ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2023-08-08 00:31 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-07-09 11:36 [PATCH 4/4] add force_incremental_sort GUC Tomas Vondra <[email protected]> 2019-07-22 12:57 [PATCH 1/2] Refactor checks for deleted GiST pages. Heikki Linnakangas <[email protected]> 2022-01-29 04:33 Re: make MaxBackends available in _PG_init Nathan Bossart <[email protected]> 2022-01-29 05:24 ` Re: make MaxBackends available in _PG_init Michael Paquier <[email protected]> 2022-01-31 07:11 ` Re: make MaxBackends available in _PG_init Michael Paquier <[email protected]> 2023-08-08 00:31 [PATCH v1 8/9] ci: Don't specify amount of memory Andres Freund <[email protected]> 2023-08-08 00:31 [PATCH v3 01/10] ci: Don't specify amount of memory Andres Freund <[email protected]> 2023-08-08 00:31 [PATCH v1 8/9] ci: Don't specify amount of memory Andres Freund <[email protected]> 2023-08-08 00:31 [PATCH v3 01/10] ci: Don't specify amount of memory Andres Freund <[email protected]> 2023-08-08 00:31 [PATCH v1 8/9] ci: Don't specify amount of memory Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox