From cbfd386c63842cb2779a8b598048381eb75a7db2 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Wed, 16 Oct 2024 12:21:12 +0200 Subject: [PATCH] Disable autovacuum on compressed hypercore chunk Autovacuum will run on all relations that are not explicitly disabled, which can lead to unnecessarily vacuuming the compressed relation inside the hypercore table access method twice: once as part of the hypercore table access method and once as a separate relation. This commit disables `autovacuum_enabled` for the compressed chunks when the main chunk is turned into a hypercore access method chunk. It deals with both the cases that you have a partially compressed chunk and that you create a new compressed chunks as part of the conversion. --- src/utils.c | 84 ++++++++++++++++++ src/utils.h | 1 + tsl/src/compression/api.c | 1 + tsl/src/hypercore/hypercore_handler.c | 3 + tsl/src/hypercore/utils.c | 34 +++++++- tsl/src/hypercore/utils.h | 1 + tsl/src/process_utility.c | 1 + tsl/test/expected/hypercore_create.out | 114 +++++++++++++++++++++++-- tsl/test/sql/hypercore_create.sql | 42 ++++++++- 9 files changed, 272 insertions(+), 9 deletions(-) diff --git a/src/utils.c b/src/utils.c index ff4c3580d2e..93bc8a14380 100644 --- a/src/utils.c +++ b/src/utils.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1840,6 +1842,88 @@ ts_is_hypercore_am(Oid amoid) return amoid == hypercore_amoid; } +/* + * Set reloption for relation. + * + * Most of the code is from ATExecSetRelOptions() in tablecmds.c since that + * function is static and we also need to do a slightly different job. + */ +static void +relation_set_reloption_impl(Relation rel, List *options, LOCKMODE lockmode) +{ + Datum repl_val[Natts_pg_class] = { 0 }; + bool repl_null[Natts_pg_class] = { false }; + bool repl_repl[Natts_pg_class] = { false }; + bool isnull; + + Assert(rel->rd_rel->relkind == RELKIND_RELATION || rel->rd_rel->relkind == RELKIND_TOASTVALUE); + + if (options == NIL) + return; /* nothing to do */ + + TS_DEBUG_LOG("setting reloptions for %s", RelationGetRelationName(rel)); + + Relation pgclass = table_open(RelationRelationId, RowExclusiveLock); + Oid relid = RelationGetRelid(rel); + HeapTuple tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", relid); + + /* Get the old reloptions */ + Datum datum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_reloptions, &isnull); + + /* Generate new proposed reloptions (text array) */ + Datum newOptions = + transformRelOptions(isnull ? (Datum) 0 : datum, options, NULL, NULL, false, false); + (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true); + + if (newOptions) + repl_val[AttrNumberGetAttrOffset(Anum_pg_class_reloptions)] = newOptions; + else + repl_null[AttrNumberGetAttrOffset(Anum_pg_class_reloptions)] = true; + + repl_repl[AttrNumberGetAttrOffset(Anum_pg_class_reloptions)] = true; + + HeapTuple newtuple = + heap_modify_tuple(tuple, RelationGetDescr(pgclass), repl_val, repl_null, repl_repl); + + CatalogTupleUpdate(pgclass, &newtuple->t_self, newtuple); + + /* Not sure if we need this one, but keeping it as a precaution */ + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); + + heap_freetuple(newtuple); + ReleaseSysCache(tuple); + table_close(pgclass, RowExclusiveLock); +} + +/* + * Set value of reloptions for given relation. + * + * This will also set the reloption for the relations' associated relations, + * in this case the TOAST table. It is based on ATExecSetRelOptions but we + * split out the code to set the reloptions rather than duplicating it. + * + * The lockmode is needed for taking a correct lock on the toast table for the + * already locked relation. It is only used for + * + * rel: Relation to add reloptions to. + * defList: List of DefElem for the new definitions. + * lockmode: the mode that the actual tables are locked in. + */ +void +ts_relation_set_reloption(Relation rel, List *options, LOCKMODE lockmode) +{ + Assert(RelationIsValid(rel)); + relation_set_reloption_impl(rel, options, lockmode); + if (OidIsValid(rel->rd_rel->reltoastrelid)) + { + Relation toastrel = table_open(rel->rd_rel->reltoastrelid, lockmode); + relation_set_reloption_impl(toastrel, options, lockmode); + table_close(toastrel, NoLock); + } +} + /* this function fills in a jsonb with the non-null fields of the error data and also includes the proc name and schema in the jsonb we include these here to avoid adding these fields to the table */ diff --git a/src/utils.h b/src/utils.h index 3ad6dbe847e..ea7702ed8f4 100644 --- a/src/utils.h +++ b/src/utils.h @@ -388,5 +388,6 @@ extern TSDLLEXPORT void ts_get_rel_info_by_name(const char *relnamespace, const Oid *relid, Oid *amoid, char *relkind); extern TSDLLEXPORT void ts_get_rel_info(Oid relid, Oid *amoid, char *relkind); extern TSDLLEXPORT Oid ts_get_rel_am(Oid relid); +extern TSDLLEXPORT void ts_relation_set_reloption(Relation rel, List *options, LOCKMODE lockmode); extern TSDLLEXPORT bool ts_is_hypercore_am(Oid amoid); extern TSDLLEXPORT Jsonb *ts_errdata_to_jsonb(ErrorData *edata, Name proc_schema, Name proc_name); diff --git a/tsl/src/compression/api.c b/tsl/src/compression/api.c index f4adac8071a..df600276afc 100644 --- a/tsl/src/compression/api.c +++ b/tsl/src/compression/api.c @@ -823,6 +823,7 @@ compress_hypercore(Chunk *chunk, bool rel_is_hypercore, UseAccessMethod useam, /* Do quick migration to hypercore of already compressed data by * simply changing the access method to hypercore in pg_am. */ hypercore_set_am(rv); + hypercore_set_reloptions(chunk); return chunk->table_id; } diff --git a/tsl/src/hypercore/hypercore_handler.c b/tsl/src/hypercore/hypercore_handler.c index 617616ea18a..66de3b3b830 100644 --- a/tsl/src/hypercore/hypercore_handler.c +++ b/tsl/src/hypercore/hypercore_handler.c @@ -3508,6 +3508,9 @@ convert_to_hypercore_finish(Oid relid) ts_chunk_constraints_create(ht_compressed, c_chunk); ts_trigger_create_all_on_chunk(c_chunk); create_proxy_vacuum_index(relation, RelationGetRelid(compressed_rel)); + /* We use makeInteger since makeBoolean does not exist prior to PG15 */ + List *options = list_make1(makeDefElem("autovacuum_enabled", (Node *) makeInteger(0), -1)); + ts_relation_set_reloption(compressed_rel, options, RowExclusiveLock); table_close(relation, NoLock); table_close(compressed_rel, NoLock); diff --git a/tsl/src/hypercore/utils.c b/tsl/src/hypercore/utils.c index 391d0005315..4e8060a6130 100644 --- a/tsl/src/hypercore/utils.c +++ b/tsl/src/hypercore/utils.c @@ -13,14 +13,46 @@ #include #include #include +#include #include #include #include #include "compat/compat.h" +#include "chunk.h" #include "extension_constants.h" +#include "src/utils.h" #include "utils.h" -#include + +/* + * Set reloptions for chunks using hypercore TAM. + * + * This sets all reloptions needed for chunks using the hypercore table access + * method. Right now this means turning of autovacuum for the compressed chunk + * associated with the hypercore chunk by setting the "autovacuum_enabled" + * option to "0" (false). + * + * It is (currently) not necessary to clear this reloption anywhere since we + * (currently) delete the compressed chunk when changing the table access + * method back to "heap". + */ +void +hypercore_set_reloptions(Chunk *chunk) +{ + /* + * Update the tuple for the compressed chunk and disable autovacuum on + * it. This requires locking the relation (to prevent changes to the + * definition), but it is sufficient to take an access share lock. + */ + Chunk *cchunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, true); + Relation compressed_rel = table_open(cchunk->table_id, AccessShareLock); + + /* We use makeInteger since makeBoolean does not exist prior to PG15 */ + List *options = list_make1(makeDefElem("autovacuum_enabled", (Node *) makeInteger(0), -1)); + ts_relation_set_reloption(compressed_rel, options, AccessShareLock); + + table_close(compressed_rel, AccessShareLock); +} /* * Make a relation use hypercore without rewriting any data, simply by diff --git a/tsl/src/hypercore/utils.h b/tsl/src/hypercore/utils.h index eaf60a21bba..3912cf1be2c 100644 --- a/tsl/src/hypercore/utils.h +++ b/tsl/src/hypercore/utils.h @@ -8,3 +8,4 @@ #include extern void hypercore_set_am(const RangeVar *rv); +extern void hypercore_set_reloptions(Chunk *chunk); diff --git a/tsl/src/process_utility.c b/tsl/src/process_utility.c index bb368f7e047..f685f0c5519 100644 --- a/tsl/src/process_utility.c +++ b/tsl/src/process_utility.c @@ -67,6 +67,7 @@ tsl_ddl_command_start(ProcessUtilityArgs *args) if (!is_hypercore && ts_chunk_is_compressed(chunk)) { hypercore_set_am(stmt->relation); + hypercore_set_reloptions(chunk); /* Skip this command in the alter table * statement since we process it via quick * migration */ diff --git a/tsl/test/expected/hypercore_create.out b/tsl/test/expected/hypercore_create.out index e2f024334b4..b290b7cd494 100644 --- a/tsl/test/expected/hypercore_create.out +++ b/tsl/test/expected/hypercore_create.out @@ -58,6 +58,28 @@ select setseed(0.3); (1 row) +-- View to get information about chunks and associated compressed +-- chunks. +create or replace view test_chunk_info as +with + ht_and_chunk as ( + select format('%I.%I', ht.schema_name, ht.table_name)::regclass as hypertable, + format('%I.%I', ch.schema_name, ch.table_name)::regclass as chunk, + case when cc.table_name is not null then + format('%I.%I', cc.schema_name, cc.table_name)::regclass + else null + end as compressed_chunk + from _timescaledb_catalog.chunk ch + left join _timescaledb_catalog.chunk cc on ch.compressed_chunk_id = cc.id + join _timescaledb_catalog.hypertable ht on ch.hypertable_id = ht.id + where ht.compression_state != 2 + ) +select hypertable, + chunk, + (select reloptions from pg_class where oid = chunk) as chunk_reloptions, + compressed_chunk, + (select reloptions from pg_class where oid = compressed_chunk) as compressed_reloptions + from ht_and_chunk; -- Testing the basic API for creating a hypercore -- This should just fail because you cannot create a plain table with -- hypercore (yet). @@ -220,7 +242,25 @@ ERROR: hypertable "test3" is missing compression settings \set ON_ERROR_STOP 1 -- Add compression settings alter table test3 set (timescaledb.compress, timescaledb.compress_orderby='time desc', timescaledb.compress_segmentby=''); +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+---------------------------------------- +hypertable | test3 +chunk | _timescaledb_internal._hyper_4_13_chunk +chunk_reloptions | +compressed_chunk | +compressed_reloptions | + alter table :chunk set access method hypercore; +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+------------------------------------------------ +hypertable | test3 +chunk | _timescaledb_internal._hyper_4_13_chunk +chunk_reloptions | +compressed_chunk | _timescaledb_internal.compress_hyper_5_14_chunk +compressed_reloptions | {toast_tuple_target=128,autovacuum_enabled=0} + +\x off -- Check that chunk is using hypercore select * from amrels where rel=:'chunk'::regclass; rel | amname | relparent @@ -228,8 +268,27 @@ select * from amrels where rel=:'chunk'::regclass; _timescaledb_internal._hyper_4_13_chunk | hypercore | test3 (1 row) --- Try same thing with compress_chunk() +-- Try same thing with compress_chunk(), and check the reloptions +-- before and after +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+------------------------------------------------ +hypertable | test3 +chunk | _timescaledb_internal._hyper_4_13_chunk +chunk_reloptions | +compressed_chunk | _timescaledb_internal.compress_hyper_5_14_chunk +compressed_reloptions | {toast_tuple_target=128,autovacuum_enabled=0} + alter table :chunk set access method heap; +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+---------------------------------------- +hypertable | test3 +chunk | _timescaledb_internal._hyper_4_13_chunk +chunk_reloptions | +compressed_chunk | +compressed_reloptions | + +\x off select compress_chunk(:'chunk', hypercore_use_access_method => true); compress_chunk ----------------------------------------- @@ -301,14 +360,35 @@ alter table test4 set (timescaledb.compress); WARNING: there was some uncertainty picking the default segment by for the hypertable: You do not have any indexes on columns that can be used for segment_by and thus we are not using segment_by for compression. Please make sure you are not missing any indexes NOTICE: default segment by for hypertable "test4" is set to "" NOTICE: default order by for hypertable "test4" is set to ""time" DESC" +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+---------------------------------------- +hypertable | test4 +chunk | _timescaledb_internal._hyper_6_20_chunk +chunk_reloptions | +compressed_chunk | +compressed_reloptions | + alter table :chunk set access method hypercore; -select * from amrels where relparent='test4'::regclass; - rel | amname | relparent ------------------------------------------+-----------+----------- - _timescaledb_internal._hyper_6_20_chunk | hypercore | test4 - _timescaledb_internal._hyper_6_21_chunk | heap | test4 -(2 rows) +select * from test_chunk_info where chunk = :'chunk'::regclass; +-[ RECORD 1 ]---------+------------------------------------------------ +hypertable | test4 +chunk | _timescaledb_internal._hyper_6_20_chunk +chunk_reloptions | +compressed_chunk | _timescaledb_internal.compress_hyper_7_22_chunk +compressed_reloptions | {toast_tuple_target=128,autovacuum_enabled=0} +select * from amrels where relparent='test4'::regclass; +-[ RECORD 1 ]-------------------------------------- +rel | _timescaledb_internal._hyper_6_20_chunk +amname | hypercore +relparent | test4 +-[ RECORD 2 ]-------------------------------------- +rel | _timescaledb_internal._hyper_6_21_chunk +amname | heap +relparent | test4 + +\x off -- test that alter table on the hypertable works alter table test4 add column magic int; \d :chunk @@ -561,6 +641,16 @@ select ch as alter_chunk from show_chunks('test2') ch limit 1 \gset LOG: statement: select ch as alter_chunk from show_chunks('test2') ch limit 1 insert into :alter_chunk values ('2022-06-01 10:00', 4, 4, 4.0, 4.0); LOG: statement: insert into _timescaledb_internal._hyper_1_1_chunk values ('2022-06-01 10:00', 4, 4, 4.0, 4.0); +\x on +select * from test_chunk_info where chunk = :'alter_chunk'::regclass; +LOG: statement: select * from test_chunk_info where chunk = '_timescaledb_internal._hyper_1_1_chunk'::regclass; +-[ RECORD 1 ]---------+------------------------------------------------ +hypertable | test2 +chunk | _timescaledb_internal._hyper_1_1_chunk +chunk_reloptions | +compressed_chunk | _timescaledb_internal.compress_hyper_3_32_chunk +compressed_reloptions | {toast_tuple_target=128} + alter table :alter_chunk set access method hypercore; LOG: statement: alter table _timescaledb_internal._hyper_1_1_chunk set access method hypercore; DEBUG: migrating table "_hyper_1_1_chunk" to hypercore @@ -568,6 +658,16 @@ DEBUG: building index "_hyper_1_1_chunk_test2_device_id_created_at_idx" on tabl DEBUG: index "_hyper_1_1_chunk_test2_device_id_created_at_idx" can safely use deduplication DEBUG: building index "_hyper_1_1_chunk_test2_created_at_idx" on table "_hyper_1_1_chunk" serially DEBUG: index "_hyper_1_1_chunk_test2_created_at_idx" can safely use deduplication +select * from test_chunk_info where chunk = :'alter_chunk'::regclass; +LOG: statement: select * from test_chunk_info where chunk = '_timescaledb_internal._hyper_1_1_chunk'::regclass; +-[ RECORD 1 ]---------+------------------------------------------------ +hypertable | test2 +chunk | _timescaledb_internal._hyper_1_1_chunk +chunk_reloptions | +compressed_chunk | _timescaledb_internal.compress_hyper_3_32_chunk +compressed_reloptions | {toast_tuple_target=128,autovacuum_enabled=0} + +\x off reset client_min_messages; LOG: statement: reset client_min_messages; -- Check pg_am dependencies for the chunks. Since they are using heap diff --git a/tsl/test/sql/hypercore_create.sql b/tsl/test/sql/hypercore_create.sql index 95f39da7725..ca795b0c88c 100644 --- a/tsl/test/sql/hypercore_create.sql +++ b/tsl/test/sql/hypercore_create.sql @@ -5,6 +5,29 @@ \ir include/hypercore_helpers.sql select setseed(0.3); +-- View to get information about chunks and associated compressed +-- chunks. +create or replace view test_chunk_info as +with + ht_and_chunk as ( + select format('%I.%I', ht.schema_name, ht.table_name)::regclass as hypertable, + format('%I.%I', ch.schema_name, ch.table_name)::regclass as chunk, + case when cc.table_name is not null then + format('%I.%I', cc.schema_name, cc.table_name)::regclass + else null + end as compressed_chunk + from _timescaledb_catalog.chunk ch + left join _timescaledb_catalog.chunk cc on ch.compressed_chunk_id = cc.id + join _timescaledb_catalog.hypertable ht on ch.hypertable_id = ht.id + where ht.compression_state != 2 + ) +select hypertable, + chunk, + (select reloptions from pg_class where oid = chunk) as chunk_reloptions, + compressed_chunk, + (select reloptions from pg_class where oid = compressed_chunk) as compressed_reloptions + from ht_and_chunk; + -- Testing the basic API for creating a hypercore -- This should just fail because you cannot create a plain table with @@ -118,13 +141,22 @@ alter table :chunk set access method hypercore; -- Add compression settings alter table test3 set (timescaledb.compress, timescaledb.compress_orderby='time desc', timescaledb.compress_segmentby=''); +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; alter table :chunk set access method hypercore; +select * from test_chunk_info where chunk = :'chunk'::regclass; +\x off -- Check that chunk is using hypercore select * from amrels where rel=:'chunk'::regclass; --- Try same thing with compress_chunk() +-- Try same thing with compress_chunk(), and check the reloptions +-- before and after +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; alter table :chunk set access method heap; +select * from test_chunk_info where chunk = :'chunk'::regclass; +\x off select compress_chunk(:'chunk', hypercore_use_access_method => true); -- Check that chunk is using hypercore @@ -166,8 +198,12 @@ select count(ch) from show_chunks('test4') ch; select ch as chunk from show_chunks('test4') ch limit 1 \gset alter table test4 set (timescaledb.compress); +\x on +select * from test_chunk_info where chunk = :'chunk'::regclass; alter table :chunk set access method hypercore; +select * from test_chunk_info where chunk = :'chunk'::regclass; select * from amrels where relparent='test4'::regclass; +\x off -- test that alter table on the hypertable works alter table test4 add column magic int; @@ -283,7 +319,11 @@ select compress_chunk(ch, hypercore_use_access_method => true) from chunks; -- compressed chunks. select ch as alter_chunk from show_chunks('test2') ch limit 1 \gset insert into :alter_chunk values ('2022-06-01 10:00', 4, 4, 4.0, 4.0); +\x on +select * from test_chunk_info where chunk = :'alter_chunk'::regclass; alter table :alter_chunk set access method hypercore; +select * from test_chunk_info where chunk = :'alter_chunk'::regclass; +\x off reset client_min_messages;