revision-id: 36f310f32dff0d34cb21a66afbcaec955d0642bc (fb-prod201801-165-g36f310f32df) parent(s): a953b2edbca118a1689497882f4cfe21f5e1f207 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2018-11-19 23:09:44 +0300 message: Make rocksdb_kill_connection() thread-safe This was observable as a failure of rocksdb.ddl_high_priority test --- storage/rocksdb/ha_rocksdb.cc | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 72f422ce1ce..da75885cd36 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -2032,6 +2032,9 @@ protected: bool m_is_delayed_snapshot = false; bool m_is_two_phase = false; + // Last known TransactionID of the underlying transaction, or UINT_MAX + // for write batches. See kill_lock_wait_by_thd. + uint64_t m_last_trx_id; private: /* Number of write operations this transaction had when we took the last @@ -2690,6 +2693,8 @@ public: s_tx_list.erase(this); RDB_MUTEX_UNLOCK_CHECK(s_tx_list_mutex); } + + static void kill_lock_wait_by_thd(THD *thd); }; /* @@ -2975,6 +2980,8 @@ public: rdb->BeginTransaction(write_opts, tx_opts, m_rocksdb_reuse_tx); m_rocksdb_reuse_tx = nullptr; + m_last_trx_id = m_rocksdb_tx->GetID(); + m_read_opts = rocksdb::ReadOptions(); set_initial_savepoint(); @@ -3246,6 +3253,7 @@ public: : Rdb_transaction(thd), m_batch(nullptr) { m_batch = new rocksdb::WriteBatchWithIndex(rocksdb::BytewiseComparator(), 0, true); + m_last_trx_id= UINT64_MAX; } virtual ~Rdb_writebatch_impl() { @@ -4319,24 +4327,32 @@ static int rocksdb_start_tx_and_assign_read_view( } -static void -rocksdb_kill_connection(handlerton* hton, - THD* thd) +void rocksdb_kill_connection(handlerton* hton, THD* thd) { - //TODO: db_env->kill_waiter(db_env, thd); - // locktree_manager::kill_waiter(void *extra) + Rdb_transaction::kill_lock_wait_by_thd(thd); +} + + +void Rdb_transaction::kill_lock_wait_by_thd(THD *thd) +{ + /* + Piggy-back on s_tx_list_mutex to avoid the situtation where the transaction + that we are trying to kill finishes after we got it from get_tx_from_thd() + but before we get its m_last_trx_id. + + The lock wait may be gone, and tx->m_rocksdb_tx may change while we are + running the code lines below. This is why we use m_last_trx_id. We may + end up with an identifier of a transaction that is already gone, but this + won't be a problem. + */ + RDB_MUTEX_LOCK_CHECK(Rdb_transaction::s_tx_list_mutex); Rdb_transaction *const tx = get_tx_from_thd(thd); - // tx can be zero if that thread doesn't have a RocksDB transaction - // TODO: how do we know if things that are done in this function are - // thread-safe? e.g. the transaction doesn't disappear while execution - // is right on this line? - if (tx && !tx->is_writebatch_trx()) + if (tx && tx->m_last_trx_id != UINT64_MAX) { - Rdb_transaction_impl *tx_impl= (Rdb_transaction_impl*) tx; - const rocksdb::Transaction *rdb_trx = tx_impl->get_rdb_trx(); - rdb->KillLockWait((void*)rdb_trx->GetID()); + rdb->KillLockWait((void*)tx->m_last_trx_id); } + RDB_MUTEX_UNLOCK_CHECK(Rdb_transaction::s_tx_list_mutex); }