Fix removal of metadata function and update script

Changing the code to remove the assumption of 1:1 mapping between chunks and chunk constraints. Including a check if a chunk constraint is shared with another chunk. In that case, skip deleting the dimension slice.
timescale · Jun 5, 2024 · 5a3eb80 · 5a3eb80
1 parent 577b923
commit 5a3eb80
Show file tree

Hide file tree

Showing 5 changed files with 208 additions and 0 deletions.
diff --git a/.unreleased/pr_6996 b/.unreleased/pr_6996
@@ -0,0 +1,2 @@
+Fixes: #6976 Fix removal of metadata function and update script
+Thanks: @gugu for reporting the issue with catalog corruption due to update
diff --git a/sql/maintenance_utils.sql b/sql/maintenance_utils.sql
@@ -144,6 +144,11 @@ BEGIN
         USING _timescaledb_catalog.chunk_constraint
         WHERE dimension_slice.id = chunk_constraint.dimension_slice_id
         AND chunk_constraint.chunk_id = _chunk_id
+        AND NOT EXISTS (
+            SELECT FROM _timescaledb_catalog.chunk_constraint cc
+            WHERE cc.chunk_id <> _chunk_id
+            AND cc.dimension_slice_id = dimension_slice.id
+        )
         RETURNING _timescaledb_catalog.dimension_slice.id
     )
     DELETE FROM _timescaledb_catalog.chunk_constraint

diff --git a/sql/updates/2.14.2--2.15.0.sql b/sql/updates/2.14.2--2.15.0.sql
@@ -49,6 +49,11 @@ BEGIN
         USING _timescaledb_catalog.chunk_constraint
         WHERE dimension_slice.id = chunk_constraint.dimension_slice_id
         AND chunk_constraint.chunk_id = _chunk_id
+        AND NOT EXISTS (
+            SELECT FROM _timescaledb_catalog.chunk_constraint cc
+            WHERE cc.chunk_id <> _chunk_id
+            AND cc.dimension_slice_id = dimension_slice.id
+        )
         RETURNING _timescaledb_catalog.dimension_slice.id
     )
     DELETE FROM _timescaledb_catalog.chunk_constraint

diff --git a/tsl/test/expected/cagg_migrate.out b/tsl/test/expected/cagg_migrate.out
@@ -2955,3 +2955,116 @@ DROP MATERIALIZED VIEW conditions_summary_daily;
 DROP MATERIALIZED VIEW conditions_summary_weekly;
 DROP TABLE conditions CASCADE;
 psql:include/cagg_migrate_custom_timezone.sql:23: NOTICE:  drop cascades to 3 other objects
+-- #########################################################
+-- Issue 6976 - space partitioning should not cause catalog corruption
+-- #########################################################
+CREATE TABLE space_partitioning (
+	time timestamptz,
+	device_id integer,
+	value float
+);
+-- Updating sequence numbers so creating a hypertable doesn't mess with
+-- data imports used by migration tests
+SELECT setval('_timescaledb_catalog.hypertable_id_seq', 999, true);
+ setval 
+--------
+    999
+(1 row)
+
+SELECT setval('_timescaledb_catalog.chunk_id_seq', 999, true);
+ setval 
+--------
+    999
+(1 row)
+
+SELECT setval('_timescaledb_catalog.dimension_id_seq', 999, true);
+ setval 
+--------
+    999
+(1 row)
+
+SELECT setval('_timescaledb_catalog.dimension_slice_id_seq', 999, true);
+ setval 
+--------
+    999
+(1 row)
+
+SELECT create_hypertable('space_partitioning', 'time', chunk_time_interval=>'1 hour'::interval);
+NOTICE:  adding not-null constraint to column "time"
+         create_hypertable          
+------------------------------------
+ (1000,public,space_partitioning,t)
+(1 row)
+
+SELECT add_dimension('space_partitioning', 'device_id', 3);
+                add_dimension                 
+----------------------------------------------
+ (1001,public,space_partitioning,device_id,t)
+(1 row)
+
+INSERT INTO space_partitioning SELECT t, 1, 1.0 FROM generate_series('2024-01-01'::timestamptz, '2024-02-01'::timestamptz, '10 minutes'::interval) t;
+INSERT INTO space_partitioning SELECT t, 1000, 1.0 FROM generate_series('2024-01-01'::timestamptz, '2024-02-01'::timestamptz, '10 minutes'::interval) t;
+CREATE MATERIALIZED VIEW space_partitioning_summary
+WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
+SELECT
+    time_bucket(INTERVAL '1 week', "time") AS bucket,
+    device_id,
+    MIN(value),
+    MAX(value),
+    SUM(value)
+FROM
+    space_partitioning
+GROUP BY
+    1, 2
+WITH NO DATA;
+-- setting up the state so that remove_dropped_chunk_metadata
+-- would run on the hypertable and trigger the catalog corruption
+UPDATE _timescaledb_catalog.chunk
+SET dropped = TRUE
+FROM _timescaledb_catalog.hypertable
+WHERE  chunk.hypertable_id = hypertable.id
+AND hypertable.table_name = 'space_partitioning'
+AND chunk.id = 1000;
+UPDATE _timescaledb_catalog.continuous_agg
+SET finalized = true
+FROM _timescaledb_catalog.hypertable
+WHERE continuous_agg.raw_hypertable_id = hypertable.id
+AND hypertable.table_name = 'space_partitioning';
+SET timescaledb.restoring TO ON;
+DROP TABLE _timescaledb_internal._hyper_1000_1000_chunk;
+SET timescaledb.restoring TO OFF;
+SELECT _timescaledb_functions.remove_dropped_chunk_metadata(id)
+FROM _timescaledb_catalog.hypertable
+WHERE table_name = 'space_partitioning';
+INFO:  Removing metadata of chunk 1000 from hypertable 1000
+ remove_dropped_chunk_metadata 
+-------------------------------
+                             1
+(1 row)
+
+-- check every chunk has as many chunk constraints as
+-- there are dimensions, should return empty result
+-- this ensures we have avoided catalog corruption
+WITH dimension_count as (
+SELECT ht.id, count(*)
+FROM _timescaledb_catalog.hypertable ht
+INNER JOIN _timescaledb_catalog.dimension d
+	ON d.hypertable_id = ht.id
+WHERE table_name = 'space_partitioning'
+GROUP BY 1),
+chunk_constraint_count AS (
+SELECT c.hypertable_id, cc.chunk_id, count(*)
+FROM _timescaledb_catalog.chunk_constraint cc
+INNER JOIN _timescaledb_catalog.chunk c
+	ON cc.chunk_id = c.id
+GROUP BY 1, 2
+)
+SELECT *
+FROM dimension_count dc
+INNER JOIN chunk_constraint_count ccc
+	ON ccc.hypertable_id = dc.id
+WHERE dc.count != ccc.count;
+ id | count | hypertable_id | chunk_id | count 
+----+-------+---------------+----------+-------
+(0 rows)
+
diff --git a/tsl/test/sql/cagg_migrate.sql b/tsl/test/sql/cagg_migrate.sql
@@ -38,3 +38,86 @@ SET timezone = 'Europe/Budapest';
 -- Test with timestamptz
 \set TIME_DIMENSION_DATATYPE TIMESTAMPTZ
 \ir include/cagg_migrate_custom_timezone.sql
+
+-- #########################################################
+-- Issue 6976 - space partitioning should not cause catalog corruption
+-- #########################################################
+
+CREATE TABLE space_partitioning (
+	time timestamptz,
+	device_id integer,
+	value float
+);
+
+-- Updating sequence numbers so creating a hypertable doesn't mess with
+-- data imports used by migration tests
+SELECT setval('_timescaledb_catalog.hypertable_id_seq', 999, true);
+SELECT setval('_timescaledb_catalog.chunk_id_seq', 999, true);
+SELECT setval('_timescaledb_catalog.dimension_id_seq', 999, true);
+SELECT setval('_timescaledb_catalog.dimension_slice_id_seq', 999, true);
+
+SELECT create_hypertable('space_partitioning', 'time', chunk_time_interval=>'1 hour'::interval);
+SELECT add_dimension('space_partitioning', 'device_id', 3);
+
+INSERT INTO space_partitioning SELECT t, 1, 1.0 FROM generate_series('2024-01-01'::timestamptz, '2024-02-01'::timestamptz, '10 minutes'::interval) t;
+INSERT INTO space_partitioning SELECT t, 1000, 1.0 FROM generate_series('2024-01-01'::timestamptz, '2024-02-01'::timestamptz, '10 minutes'::interval) t;
+
+CREATE MATERIALIZED VIEW space_partitioning_summary
+WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
+SELECT
+    time_bucket(INTERVAL '1 week', "time") AS bucket,
+    device_id,
+    MIN(value),
+    MAX(value),
+    SUM(value)
+FROM
+    space_partitioning
+GROUP BY
+    1, 2
+WITH NO DATA;
+
+-- setting up the state so that remove_dropped_chunk_metadata
+-- would run on the hypertable and trigger the catalog corruption
+UPDATE _timescaledb_catalog.chunk
+SET dropped = TRUE
+FROM _timescaledb_catalog.hypertable
+WHERE  chunk.hypertable_id = hypertable.id
+AND hypertable.table_name = 'space_partitioning'
+AND chunk.id = 1000;
+UPDATE _timescaledb_catalog.continuous_agg
+SET finalized = true
+FROM _timescaledb_catalog.hypertable
+WHERE continuous_agg.raw_hypertable_id = hypertable.id
+AND hypertable.table_name = 'space_partitioning';
+SET timescaledb.restoring TO ON;
+DROP TABLE _timescaledb_internal._hyper_1000_1000_chunk;
+SET timescaledb.restoring TO OFF;
+
+SELECT _timescaledb_functions.remove_dropped_chunk_metadata(id)
+FROM _timescaledb_catalog.hypertable
+WHERE table_name = 'space_partitioning';
+
+
+-- check every chunk has as many chunk constraints as
+-- there are dimensions, should return empty result
+-- this ensures we have avoided catalog corruption
+WITH dimension_count as (
+SELECT ht.id, count(*)
+FROM _timescaledb_catalog.hypertable ht
+INNER JOIN _timescaledb_catalog.dimension d
+	ON d.hypertable_id = ht.id
+WHERE table_name = 'space_partitioning'
+GROUP BY 1),
+chunk_constraint_count AS (
+SELECT c.hypertable_id, cc.chunk_id, count(*)
+FROM _timescaledb_catalog.chunk_constraint cc
+INNER JOIN _timescaledb_catalog.chunk c
+	ON cc.chunk_id = c.id
+GROUP BY 1, 2
+)
+SELECT *
+FROM dimension_count dc
+INNER JOIN chunk_constraint_count ccc
+	ON ccc.hypertable_id = dc.id
+WHERE dc.count != ccc.count;
+