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]>
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]>
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-16 08:43 Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2022-02-20 04:57 ` Noah Misch <[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-02-16 08:43 Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2022-03-15 21:48 ` Peter Geoghegan <[email protected]>
2022-03-31 09:51 ` Re: do only critical work during single-user vacuum? 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-02-16 08:43 Re: do only critical work during single-user vacuum? John Naylor <[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 ` John Naylor <[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