Skip to content

Commit

Permalink
Fix compression settings handling in Hypercore TAM
Browse files Browse the repository at this point in the history
Fix a bug which caused compression settings to remain after having
converted a Hypercore TAM chunk back to another TAM (e.g., heap).

Remove any orphaned settings in the update script.
  • Loading branch information
erimatnor committed Feb 21, 2025
1 parent c957433 commit e99c106
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 60 deletions.
1 change: 1 addition & 0 deletions .unreleased/pr_7764
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7764 Fix compression settings handling in Hypercore TAM
10 changes: 10 additions & 0 deletions sql/updates/post-update.sql
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,13 @@ $$;
-- Repair relations that have relacl entries for users that do not
-- exist in pg_authid
CALL _timescaledb_functions.repair_relation_acls();

-- Cleanup orphaned compression settings
WITH orphaned_settings AS (
SELECT cs.relid, cl.relname
FROM _timescaledb_catalog.compression_settings cs
LEFT JOIN pg_class cl ON (cs.relid = cl.oid)
WHERE cl.relname IS NULL
)
DELETE FROM _timescaledb_catalog.compression_settings AS cs
USING orphaned_settings AS os WHERE cs.relid = os.relid;
75 changes: 15 additions & 60 deletions tsl/src/hypercore/hypercore_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ get_chunk_id_from_relid(Oid relid)
return chunk_id;
}

static int32
static Oid
chunk_get_compressed_chunk_relid(Oid relid)
{
FormData_chunk fd;
Expand Down Expand Up @@ -3704,36 +3704,6 @@ convert_to_hypercore(Oid relid)
table_close(relation, NoLock);
}

/*
* List of relation IDs used to clean up the compressed relation when
* converting from Hypercore to another TAM (typically heap).
*/
static List *cleanup_relids = NIL;

static void
cleanup_compression_relations(void)
{
if (cleanup_relids != NIL)
{
ListCell *lc;

foreach (lc, cleanup_relids)
{
Oid relid = lfirst_oid(lc);
Chunk *chunk = ts_chunk_get_by_relid(relid, true);
Chunk *compress_chunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, false);

ts_chunk_clear_compressed_chunk(chunk);

if (compress_chunk)
ts_chunk_drop(compress_chunk, DROP_RESTRICT, -1);
}

list_free(cleanup_relids);
cleanup_relids = NIL;
}
}

void
hypercore_xact_event(XactEvent event, void *arg)
{
Expand Down Expand Up @@ -3771,16 +3741,6 @@ hypercore_xact_event(XactEvent event, void *arg)
list_free(partially_compressed_relids);
partially_compressed_relids = NIL;
}

/*
* Cleanup in case of aborted transaction. Need not explicitly check for
* abort since the states should only exist if it is an abort.
*/
if (cleanup_relids != NIL)
{
list_free(cleanup_relids);
cleanup_relids = NIL;
}
}

static void
Expand Down Expand Up @@ -3872,28 +3832,11 @@ convert_to_hypercore_finish(Oid relid)
Assert(conversionstate == NULL);
}

/*
* Convert the chunk away from Hypercore to another table access method.
* When this happens it is necessary to cleanup metadata.
*/
static void
convert_from_hypercore(Oid relid)
{
check_guc_setting_compatible_with_scan();
int32 chunk_id = get_chunk_id_from_relid(relid);
ts_compression_chunk_size_delete(chunk_id);

/* Need to truncate the compressed relation after converting from Hypercore */
MemoryContext oldmcxt = MemoryContextSwitchTo(CurTransactionContext);
cleanup_relids = lappend_oid(cleanup_relids, relid);
MemoryContextSwitchTo(oldmcxt);
}

void
hypercore_alter_access_method_begin(Oid relid, bool to_other_am)
{
if (to_other_am)
convert_from_hypercore(relid);
check_guc_setting_compatible_with_scan();
else
convert_to_hypercore(relid);
}
Expand All @@ -3905,7 +3848,19 @@ void
hypercore_alter_access_method_finish(Oid relid, bool to_other_am)
{
if (to_other_am)
cleanup_compression_relations();
{
Chunk *chunk = ts_chunk_get_by_relid(relid, true);
Chunk *compress_chunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, false);

ts_compression_chunk_size_delete(chunk->fd.id);
ts_chunk_clear_compressed_chunk(chunk);

if (compress_chunk)
{
ts_compression_settings_delete(compress_chunk->table_id);
ts_chunk_drop(compress_chunk, DROP_RESTRICT, -1);
}
}

/* Finishing the conversion to Hypercore is handled in the
* finish_bulk_insert callback */
Expand Down
11 changes: 11 additions & 0 deletions tsl/test/expected/hypercore_create.out
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,17 @@ from compressed_rel_size_stats;
0
(1 row)

-- Compression settings should be removed except for parent
-- hypertables
select cs.relid, cl.relname
from _timescaledb_catalog.compression_settings cs
left join pg_class cl on (cs.relid = cl.oid);
relid | relname
-------+---------
test2 | test2
test3 | test3
(2 rows)

-- Create hypercores again and check that compression size stats are
-- updated showing compressed data
select compress_chunk(ch, hypercore_use_access_method => true)
Expand Down
6 changes: 6 additions & 0 deletions tsl/test/sql/hypercore_create.sql
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ select decompress_chunk(rel)
select count(*) as orphaned_stats
from compressed_rel_size_stats;

-- Compression settings should be removed except for parent
-- hypertables
select cs.relid, cl.relname
from _timescaledb_catalog.compression_settings cs
left join pg_class cl on (cs.relid = cl.oid);

-- Create hypercores again and check that compression size stats are
-- updated showing compressed data
select compress_chunk(ch, hypercore_use_access_method => true)
Expand Down

0 comments on commit e99c106

Please sign in to comment.