Skip to content

Commit 6d8a0c8

Browse files
yoshinorimGunnar Kudrjavets
authored andcommitted
Fix SIGSEGV on CREATE TABLE contention (issue mysql#104)
Summary: MyRocks did strict dictionary consistency check that newer index_id (passed by CREATE TABLE) must be larger than max_index_id stored in data dictionary. This is not always true. Suppose the following conditions. CREATE TABLE t1: 1) index_id= ddl_manager.get_next_number(); 4) dict_manager.update_max_index_id(batch, index_id) CREATE TABLE t2: 2) index_id= ddl_manager.get_next_number(); 3) dict_manager.update_max_index_id(batch, index_id) MyRocks returned an error on CREATE TABLE t1 because of dictionary condition check (dict_manager.update_max_index_id()). Then MyRocks crashed later by memory violation, because ddl_manager.put_and_write() was not cleaned up after failure. This diff fixes the issue by updating max_index_id in dictionary when getting new index id. This diff also renaming function name from ddl_manager.get_next_number() to ddl_manager.get_and_update_next_number(). Test Plan: mtr Reviewers: maykov, spetrunia, jkedgar, hermanlee4 Reviewed By: hermanlee4 Differential Revision: https://reviews.facebook.net/D45963
1 parent 7efa5cd commit 6d8a0c8

File tree

3 files changed

+17
-11
lines changed

3 files changed

+17
-11
lines changed

storage/rocksdb/ha_rocksdb.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2748,7 +2748,7 @@ int ha_rocksdb::create_key_defs(TABLE *table_arg, const char *db_table, uint len
27482748
if (!cf_handle)
27492749
goto error;
27502750

2751-
index_id= ddl_manager.get_next_number();
2751+
index_id= ddl_manager.get_and_update_next_number(&dict_manager);
27522752
if (!(key_descr[i]= new RDBSE_KEYDEF(index_id, i,
27532753
cf_handle,
27542754
is_cf_name_reverse(comment),
@@ -2762,7 +2762,6 @@ int ha_rocksdb::create_key_defs(TABLE *table_arg, const char *db_table, uint len
27622762
tbl_def->dbname_tablename.append(db_table, len);
27632763
dict_manager.lock();
27642764
write_err= ddl_manager.put_and_write(tbl_def, batch)
2765-
|| dict_manager.update_max_index_id(batch, index_id)
27662765
|| dict_manager.commit(batch);
27672766
dict_manager.unlock();
27682767

storage/rocksdb/rdb_datadic.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,3 +2199,16 @@ Dict_manager::get_stats(
21992199

22002200
return MyRocksTablePropertiesCollector::IndexStats();
22012201
}
2202+
2203+
uint Sequence_generator::get_and_update_next_number(Dict_manager *dict)
2204+
{
2205+
uint res;
2206+
mysql_mutex_lock(&mutex);
2207+
res= next_number++;
2208+
std::unique_ptr<rocksdb::WriteBatch> wb= dict->begin();
2209+
rocksdb::WriteBatch *batch= wb.get();
2210+
dict->update_max_index_id(batch, res);
2211+
dict->commit(batch);
2212+
mysql_mutex_unlock(&mutex);
2213+
return res;
2214+
}

storage/rocksdb/rdb_datadic.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -552,14 +552,7 @@ class Sequence_generator
552552
next_number= initial_number;
553553
}
554554

555-
uint get_next_number()
556-
{
557-
uint res;
558-
mysql_mutex_lock(&mutex);
559-
res= next_number++;
560-
mysql_mutex_unlock(&mutex);
561-
return res;
562-
}
555+
uint get_and_update_next_number(Dict_manager *dict);
563556

564557
void cleanup()
565558
{
@@ -609,7 +602,8 @@ class Table_ddl_manager
609602
bool rename(uchar *from, uint from_len, uchar *to, uint to_len,
610603
rocksdb::WriteBatch *batch);
611604

612-
uint get_next_number() { return sequence.get_next_number(); }
605+
uint get_and_update_next_number(Dict_manager *dict)
606+
{ return sequence.get_and_update_next_number(dict); }
613607
void add_changed_indexes(const std::vector<uint32_t>& changed_indexes);
614608
std::unordered_set<uint32_t> get_changed_indexes();
615609

0 commit comments

Comments
 (0)