Hello!
Tomáš Vondra has shared a few ideas to improve BRIN index in czech
PostgreSQL mail list some time ago [1 , in czech only]. This is first
try to implement one of those ideas.
Currently BRIN index blocks HOT update even it is not linked tuples
directly. I'm attaching the initial patch allowing HOT update even on
BRIN indexed columns. This patch went through an initial review on
czech PostgreSQL mail list [1].
It can be viewed online (latest version) on GitHub [2] as well.
- small overview
1. I have added "amhotblocking" flag to index AM descriptor set to
"true" for all, except BRIN, index types. And later in heap_update
method (heapam.c) I do filter attributes based on this new flag,
instead of currently checking for any existing index.
2. I had to enhance the "RelationGetIndexAttrBitmap" function to be
able to return a bitmap of index attribute numbers related to the new
AM flag using "INDEX_ATTR_BITMAP_HOT_BLOCKING" filter.
PS: Originally the "INDEX_ATTR_BITMAP_ALL" filter was used for HOT
check update and most likely could be removed (including all logic
related in RelationGetIndexAttrBitmap), since I have not found any
other usage.
3. I have created an initial regression test using
"pg_stat_get_tuples_hot_updated" to find out HOT was successful on the
BRIN indexed column. Unfortunately "pg_stat_get_tuples_hot_updated" is
not updated immediately and I have not found any way to enforce the
update. Thus (at least for now) I have used a similar approach to
stats.sql using the "wait_for_stats" function (waiting for 30 seconds
and checking each 100ms for change).
I'm attaching patch to CF 2021-07.
[1] https://groups.google.com/g/postgresql-cz/c/oxA_v3H17Qg
[2] https://github.com/simi/postgres/pull/7
From a997caa436757000970e2794cb44901b9e7cf6d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= <josef.sima...@gmail.com>
Date: Sun, 13 Jun 2021 20:12:40 +0200
Subject: [PATCH] Allow HOT for BRIN indexed columns.
---
src/backend/access/brin/brin.c | 1 +
src/backend/access/gin/ginutil.c | 1 +
src/backend/access/gist/gist.c | 1 +
src/backend/access/hash/hash.c | 1 +
src/backend/access/heap/heapam.c | 2 +-
src/backend/access/nbtree/nbtree.c | 1 +
src/backend/access/spgist/spgutils.c | 1 +
src/backend/utils/cache/relcache.c | 14 ++++++
src/include/access/amapi.h | 2 +
src/include/utils/rel.h | 1 +
src/include/utils/relcache.h | 3 +-
.../modules/dummy_index_am/dummy_index_am.c | 1 +
src/test/regress/expected/brin.out | 49 +++++++++++++++++++
src/test/regress/sql/brin.sql | 46 +++++++++++++++++
14 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index ccc9fa0959a9..f521bb963567 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -108,6 +108,7 @@ brinhandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = false;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = false;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_CLEANUP;
amroutine->amkeytype = InvalidOid;
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index cdd626ff0a44..878a041be073 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -56,6 +56,7 @@ ginhandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = false;
amroutine->amusemaintenanceworkmem = true;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_CLEANUP;
amroutine->amkeytype = InvalidOid;
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 0683f42c2588..d96ce1c0a99b 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -77,6 +77,7 @@ gisthandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = true;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
amroutine->amkeytype = InvalidOid;
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 0752fb38a924..b30b255e021c 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -74,6 +74,7 @@ hashhandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = false;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL;
amroutine->amkeytype = INT4OID;
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 2433998f39bd..d0ef78b84a44 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3224,7 +3224,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
* Note that we get copies of each bitmap, so we need not worry about
* relcache flush happening midway through.
*/
- hot_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_ALL);
+ hot_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_HOT_BLOCKING);
key_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_KEY);
id_attrs = RelationGetIndexAttrBitmap(relation,
INDEX_ATTR_BITMAP_IDENTITY_KEY);
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 1360ab80c1e8..99cd99ee861b 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -113,6 +113,7 @@ bthandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = true;
amroutine->amcaninclude = true;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
amroutine->amkeytype = InvalidOid;
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 9ff280a2526b..c9a037f15819 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -61,6 +61,7 @@ spghandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = true;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
amroutine->amkeytype = InvalidOid;
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 94fbf1aa190c..2220f1389a5b 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5003,6 +5003,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
Bitmapset *uindexattrs; /* columns in unique indexes */
Bitmapset *pkindexattrs; /* columns in the primary index */
Bitmapset *idindexattrs; /* columns in the replica identity */
+ Bitmapset *hotblockingattrs; /* columns with HOT blocking indexes */
List *indexoidlist;
List *newindexoidlist;
Oid relpkindex;
@@ -5023,6 +5024,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
return bms_copy(relation->rd_pkattr);
case INDEX_ATTR_BITMAP_IDENTITY_KEY:
return bms_copy(relation->rd_idattr);
+ case INDEX_ATTR_BITMAP_HOT_BLOCKING:
+ return bms_copy(relation->rd_hotblockingattr);
default:
elog(ERROR, "unknown attrKind %u", attrKind);
}
@@ -5066,6 +5069,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
uindexattrs = NULL;
pkindexattrs = NULL;
idindexattrs = NULL;
+ hotblockingattrs = NULL;
foreach(l, indexoidlist)
{
Oid indexOid = lfirst_oid(l);
@@ -5133,6 +5137,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
indexattrs = bms_add_member(indexattrs,
attrnum - FirstLowInvalidHeapAttributeNumber);
+ if (indexDesc->rd_indam->amhotblocking)
+ hotblockingattrs = bms_add_member(hotblockingattrs,
+ attrnum - FirstLowInvalidHeapAttributeNumber);
+
if (isKey && i < indexDesc->rd_index->indnkeyatts)
uindexattrs = bms_add_member(uindexattrs,
attrnum - FirstLowInvalidHeapAttributeNumber);
@@ -5179,6 +5187,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
bms_free(uindexattrs);
bms_free(pkindexattrs);
bms_free(idindexattrs);
+ bms_free(hotblockingattrs);
bms_free(indexattrs);
goto restart;
@@ -5193,6 +5202,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
relation->rd_pkattr = NULL;
bms_free(relation->rd_idattr);
relation->rd_idattr = NULL;
+ bms_free(relation->rd_hotblockingattr);
+ relation->rd_hotblockingattr = NULL;
/*
* Now save copies of the bitmaps in the relcache entry. We intentionally
@@ -5205,6 +5216,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
relation->rd_keyattr = bms_copy(uindexattrs);
relation->rd_pkattr = bms_copy(pkindexattrs);
relation->rd_idattr = bms_copy(idindexattrs);
+ relation->rd_hotblockingattr = bms_copy(hotblockingattrs);
relation->rd_indexattr = bms_copy(indexattrs);
MemoryContextSwitchTo(oldcxt);
@@ -5219,6 +5231,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
return pkindexattrs;
case INDEX_ATTR_BITMAP_IDENTITY_KEY:
return idindexattrs;
+ case INDEX_ATTR_BITMAP_HOT_BLOCKING:
+ return hotblockingattrs;
default:
elog(ERROR, "unknown attrKind %u", attrKind);
return NULL;
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index d357ebb55980..a0ab70df8938 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -244,6 +244,8 @@ typedef struct IndexAmRoutine
bool amcaninclude;
/* does AM use maintenance_work_mem? */
bool amusemaintenanceworkmem;
+ /* does AM block HOT update? */
+ bool amhotblocking;
/* OR of parallel vacuum flags. See vacuum.h for flags. */
uint8 amparallelvacuumoptions;
/* type of data stored in index, or InvalidOid if variable */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 77d176a93482..d586b6811716 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -159,6 +159,7 @@ typedef struct RelationData
Bitmapset *rd_keyattr; /* cols that can be ref'd by foreign keys */
Bitmapset *rd_pkattr; /* cols included in primary key */
Bitmapset *rd_idattr; /* included in replica identity index */
+ Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */
PublicationActions *rd_pubactions; /* publication actions */
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index f772855ac69d..1bc12df96a9c 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -59,7 +59,8 @@ typedef enum IndexAttrBitmapKind
INDEX_ATTR_BITMAP_ALL,
INDEX_ATTR_BITMAP_KEY,
INDEX_ATTR_BITMAP_PRIMARY_KEY,
- INDEX_ATTR_BITMAP_IDENTITY_KEY
+ INDEX_ATTR_BITMAP_IDENTITY_KEY,
+ INDEX_ATTR_BITMAP_HOT_BLOCKING
} IndexAttrBitmapKind;
extern Bitmapset *RelationGetIndexAttrBitmap(Relation relation,
diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c b/src/test/modules/dummy_index_am/dummy_index_am.c
index 5365b0639ec3..9d409faff540 100644
--- a/src/test/modules/dummy_index_am/dummy_index_am.c
+++ b/src/test/modules/dummy_index_am/dummy_index_am.c
@@ -298,6 +298,7 @@ dihandler(PG_FUNCTION_ARGS)
amroutine->amcanparallel = false;
amroutine->amcaninclude = false;
amroutine->amusemaintenanceworkmem = false;
+ amroutine->amhotblocking = true;
amroutine->amparallelvacuumoptions = VACUUM_OPTION_NO_PARALLEL;
amroutine->amkeytype = InvalidOid;
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index e53d6e488567..08dab22b1c64 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -567,3 +567,52 @@ SELECT * FROM brintest_3 WHERE b < '0';
DROP TABLE brintest_3;
RESET enable_seqscan;
+-- test BRIN index doesn't block HOT update
+CREATE TABLE brin_hot (
+ id integer PRIMARY KEY,
+ val integer NOT NULL
+) WITH (autovacuum_enabled = off, fillfactor = 70);
+INSERT INTO brin_hot SELECT *, 0 FROM generate_series(1, 235);
+CREATE INDEX val_brin ON brin_hot using brin(val);
+CREATE FUNCTION wait_for_hot_stats() RETURNS void AS $$
+DECLARE
+ start_time timestamptz := clock_timestamp();
+ updated bool;
+BEGIN
+ -- we don't want to wait forever; loop will exit after 30 seconds
+ FOR i IN 1 .. 300 LOOP
+ SELECT (pg_stat_get_tuples_hot_updated('brin_hot'::regclass::oid) > 0) INTO updated;
+ EXIT WHEN updated;
+
+ -- wait a little
+ PERFORM pg_sleep_for('100 milliseconds');
+ -- reset stats snapshot so we can test again
+ PERFORM pg_stat_clear_snapshot();
+ END LOOP;
+ -- report time waited in postmaster log (where it won't change test output)
+ RAISE log 'wait_for_hot_stats delayed % seconds',
+ EXTRACT(epoch FROM clock_timestamp() - start_time);
+END
+$$ LANGUAGE plpgsql;
+UPDATE brin_hot SET val = -3 WHERE id = 42;
+-- We can't just call wait_for_hot_stats() at this point, because we only
+-- transmit stats when the session goes idle, and we probably didn't
+-- transmit the last couple of counts yet thanks to the rate-limiting logic
+-- in pgstat_report_stat(). But instead of waiting for the rate limiter's
+-- timeout to elapse, let's just start a new session. The old one will
+-- then send its stats before dying.
+\c -
+SELECT wait_for_hot_stats();
+ wait_for_hot_stats
+--------------------
+
+(1 row)
+
+SELECT pg_stat_get_tuples_hot_updated('brin_hot'::regclass::oid);
+ pg_stat_get_tuples_hot_updated
+--------------------------------
+ 1
+(1 row)
+
+DROP TABLE brin_hot;
+DROP FUNCTION wait_for_hot_stats();
diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sql
index 3bd866d947a1..740e0b77a548 100644
--- a/src/test/regress/sql/brin.sql
+++ b/src/test/regress/sql/brin.sql
@@ -509,3 +509,49 @@ SELECT * FROM brintest_3 WHERE b < '0';
DROP TABLE brintest_3;
RESET enable_seqscan;
+
+-- test BRIN index doesn't block HOT update
+CREATE TABLE brin_hot (
+ id integer PRIMARY KEY,
+ val integer NOT NULL
+) WITH (autovacuum_enabled = off, fillfactor = 70);
+
+INSERT INTO brin_hot SELECT *, 0 FROM generate_series(1, 235);
+CREATE INDEX val_brin ON brin_hot using brin(val);
+
+CREATE FUNCTION wait_for_hot_stats() RETURNS void AS $$
+DECLARE
+ start_time timestamptz := clock_timestamp();
+ updated bool;
+BEGIN
+ -- we don't want to wait forever; loop will exit after 30 seconds
+ FOR i IN 1 .. 300 LOOP
+ SELECT (pg_stat_get_tuples_hot_updated('brin_hot'::regclass::oid) > 0) INTO updated;
+ EXIT WHEN updated;
+
+ -- wait a little
+ PERFORM pg_sleep_for('100 milliseconds');
+ -- reset stats snapshot so we can test again
+ PERFORM pg_stat_clear_snapshot();
+ END LOOP;
+ -- report time waited in postmaster log (where it won't change test output)
+ RAISE log 'wait_for_hot_stats delayed % seconds',
+ EXTRACT(epoch FROM clock_timestamp() - start_time);
+END
+$$ LANGUAGE plpgsql;
+
+UPDATE brin_hot SET val = -3 WHERE id = 42;
+
+-- We can't just call wait_for_hot_stats() at this point, because we only
+-- transmit stats when the session goes idle, and we probably didn't
+-- transmit the last couple of counts yet thanks to the rate-limiting logic
+-- in pgstat_report_stat(). But instead of waiting for the rate limiter's
+-- timeout to elapse, let's just start a new session. The old one will
+-- then send its stats before dying.
+\c -
+
+SELECT wait_for_hot_stats();
+SELECT pg_stat_get_tuples_hot_updated('brin_hot'::regclass::oid);
+
+DROP TABLE brin_hot;
+DROP FUNCTION wait_for_hot_stats();
