From 111555789fd62a8ddf69050ce196a7cb7d18463f Mon Sep 17 00:00:00 2001 From: Eugene Kosov Date: Sun, 21 Jan 2018 21:40:47 +0300 Subject: [PATCH] simplify and make counter atomic additionally fix data race which looks like this: WARNING: ThreadSanitizer: data race (pid=30515) Write of size 8 at 0x0000039ee908 by thread T21: #0 ib_counter_t::add(unsigned long, long) storage/innobase/include/ut0counter.h:132:16 (mysqld+0x21cd166) #1 ib_counter_t::add(long) storage/innobase/include/ut0counter.h:122:34 (mysqld+0x21cc102) #2 rw_lock_x_lock_wait_func(rw_lock_t*, unsigned long, long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:489:38 (mysqld+0x21cb91f) #3 rw_lock_x_lock_low(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:538:3 (mysqld+0x21c9339) #4 rw_lock_x_lock_func(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:698:6 (mysqld+0x21c8c4d) #5 pfs_rw_lock_x_lock_func(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/include/sync0rw.ic:568:3 (mysqld+0x1afbdb4) #6 buf_page_get_gen(page_id_t const&, page_size_t const&, unsigned long, buf_block_t*, unsigned long, char const*, unsigned int, mtr_t*, dberr_t*) storage/innobase/buf/buf0buf.cc:4782:3 (mysqld+0x1b04e28) #7 btr_cur_search_to_nth_level_func(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_cur_t*, rw_lock_t*, char const*, unsigned int, mtr_t*, unsigned long) storage/innobase/btr/btr0cur.cc:1312:10 (mysqld+0x1bf362c) #8 btr_pcur_open_low(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_pcur_t*, char const*, unsigned int, unsigned long, mtr_t*) storage/innobase/include/btr0pcur.ic:457:8 (mysqld+0x20eecd0) #9 row_search_on_row_ref(btr_pcur_t*, unsigned long, dict_table_t const*, dtuple_t const*, mtr_t*) storage/innobase/row/row0row.cc:1030:3 (mysqld+0x20ee1b4) #10 row_purge_reposition_pcur(unsigned long, purge_node_t*, mtr_t*) storage/innobase/row/row0purge.cc:103:23 (mysqld+0x20d7f6f) #11 row_purge_reset_trx_id(purge_node_t*, mtr_t*) storage/innobase/row/row0purge.cc:678:6 (mysqld+0x20dcbcd) #12 row_purge_record_func(purge_node_t*, unsigned char*, que_thr_t const*, bool) storage/innobase/row/row0purge.cc:1062:4 (mysqld+0x20db46f) #13 row_purge(purge_node_t*, unsigned char*, que_thr_t*) storage/innobase/row/row0purge.cc:1111:18 (mysqld+0x20d8aa4) #14 row_purge_step(que_thr_t*) storage/innobase/row/row0purge.cc:1190:3 (mysqld+0x20d872c) #15 que_thr_step(que_thr_t*) storage/innobase/que/que0que.cc:1055:9 (mysqld+0x1fcfa6f) #16 que_run_threads_low(que_thr_t*) storage/innobase/que/que0que.cc:1117:14 (mysqld+0x1fcdd6e) #17 que_run_threads(que_thr_t*) storage/innobase/que/que0que.cc:1157:2 (mysqld+0x1fcd908) #18 srv_task_execute() storage/innobase/srv/srv0srv.cc:2520:3 (mysqld+0x21a6684) #19 srv_worker_thread storage/innobase/srv/srv0srv.cc:2567:7 (mysqld+0x21a6247) Previous write of size 8 at 0x0000039ee908 by thread T20: #0 ib_counter_t::add(unsigned long, long) storage/innobase/include/ut0counter.h:132:16 (mysqld+0x21cd166) #1 ib_counter_t::add(long) storage/innobase/include/ut0counter.h:122:34 (mysqld+0x21cc102) #2 rw_lock_x_lock_wait_func(rw_lock_t*, unsigned long, long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:489:38 (mysqld+0x21cb91f) #3 rw_lock_x_lock_low(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:538:3 (mysqld+0x21c9339) #4 rw_lock_x_lock_func(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/sync/sync0rw.cc:698:6 (mysqld+0x21c8c4d) #5 pfs_rw_lock_x_lock_func(rw_lock_t*, unsigned long, char const*, unsigned int) storage/innobase/include/sync0rw.ic:568:3 (mysqld+0x1afbdb4) #6 buf_page_get_gen(page_id_t const&, page_size_t const&, unsigned long, buf_block_t*, unsigned long, char const*, unsigned int, mtr_t*, dberr_t*) storage/innobase/buf/buf0buf.cc:4782:3 (mysqld+0x1b04e28) #7 btr_cur_search_to_nth_level_func(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_cur_t*, rw_lock_t*, char const*, unsigned int, mtr_t*, unsigned long) storage/innobase/btr/btr0cur.cc:1312:10 (mysqld+0x1bf362c) #8 btr_pcur_open_low(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_pcur_t*, char const*, unsigned int, unsigned long, mtr_t*) storage/innobase/include/btr0pcur.ic:457:8 (mysqld+0x20eecd0) #9 row_search_on_row_ref(btr_pcur_t*, unsigned long, dict_table_t const*, dtuple_t const*, mtr_t*) storage/innobase/row/row0row.cc:1030:3 (mysqld+0x20ee1b4) #10 row_purge_reposition_pcur(unsigned long, purge_node_t*, mtr_t*) storage/innobase/row/row0purge.cc:103:23 (mysqld+0x20d7f6f) #11 row_purge_reset_trx_id(purge_node_t*, mtr_t*) storage/innobase/row/row0purge.cc:678:6 (mysqld+0x20dcbcd) #12 row_purge_record_func(purge_node_t*, unsigned char*, que_thr_t const*, bool) storage/innobase/row/row0purge.cc:1062:4 (mysqld+0x20db46f) #13 row_purge(purge_node_t*, unsigned char*, que_thr_t*) storage/innobase/row/row0purge.cc:1111:18 (mysqld+0x20d8aa4) #14 row_purge_step(que_thr_t*) storage/innobase/row/row0purge.cc:1190:3 (mysqld+0x20d872c) #15 que_thr_step(que_thr_t*) storage/innobase/que/que0que.cc:1055:9 (mysqld+0x1fcfa6f) #16 que_run_threads_low(que_thr_t*) storage/innobase/que/que0que.cc:1117:14 (mysqld+0x1fcdd6e) #17 que_run_threads(que_thr_t*) storage/innobase/que/que0que.cc:1157:2 (mysqld+0x1fcd908) #18 srv_task_execute() storage/innobase/srv/srv0srv.cc:2520:3 (mysqld+0x21a6684) #19 srv_worker_thread storage/innobase/srv/srv0srv.cc:2567:7 (mysqld+0x21a6247) --- storage/innobase/handler/ha_innodb.cc | 12 ++---- storage/innobase/include/srv0srv.h | 2 +- storage/innobase/include/sync0rw.h | 2 +- storage/innobase/include/ut0counter.h | 57 ++++----------------------- storage/innobase/row/row0mysql.cc | 16 ++++---- storage/innobase/row/row0sel.cc | 3 +- 6 files changed, 22 insertions(+), 70 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 42062e452ee4b..4690aa33ea249 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -9697,11 +9697,9 @@ ha_innobase::index_read( error = 0; table->status = 0; if (m_prebuilt->table->is_system_db) { - srv_stats.n_system_rows_read.add( - thd_get_thread_id(m_prebuilt->trx->mysql_thd), 1); + srv_stats.n_system_rows_read.inc(); } else { - srv_stats.n_rows_read.add( - thd_get_thread_id(m_prebuilt->trx->mysql_thd), 1); + srv_stats.n_rows_read.inc(); } break; @@ -10020,11 +10018,9 @@ ha_innobase::general_fetch( error = 0; table->status = 0; if (m_prebuilt->table->is_system_db) { - srv_stats.n_system_rows_read.add( - thd_get_thread_id(trx->mysql_thd), 1); + srv_stats.n_system_rows_read.inc(); } else { - srv_stats.n_rows_read.add( - thd_get_thread_id(trx->mysql_thd), 1); + srv_stats.n_rows_read.inc(); } break; case DB_RECORD_NOT_FOUND: diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h index 9f37b78ac5dda..2f55d68cc8ee7 100644 --- a/storage/innobase/include/srv0srv.h +++ b/storage/innobase/include/srv0srv.h @@ -60,7 +60,7 @@ Created 10/10/1995 Heikki Tuuri /** Global counters used inside InnoDB. */ struct srv_stats_t { - typedef ib_counter_t ulint_ctr_64_t; + typedef ib_counter_t ulint_ctr_64_t; typedef simple_counter lsn_ctr_1_t; typedef simple_counter ulint_ctr_1_t; typedef simple_counter int64_ctr_1_t; diff --git a/storage/innobase/include/sync0rw.h b/storage/innobase/include/sync0rw.h index c9bc443fc55a7..ea83a97bae4f9 100644 --- a/storage/innobase/include/sync0rw.h +++ b/storage/innobase/include/sync0rw.h @@ -41,7 +41,7 @@ Created 9/11/1995 Heikki Tuuri /** Counters for RW locks. */ struct rw_lock_stats_t { - typedef ib_counter_t int64_counter_t; + typedef ib_counter_t int64_counter_t; /** number of spin waits on rw-latches, resulted during shared (read) locks */ diff --git a/storage/innobase/include/ut0counter.h b/storage/innobase/include/ut0counter.h index f1a9384667e0c..3cd5c31ea353a 100644 --- a/storage/innobase/include/ut0counter.h +++ b/storage/innobase/include/ut0counter.h @@ -86,69 +86,26 @@ struct counter_indexer_t : public generic_indexer_t { #define default_indexer_t counter_indexer_t -/** Class for using fuzzy counters. The counter is not protected by any -mutex and the results are not guaranteed to be 100% accurate but close -enough. Creates an array of counters and separates each element by the -CACHE_LINE_SIZE bytes */ -template < - typename Type, - int N = IB_N_SLOTS, - template class Indexer = default_indexer_t> +/** Atomic counter */ struct MY_ALIGNED(CACHE_LINE_SIZE) ib_counter_t { -#ifdef UNIV_DEBUG - ~ib_counter_t() - { - size_t n = (CACHE_LINE_SIZE / sizeof(Type)); - - /* Check that we aren't writing outside our defined bounds. */ - for (size_t i = 0; i < UT_ARR_SIZE(m_counter); i += n) { - for (size_t j = 1; j < n - 1; ++j) { - ut_ad(m_counter[i + j] == 0); - } - } - } -#endif /* UNIV_DEBUG */ + ib_counter_t() : m_counter(0) {} /** Increment the counter by 1. */ void inc() UNIV_NOTHROW { add(1); } - /** Increment the counter by 1. - @param[in] index a reasonably thread-unique identifier */ - void inc(size_t index) UNIV_NOTHROW { add(index, 1); } - - /** Add to the counter. - @param[in] n amount to be added */ - void add(Type n) UNIV_NOTHROW { add(m_policy.get_rnd_offset(), n); } - /** Add to the counter. - @param[in] index a reasonably thread-unique identifier @param[in] n amount to be added */ - void add(size_t index, Type n) UNIV_NOTHROW { - size_t i = m_policy.offset(index); - - ut_ad(i < UT_ARR_SIZE(m_counter)); - - m_counter[i] += n; - } + void add(uint64_t n) UNIV_NOTHROW { my_atomic_add64(&m_counter, n); } /* @return total value - not 100% accurate, since it is not atomic. */ - operator Type() const UNIV_NOTHROW { - Type total = 0; - - for (size_t i = 0; i < N; ++i) { - total += m_counter[m_policy.offset(i)]; - } - - return(total); + operator uint64_t() const UNIV_NOTHROW { + return(my_atomic_load64_explicit(&m_counter, + MY_MEMORY_ORDER_RELAXED)); } private: - /** Indexer into the array */ - Indexerm_policy; - - /** Slot 0 is unused. */ - Type m_counter[(N + 1) * (CACHE_LINE_SIZE / sizeof(Type))]; + uint64_t m_counter; }; #endif /* ut0counter_h */ diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index 5bf0a3dcfc679..66bf98b40964c 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -1607,9 +1607,9 @@ row_insert_for_mysql( que_thr_stop_for_mysql_no_error(thr, trx); if (table->is_system_db) { - srv_stats.n_system_rows_inserted.inc(size_t(trx->id)); + srv_stats.n_system_rows_inserted.inc(); } else { - srv_stats.n_rows_inserted.inc(size_t(trx->id)); + srv_stats.n_rows_inserted.inc(); } /* Not protected by dict_table_stats_lock() for performance @@ -2146,11 +2146,11 @@ row_update_for_mysql(row_prebuilt_t* prebuilt) dict_table_n_rows_dec(node->table); update_statistics = !srv_stats_include_delete_marked; - srv_stats.n_rows_deleted.inc(size_t(trx->id)); + srv_stats.n_rows_deleted.inc(); } else { update_statistics = !(node->cmpl_info & UPD_NODE_NO_ORD_CHANGE); - srv_stats.n_rows_updated.inc(size_t(trx->id)); + srv_stats.n_rows_updated.inc(); } if (update_statistics) { @@ -2171,17 +2171,17 @@ row_update_for_mysql(row_prebuilt_t* prebuilt) dict_table_n_rows_dec(prebuilt->table); if (table->is_system_db) { - srv_stats.n_system_rows_deleted.inc(size_t(trx->id)); + srv_stats.n_system_rows_deleted.inc(); } else { - srv_stats.n_rows_deleted.inc(size_t(trx->id)); + srv_stats.n_rows_deleted.inc(); } update_statistics = !srv_stats_include_delete_marked; } else { if (table->is_system_db) { - srv_stats.n_system_rows_updated.inc(size_t(trx->id)); + srv_stats.n_system_rows_updated.inc(); } else { - srv_stats.n_rows_updated.inc(size_t(trx->id)); + srv_stats.n_rows_updated.inc(); } update_statistics diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 6f4faad93ff1e..dfa483dce72bf 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -3283,8 +3283,7 @@ row_sel_get_clust_rec_for_mysql( *out_rec = NULL; trx = thr_get_trx(thr); - srv_stats.n_sec_rec_cluster_reads.inc( - thd_get_thread_id(trx->mysql_thd)); + srv_stats.n_sec_rec_cluster_reads.inc(); row_build_row_ref_in_tuple(prebuilt->clust_ref, rec, sec_index, *offsets, trx);