Fix locking when altering compression options

Take an exclusive lock when taking compression options as it is
safer.
This commit is contained in:
Matvey Arye 2019-08-19 16:16:46 -04:00 committed by Matvey Arye
parent 0059360522
commit bce292a64f
5 changed files with 38 additions and 18 deletions

View File

@ -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;
}

View File

@ -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);

View File

@ -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);

View File

@ -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 */

View File

@ -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