diff --git a/src/chunk.c b/src/chunk.c index 8baf3cac9..c770c5fc3 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -1930,7 +1930,7 @@ ts_chunk_set_compressed_chunk(Chunk *chunk, int32 compressed_chunk_id, bool isnu &compress_id, 0, ForwardScanDirection, - AccessShareLock, + RowExclusiveLock, CurrentMemoryContext) > 0; } diff --git a/src/compression_with_clause.c b/src/compression_with_clause.c index 0c39c3145..91d8f7c5f 100644 --- a/src/compression_with_clause.c +++ b/src/compression_with_clause.c @@ -82,7 +82,7 @@ parse_segment_collist(char *inpstr, Hypertable *hypertable) RawStmt *raw; #endif - if(strlen(inpstr) == 0) + if (strlen(inpstr) == 0) return NIL; initStringInfo(&buf); @@ -174,7 +174,7 @@ parse_order_collist(char *inpstr, Hypertable *hypertable) RawStmt *raw; #endif - if(strlen(inpstr) == 0) + if (strlen(inpstr) == 0) return NIL; initStringInfo(&buf); diff --git a/tsl/src/compression/compress_utils.c b/tsl/src/compression/compress_utils.c index d88094faa..572d41a2e 100644 --- a/tsl/src/compression/compress_utils.c +++ b/tsl/src/compression/compress_utils.c @@ -190,6 +190,11 @@ compress_chunk_impl(Oid hypertable_relid, Oid chunk_relid) LockRelationOid(cxt.compress_ht->main_table_relid, AccessShareLock); LockRelationOid(cxt.srcht_chunk->table_id, AccessShareLock); /*upgrade when needed */ + /* aquire locks on catalog tables to keep till end of txn */ + LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE_COMPRESSION), + AccessShareLock); + LockRelationOid(catalog_get_table_id(ts_catalog_get(), CHUNK), RowExclusiveLock); + // get compression properties for hypertable htcols_list = get_hypertablecompression_info(cxt.srcht->fd.id); htcols_listlen = list_length(htcols_list); @@ -262,9 +267,14 @@ decompress_chunk_impl(Oid uncompressed_hypertable_relid, Oid uncompressed_chunk_ LockRelationOid(compressed_hypertable->main_table_relid, AccessShareLock); LockRelationOid(uncompressed_chunk->table_id, AccessShareLock); /*upgrade when needed */ + /* aquire locks on catalog tables to keep till end of txn */ + LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE_COMPRESSION), + AccessShareLock); + LockRelationOid(catalog_get_table_id(ts_catalog_get(), CHUNK), RowExclusiveLock); + decompress_chunk(compressed_chunk->table_id, uncompressed_chunk->table_id); compression_chunk_size_delete(uncompressed_chunk->fd.id); - ts_chunk_set_compressed_chunk(uncompressed_chunk, 0, true); + ts_chunk_set_compressed_chunk(uncompressed_chunk, INVALID_CHUNK_ID, true); ts_chunk_drop(compressed_chunk, false, -1); ts_cache_release(hcache); diff --git a/tsl/src/compression/create.c b/tsl/src/compression/create.c index f10e685e5..e6b2e1e34 100644 --- a/tsl/src/compression/create.c +++ b/tsl/src/compression/create.c @@ -395,17 +395,24 @@ tsl_process_compress_table(AlterTableCmd *cmd, Hypertable *ht, int32 compress_htid; struct CompressColInfo compress_cols; bool compress_enable = DatumGetBool(with_clause_options[CompressEnabled].parsed); - Oid ownerid = ts_rel_get_owner(ht->main_table_relid); - List *segmentby_cols = ts_compress_hypertable_parse_segment_by(with_clause_options, ht); - List *orderby_cols = ts_compress_hypertable_parse_order_by(with_clause_options, ht); - orderby_cols = add_time_to_order_by_if_not_included(orderby_cols, segmentby_cols, ht); + Oid ownerid; + List *segmentby_cols; + List *orderby_cols; + bool compression_already_enabled; + bool compressed_chunks_exist; - bool compression_already_enabled = ht->fd.compressed_hypertable_id != INVALID_HYPERTABLE_ID; - bool compressed_chunks_exist = + /* Lock the uncompressed ht in exclusive mode and keep till end of txn */ + LockRelationOid(ht->main_table_relid, AccessExclusiveLock); + + /* reload info after lock */ + ht = ts_hypertable_get_by_id(ht->fd.id); + ownerid = ts_rel_get_owner(ht->main_table_relid); + segmentby_cols = ts_compress_hypertable_parse_segment_by(with_clause_options, ht); + orderby_cols = ts_compress_hypertable_parse_order_by(with_clause_options, ht); + orderby_cols = add_time_to_order_by_if_not_included(orderby_cols, segmentby_cols, ht); + compression_already_enabled = ht->fd.compressed_hypertable_id != INVALID_HYPERTABLE_ID; + compressed_chunks_exist = compression_already_enabled && ts_chunk_exists_with_compression(ht->fd.id); - /* we need an AccessShare lock on the hypertable so that there are - * no DDL changes while we create the compressed hypertable - */ if (!compress_enable) { @@ -420,7 +427,8 @@ tsl_process_compress_table(AlterTableCmd *cmd, Hypertable *ht, if (compressed_chunks_exist) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot change compression options if compressed chunks exist"))); + errmsg("cannot change compression options as compressed chunks already exist for " + "this table"))); /* Require both order by and segment by when altering because otherwise it's not clear what * the default value means: does it mean leave as-is or is it an empty list. */ @@ -433,7 +441,11 @@ tsl_process_compress_table(AlterTableCmd *cmd, Hypertable *ht, compresscolinfo_init(&compress_cols, ht->main_table_relid, segmentby_cols, orderby_cols); - LockRelationOid(ht->main_table_relid, AccessShareLock); + /* take explicit locks on catalog tables and keep them till end of txn */ + LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE), RowExclusiveLock); + LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE_COMPRESSION), + RowExclusiveLock); + if (compression_already_enabled) { Hypertable *compressed = ts_hypertable_get_by_id(ht->fd.compressed_hypertable_id); @@ -446,8 +458,6 @@ tsl_process_compress_table(AlterTableCmd *cmd, Hypertable *ht, } compress_htid = create_compression_table(ownerid, &compress_cols); - /* block concurrent DDL that creates same compressed hypertable*/ - LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE), RowExclusiveLock); ts_hypertable_set_compressed_id(ht, compress_htid); compresscolinfo_add_catalog_entries(&compress_cols, ht->fd.id); /* do not release any locks, will get released by xact end */ diff --git a/tsl/test/expected/compression_errors.out b/tsl/test/expected/compression_errors.out index fc9d15bce..e8be874ad 100644 --- a/tsl/test/expected/compression_errors.out +++ b/tsl/test/expected/compression_errors.out @@ -159,7 +159,7 @@ select compress_chunk(ch1.schema_name|| '.' || ch1.table_name) FROM _timescaledb_catalog.chunk ch1, _timescaledb_catalog.hypertable ht where ch1.hypertable_id = ht.id and ht.table_name like 'non_compressed' limit 1; ERROR: chunks can be compressed only if compression property is set on the hypertable ALTER TABLE foo set (timescaledb.compress, timescaledb.compress_orderby = 'a', timescaledb.compress_segmentby = 'c'); -ERROR: cannot change compression options if compressed chunks exist +ERROR: cannot change compression options as compressed chunks already exist for this table select decompress_chunk(ch1.schema_name|| '.' || ch1.table_name) FROM _timescaledb_catalog.chunk ch1, _timescaledb_catalog.hypertable ht where ch1.hypertable_id = ht.id and ht.table_name like 'non_compressed' limit 1; ERROR: missing compressed hypertable