public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 6/6] Ignore correlation for new BRIN opclasses 5+ messages / 4 participants [nested] [flat]
* [PATCH 6/6] Ignore correlation for new BRIN opclasses @ 2020-09-12 13:07 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw) The new BRIN opclasses (bloom and minmax-multi) are less sensitive to poorly correlated data, so just assume the data is perfectly correlated during costing. Author: Tomas Vondra <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin_bloom.c | 1 + src/backend/access/brin/brin_minmax_multi.c | 1 + src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++- src/include/access/brin_internal.h | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index 000c2387ee..0137095660 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -385,6 +385,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(BloomOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (BloomOpaque *) diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index bd17da8cd5..6ef5c5f2bf 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -1309,6 +1309,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(MinmaxMultiOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (MinmaxMultiOpaque *) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index d5e61664bc..d0cc145938 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -98,6 +98,7 @@ #include <math.h> #include "access/brin.h" +#include "access/brin_internal.h" #include "access/brin_page.h" #include "access/gin.h" #include "access/table.h" @@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, double minimalRanges; double estimatedRanges; double selec; - Relation indexRel; + Relation indexRel = NULL; + TupleDesc tupdesc = NULL; ListCell *l; VariableStatData vardata; @@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, */ indexRel = index_open(index->indexoid, NoLock); brinGetStats(indexRel, &statsData); + tupdesc = RelationGetDescr(indexRel); index_close(indexRel, NoLock); /* work out the actual number of ranges in the index */ @@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, { IndexClause *iclause = lfirst_node(IndexClause, l); AttrNumber attnum = index->indexkeys[iclause->indexcol]; + FmgrInfo *opcInfoFn; + BrinOpcInfo *opcInfo; + Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol); + bool ignore_correlation; + + opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO); + + opcInfo = (BrinOpcInfo *) + DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid)); + + ignore_correlation = opcInfo->oi_ignore_correlation; /* attempt to lookup stats in relation for this index column */ if (attnum != 0) @@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (sslot.nnumbers > 0) varCorrelation = Abs(sslot.numbers[0]); + if (ignore_correlation) + varCorrelation = 1.0; + if (varCorrelation > *indexCorrelation) *indexCorrelation = varCorrelation; diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index fdaff42722..5fbf8cf9c7 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -30,6 +30,9 @@ typedef struct BrinOpcInfo /* Regular processing of NULLs in BrinValues? */ bool oi_regular_nulls; + /* Ignore correlation during cost estimation */ + bool oi_ignore_correlation; + /* Opaque pointer for the opclass' private use */ void *oi_opaque; -- 2.26.2 --------------310A2AE1CC4C2E2E77559E3D-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: do only critical work during single-user vacuum? @ 2022-02-16 08:43 John Naylor <[email protected]> 0 siblings, 2 replies; 5+ messages in thread From: John Naylor @ 2022-02-16 08:43 UTC (permalink / raw) To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Feb 16, 2022 at 6:17 AM Peter Geoghegan <[email protected]> wrote: > > On Tue, Feb 15, 2022 at 9:28 AM Peter Geoghegan <[email protected]> wrote: > > I did notice from my own testing of the failsafe (by artificially > > inducing wraparound failure using an XID burning C function) that > > autovacuum seemed to totally correct the problem, even when the system > > had already crossed xidStopLimit - it came back on its own. I wasn't > > completely sure of how robust this effect was, though. I'll put some effort in finding any way that it might not be robust. After that, changing the message and docs is trivial. > It seemed worth noting this in comments above > should_attempt_truncation(). Pushed a commit to do that just now. Thanks for that. -- John Naylor EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: do only critical work during single-user vacuum? @ 2022-02-20 04:57 Noah Misch <[email protected]> parent: John Naylor <[email protected]> 1 sibling, 0 replies; 5+ messages in thread From: Noah Misch @ 2022-02-20 04:57 UTC (permalink / raw) To: John Naylor <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Andres Freund <[email protected]>; Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Feb 16, 2022 at 03:43:12PM +0700, John Naylor wrote: > On Wed, Feb 16, 2022 at 6:17 AM Peter Geoghegan <[email protected]> wrote: > > On Tue, Feb 15, 2022 at 9:28 AM Peter Geoghegan <[email protected]> wrote: > > > > I did notice from my own testing of the failsafe (by artificially > > > inducing wraparound failure using an XID burning C function) that > > > autovacuum seemed to totally correct the problem, even when the system > > > had already crossed xidStopLimit - it came back on its own. I wasn't > > > completely sure of how robust this effect was, though. > > I'll put some effort in finding any way that it might not be robust. A VACUUM may create a not-trivially-bounded number of multixacts via FreezeMultiXactId(). In a cluster at multiStopLimit, completing VACUUM without error needs preparation something like: 1. Kill each XID that might appear in a multixact. 2. Resolve each prepared transaction that might appear in a multixact. 3. Run VACUUM. At this point, multiStopLimit is blocking new multixacts from other commands, and the lack of running multixact members removes the need for FreezeMultiXactId() to create multixacts. Adding to the badness of single-user mode so well described upthread, one can enter it without doing (2) and then wrap the nextMXact counter. ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: do only critical work during single-user vacuum? @ 2022-03-15 21:48 Peter Geoghegan <[email protected]> parent: John Naylor <[email protected]> 1 sibling, 1 reply; 5+ messages in thread From: Peter Geoghegan @ 2022-03-15 21:48 UTC (permalink / raw) To: John Naylor <[email protected]>; +Cc: Andres Freund <[email protected]>; Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Feb 16, 2022 at 12:43 AM John Naylor <[email protected]> wrote: > I'll put some effort in finding any way that it might not be robust. > After that, changing the message and docs is trivial. It would be great to be able to totally drop the idea of using single-user mode before Postgres 15 feature freeze. How's that going? I suggest that we apply the following patch as part of that work. It adds one last final failsafe check at the point that VACUUM makes a final decision on rel truncation. It seems unlikely that the patch will ever make the crucial difference in a wraparound scenario -- in practice it's very likely that we'd have triggered the wraparound at that point if we run into trouble with the target rel's relfrozenxid age. And even if it does get to that point, it would still be possible for the autovacuum launcher to launch another autovacuum -- this time around we will avoid rel truncation, restoring the system to normal operation (i.e. no more xidStopLimit state). On the other hand it's possible that lazy_cleanup_all_indexes() will take a very long time to run, and it runs after the current final failsafe check. An index AM's amvacuumcleanup() routine can take a long time to run sometimes, especially with GIN indexes. And so it's just about possible that we won't have triggered the failsafe by the time lazy_cleanup_all_indexes() is called, which then spends a long time doing index cleanup -- long enough for the system to reach xidStopLimit due to the target rel's relfrozenxid age crossing the crucial xidStopLimit crossover point. This patch makes this problem scenario virtually impossible. Right now I'm only prepared to say it's very unlikely. I don't see a reason to take any chances, though. -- Peter Geoghegan Attachments: [application/octet-stream] v1-0001-Perform-final-failsafe-check-before-truncation.patch (922B, ../../CAH2-Wzk0ZrsYx7FxQZ2dGm4XLDNwp2jpNDXwjpASiJNs4HBhDQ@mail.gmail.com/2-v1-0001-Perform-final-failsafe-check-before-truncation.patch) download | inline diff: From 72aebc7854d24fb8466b45536e41cc3d768cd445 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan <[email protected]> Date: Tue, 15 Mar 2022 14:24:12 -0700 Subject: [PATCH v1] Perform final failsafe check before truncation. --- src/backend/access/heap/vacuumlazy.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 87ab7775a..73336560b 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2822,7 +2822,13 @@ should_attempt_truncation(LVRelState *vacrel) if (possibly_freeable > 0 && (possibly_freeable >= REL_TRUNCATE_MINIMUM || possibly_freeable >= vacrel->rel_pages / REL_TRUNCATE_FRACTION)) + { + /* Perform a final failsafe check out of an abundance of caution */ + if (lazy_check_wraparound_failsafe(vacrel)) + return false; + return true; + } return false; } -- 2.30.2 ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: do only critical work during single-user vacuum? @ 2022-03-31 09:51 John Naylor <[email protected]> parent: Peter Geoghegan <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: John Naylor @ 2022-03-31 09:51 UTC (permalink / raw) To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Mar 16, 2022 at 4:48 AM Peter Geoghegan <[email protected]> wrote: > > On Wed, Feb 16, 2022 at 12:43 AM John Naylor > <[email protected]> wrote: > > I'll put some effort in finding any way that it might not be robust. > > After that, changing the message and docs is trivial. > > It would be great to be able to totally drop the idea of using > single-user mode before Postgres 15 feature freeze. How's that going? Unfortunately, I was distracted from this work for a time, and just as I had intended to focus on it during March, I was out sick for 2-3 weeks. I gather from subsequent discussion that a full solution goes beyond just a new warning message and documentation. Either way I'm not quite prepared to address this in time for v15. > I suggest that we apply the following patch as part of that work. It > adds one last final failsafe check at the point that VACUUM makes a > final decision on rel truncation. That is one thing that was in the back of my mind, and it seems reasonable to me. -- John Naylor EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2022-03-31 09:51 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-12 13:07 [PATCH 6/6] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]> 2022-02-16 08:43 Re: do only critical work during single-user vacuum? John Naylor <[email protected]> 2022-02-20 04:57 ` Re: do only critical work during single-user vacuum? Noah Misch <[email protected]> 2022-03-15 21:48 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]> 2022-03-31 09:51 ` Re: do only critical work during single-user vacuum? John Naylor <[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