diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2019-09-03 13:04:05 +0300 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2019-09-04 09:42:38 +0300 |
commit | dae1b3b04c839b25233700c7ff7def578c545508 (patch) | |
tree | c5c538f92e6835d9879226387a5cf0fed08ece08 /storage/innobase/include | |
parent | b07beff8943d083cf900c8f2ea0a386a84d9f8db (diff) | |
download | mariadb-git-dae1b3b04c839b25233700c7ff7def578c545508.tar.gz |
MDEV-15326: Backport trx_t::is_referenced()
Backport the applicable part of Sergey Vojtovich's commit
0ca2ea1a6550bb718efc878ed451be69e8244cd4 from MariaDB Server 10.3.
trx reference counter was updated under mutex and read without any
protection. This is both slow and unsafe. Use atomic operations for
reference counter accesses.
Diffstat (limited to 'storage/innobase/include')
-rw-r--r-- | storage/innobase/include/row0vers.h | 2 | ||||
-rw-r--r-- | storage/innobase/include/trx0sys.ic | 8 | ||||
-rw-r--r-- | storage/innobase/include/trx0trx.h | 62 | ||||
-rw-r--r-- | storage/innobase/include/trx0trx.ic | 17 |
4 files changed, 48 insertions, 41 deletions
diff --git a/storage/innobase/include/row0vers.h b/storage/innobase/include/row0vers.h index b5279e5851b..327bb546356 100644 --- a/storage/innobase/include/row0vers.h +++ b/storage/innobase/include/row0vers.h @@ -44,7 +44,7 @@ index record. @param[in] index secondary index @param[in] offsets rec_get_offsets(rec, index) @return the active transaction; state must be rechecked after -trx_mutex_enter(), and trx_release_reference() must be invoked +trx_mutex_enter(), and trx->release_reference() must be invoked @retval NULL if the record was committed */ trx_t* row_vers_impl_x_locked( diff --git a/storage/innobase/include/trx0sys.ic b/storage/innobase/include/trx0sys.ic index ac84404a397..0a4d583671f 100644 --- a/storage/innobase/include/trx0sys.ic +++ b/storage/innobase/include/trx0sys.ic @@ -346,7 +346,6 @@ inline trx_t* trx_rw_is_active(trx_id_t trx_id, bool* corrupt, bool ref_count) mutex_enter(trx_mutex); ut_ad(!trx_state_eq(trx, TRX_STATE_NOT_STARTED)); ut_ad(trx->id == trx_id); - ut_ad(trx->n_ref >= 0); if (trx_state_eq(trx, TRX_STATE_COMMITTED_IN_MEMORY)) { /* We have an early state check here to avoid committer starvation in a wait loop for @@ -357,7 +356,12 @@ inline trx_t* trx_rw_is_active(trx_id_t trx_id, bool* corrupt, bool ref_count) rechecked by the caller after reacquiring the mutex. */ trx = NULL; } else { - ++trx->n_ref; + /* The reference could be safely incremented after + releasing one of trx_mutex or trx_sys->mutex. + Holding trx->mutex here may prevent a few false + references that could have a negative performance + impact on trx_commit_in_memory(). */ + trx->reference(); } mutex_exit(trx_mutex); } diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 796e6b8bda6..1d71920d6c5 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -501,18 +501,6 @@ trx_set_rw_mode( trx_t* trx); /** -Release the transaction. Decrease the reference count. -@param trx Transaction that is being released */ -UNIV_INLINE -void -trx_release_reference( - trx_t* trx); - -/** -Check if the transaction is being referenced. */ -#define trx_is_referenced(t) ((t)->n_ref > 0) - -/** Transactions that aren't started by the MySQL server don't set the trx_t::mysql_thd field. For such transactions we set the lock wait timeout to 0 instead of the user configured value that comes @@ -571,7 +559,7 @@ Check transaction state */ ut_ad(trx_state_eq((t), TRX_STATE_NOT_STARTED)); \ ut_ad(!(t)->id); \ ut_ad(!(t)->has_logged()); \ - ut_ad(!(t)->n_ref); \ + ut_ad(!(t)->is_referenced()); \ ut_ad(!MVCC::is_view_active((t)->read_view)); \ ut_ad((t)->lock.wait_thr == NULL); \ ut_ad(UT_LIST_GET_LEN((t)->lock.trx_locks) == 0); \ @@ -801,6 +789,19 @@ struct trx_rsegs_t { }; struct trx_t { +private: + /** + Count of references. + + We can't release the locks nor commit the transaction until this reference + is 0. We can change the state to TRX_STATE_COMMITTED_IN_MEMORY to signify + that it is no longer "active". + */ + + int32_t n_ref; + + +public: TrxMutex mutex; /*!< Mutex protecting the fields state and lock (except some fields of lock, which are protected by @@ -1100,14 +1101,6 @@ struct trx_t { const char* start_file; /*!< Filename where it was started */ #endif /* UNIV_DEBUG */ - lint n_ref; /*!< Count of references, protected - by trx_t::mutex. We can't release the - locks nor commit the transaction until - this reference is 0. We can change - the state to COMMITTED_IN_MEMORY to - signify that it is no longer - "active". */ - XID* xid; /*!< X/Open XA transaction identification to identify a transaction branch */ @@ -1184,6 +1177,33 @@ public: /** Release any explicit locks of a committing transaction. */ inline void release_locks(); + + bool is_referenced() + { + return my_atomic_load32_explicit(&n_ref, MY_MEMORY_ORDER_RELAXED) > 0; + } + + + void reference() + { +#ifdef UNIV_DEBUG + int32_t old_n_ref= +#endif + my_atomic_add32_explicit(&n_ref, 1, MY_MEMORY_ORDER_RELAXED); + ut_ad(old_n_ref >= 0); + } + + + void release_reference() + { +#ifdef UNIV_DEBUG + int32_t old_n_ref= +#endif + my_atomic_add32_explicit(&n_ref, -1, MY_MEMORY_ORDER_RELAXED); + ut_ad(old_n_ref > 0); + } + + private: /** Assign a rollback segment for modifying temporary tables. @return the assigned rollback segment */ diff --git a/storage/innobase/include/trx0trx.ic b/storage/innobase/include/trx0trx.ic index 64681e00a62..4a5b1ba717f 100644 --- a/storage/innobase/include/trx0trx.ic +++ b/storage/innobase/include/trx0trx.ic @@ -211,23 +211,6 @@ ok: } /** -Release the transaction. Decrease the reference count. -@param trx Transaction that is being released */ -UNIV_INLINE -void -trx_release_reference( - trx_t* trx) -{ - trx_mutex_enter(trx); - - ut_ad(trx->n_ref > 0); - --trx->n_ref; - - trx_mutex_exit(trx); -} - - -/** @param trx Get the active view for this transaction, if one exists @return the transaction's read view or NULL if one not assigned. */ UNIV_INLINE |