Percona Server moved to https://jira.percona.com/projects/PS

Merge lp:~akopytov/percona-server/bug1123921 into lp:percona-server/5.5

  1. bug1123921
  2. Merge into 5.5
Proposed by Alexey Kopytov
Status: Merged
Approved by: Laurynas Biveinis
Approved revision: no longer in the source branch.
Merged at revision: 459
Proposed branch: lp:~akopytov/percona-server/bug1123921
Merge into: lp:percona-server/5.5
Diff against target: 217 lines (+26/-24)
5 files modified
Percona-Server/storage/innobase/buf/buf0buf.c (+16/-14)
Percona-Server/storage/innobase/lock/lock0lock.c (+2/-2)
Percona-Server/storage/innobase/os/os0file.c (+3/-3)
Percona-Server/storage/innobase/srv/srv0srv.c (+2/-2)
Percona-Server/storage/innobase/trx/trx0trx.c (+3/-3)
To merge this branch: bzr merge lp:~akopytov/percona-server/bug1123921
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Approve
Stewart Smith (community) Needs Fixing
Sergei Glushchenko (community) g2 Approve
Review via email: mp+149830@code.launchpad.net

Description of the change

  Bug #1123921: Suspicious logic in XtraDB extra slow log stats collection

  Fixed unnecessary calls to innobase_get_slow_log() and added compiler
  branch prediction hints.

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Approve

review: Approve (g2)
Revision history for this message
Stewart Smith (stewart) wrote :

http://jenkins.percona.com/view/Merge/job/merge-PS-5.5-staging/52/

Many test failures

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote :

Fixed. The problem was that not all code branches were checking the (trx != NULL && trx->take_stats) invariant before calling _increment_page_get_statistics().

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/695/

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Percona-Server/storage/innobase/buf/buf0buf.c'
--- Percona-Server/storage/innobase/buf/buf0buf.c 2013-02-07 04:38:31 +0000
+++ Percona-Server/storage/innobase/buf/buf0buf.c 2013-02-26 09:04:21 +0000
@@ -66,9 +66,7 @@
66 byte block_hash_offset;66 byte block_hash_offset;
6767
68 ut_ad(block);68 ut_ad(block);
6969 ut_ad(trx && trx->take_stats);
70 if (!innobase_get_slow_log() || !trx || !trx->take_stats)
71 return;
7270
73 if (!trx->distinct_page_access_hash) {71 if (!trx->distinct_page_access_hash) {
74 trx->distinct_page_access_hash = mem_alloc(DPAH_SIZE);72 trx->distinct_page_access_hash = mem_alloc(DPAH_SIZE);
@@ -1967,7 +1965,7 @@
1967 ib_uint64_t finish_time;1965 ib_uint64_t finish_time;
1968 buf_pool_t* buf_pool = buf_pool_get(space, offset);1966 buf_pool_t* buf_pool = buf_pool_get(space, offset);
19691967
1970 if (innobase_get_slow_log()) {1968 if (UNIV_UNLIKELY(innobase_get_slow_log())) {
1971 trx = innobase_get_trx();1969 trx = innobase_get_trx();
1972 }1970 }
1973 buf_pool->stat.n_page_gets++;1971 buf_pool->stat.n_page_gets++;
@@ -2111,7 +2109,7 @@
2111 /* Let us wait until the read operation2109 /* Let us wait until the read operation
2112 completes */2110 completes */
21132111
2114 if (innobase_get_slow_log() && trx && trx->take_stats)2112 if (UNIV_UNLIKELY(trx && trx->take_stats))
2115 {2113 {
2116 ut_usectime(&sec, &ms);2114 ut_usectime(&sec, &ms);
2117 start_time = (ib_uint64_t)sec * 1000000 + ms;2115 start_time = (ib_uint64_t)sec * 1000000 + ms;
@@ -2132,7 +2130,7 @@
2132 break;2130 break;
2133 }2131 }
2134 }2132 }
2135 if (innobase_get_slow_log() && trx && trx->take_stats && start_time)2133 if (UNIV_UNLIKELY(start_time != 0))
2136 {2134 {
2137 ut_usectime(&sec, &ms);2135 ut_usectime(&sec, &ms);
2138 finish_time = (ib_uint64_t)sec * 1000000 + ms;2136 finish_time = (ib_uint64_t)sec * 1000000 + ms;
@@ -2487,7 +2485,7 @@
2487 || ibuf_page_low(space, zip_size, offset,2485 || ibuf_page_low(space, zip_size, offset,
2488 FALSE, file, line, NULL));2486 FALSE, file, line, NULL));
2489#endif2487#endif
2490 if (innobase_get_slow_log()) {2488 if (UNIV_UNLIKELY(innobase_get_slow_log())) {
2491 trx = innobase_get_trx();2489 trx = innobase_get_trx();
2492 }2490 }
2493 buf_pool->stat.n_page_gets++;2491 buf_pool->stat.n_page_gets++;
@@ -2913,7 +2911,7 @@
2913 /* Let us wait until the read operation2911 /* Let us wait until the read operation
2914 completes */2912 completes */
29152913
2916 if (innobase_get_slow_log() && trx && trx->take_stats)2914 if (UNIV_UNLIKELY(trx && trx->take_stats))
2917 {2915 {
2918 ut_usectime(&sec, &ms);2916 ut_usectime(&sec, &ms);
2919 start_time = (ib_uint64_t)sec * 1000000 + ms;2917 start_time = (ib_uint64_t)sec * 1000000 + ms;
@@ -2935,7 +2933,7 @@
2935 break;2933 break;
2936 }2934 }
2937 }2935 }
2938 if (innobase_get_slow_log() && trx && trx->take_stats && start_time)2936 if (UNIV_UNLIKELY(start_time != 0))
2939 {2937 {
2940 ut_usectime(&sec, &ms);2938 ut_usectime(&sec, &ms);
2941 finish_time = (ib_uint64_t)sec * 1000000 + ms;2939 finish_time = (ib_uint64_t)sec * 1000000 + ms;
@@ -2974,7 +2972,7 @@
2974 ut_a(ibuf_count_get(buf_block_get_space(block),2972 ut_a(ibuf_count_get(buf_block_get_space(block),
2975 buf_block_get_page_no(block)) == 0);2973 buf_block_get_page_no(block)) == 0);
2976#endif2974#endif
2977 if (innobase_get_slow_log()) {2975 if (UNIV_UNLIKELY(trx && trx->take_stats)) {
2978 _increment_page_get_statistics(block, trx);2976 _increment_page_get_statistics(block, trx);
2979 }2977 }
29802978
@@ -3079,7 +3077,7 @@
3079#if defined UNIV_DEBUG_FILE_ACCESSES || defined UNIV_DEBUG3077#if defined UNIV_DEBUG_FILE_ACCESSES || defined UNIV_DEBUG
3080 ut_a(block->page.file_page_was_freed == FALSE);3078 ut_a(block->page.file_page_was_freed == FALSE);
3081#endif3079#endif
3082 if (innobase_get_slow_log()) {3080 if (UNIV_UNLIKELY(innobase_get_slow_log())) {
3083 trx = innobase_get_trx();3081 trx = innobase_get_trx();
3084 }3082 }
30853083
@@ -3100,7 +3098,7 @@
3100 buf_pool = buf_pool_from_block(block);3098 buf_pool = buf_pool_from_block(block);
3101 buf_pool->stat.n_page_gets++;3099 buf_pool->stat.n_page_gets++;
31023100
3103 if (innobase_get_slow_log()) {3101 if (UNIV_UNLIKELY(trx && trx->take_stats)) {
3104 _increment_page_get_statistics(block, trx);3102 _increment_page_get_statistics(block, trx);
3105 }3103 }
3106 return(TRUE);3104 return(TRUE);
@@ -3212,9 +3210,13 @@
3212#endif3210#endif
3213 buf_pool->stat.n_page_gets++;3211 buf_pool->stat.n_page_gets++;
32143212
3215 if (innobase_get_slow_log()) {3213 if (UNIV_UNLIKELY(innobase_get_slow_log())) {
3214
3216 trx = innobase_get_trx();3215 trx = innobase_get_trx();
3217 _increment_page_get_statistics(block, trx);3216 if (trx != NULL && trx->take_stats) {
3217
3218 _increment_page_get_statistics(block, trx);
3219 }
3218 }3220 }
32193221
3220 return(TRUE);3222 return(TRUE);
32213223
=== modified file 'Percona-Server/storage/innobase/lock/lock0lock.c'
--- Percona-Server/storage/innobase/lock/lock0lock.c 2013-01-18 12:02:28 +0000
+++ Percona-Server/storage/innobase/lock/lock0lock.c 2013-02-26 09:04:21 +0000
@@ -1829,7 +1829,7 @@
1829 trx->que_state = TRX_QUE_LOCK_WAIT;1829 trx->que_state = TRX_QUE_LOCK_WAIT;
1830 trx->was_chosen_as_deadlock_victim = FALSE;1830 trx->was_chosen_as_deadlock_victim = FALSE;
1831 trx->wait_started = time(NULL);1831 trx->wait_started = time(NULL);
1832 if (innobase_get_slow_log() && trx->take_stats) {1832 if (UNIV_UNLIKELY(trx->take_stats)) {
1833 ut_usectime(&sec, &ms);1833 ut_usectime(&sec, &ms);
1834 trx->lock_que_wait_ustarted = (ib_uint64_t)sec * 1000000 + ms;1834 trx->lock_que_wait_ustarted = (ib_uint64_t)sec * 1000000 + ms;
1835 }1835 }
@@ -3837,7 +3837,7 @@
3837 return(DB_SUCCESS);3837 return(DB_SUCCESS);
3838 }3838 }
38393839
3840 if (innobase_get_slow_log() && trx->take_stats) {3840 if (UNIV_UNLIKELY(trx->take_stats)) {
3841 ut_usectime(&sec, &ms);3841 ut_usectime(&sec, &ms);
3842 trx->lock_que_wait_ustarted = (ib_uint64_t)sec * 1000000 + ms;3842 trx->lock_que_wait_ustarted = (ib_uint64_t)sec * 1000000 + ms;
3843 }3843 }
38443844
=== modified file 'Percona-Server/storage/innobase/os/os0file.c'
--- Percona-Server/storage/innobase/os/os0file.c 2013-02-06 09:26:12 +0000
+++ Percona-Server/storage/innobase/os/os0file.c 2013-02-26 09:04:21 +0000
@@ -2289,7 +2289,7 @@
22892289
2290 os_n_file_reads++;2290 os_n_file_reads++;
22912291
2292 if (innobase_get_slow_log() && trx && trx->take_stats)2292 if (UNIV_UNLIKELY(trx && trx->take_stats))
2293 {2293 {
2294 trx->io_reads++;2294 trx->io_reads++;
2295 trx->io_read += n;2295 trx->io_read += n;
@@ -2322,7 +2322,7 @@
2322 os_n_pending_reads--;2322 os_n_pending_reads--;
2323 os_mutex_exit(os_file_count_mutex);2323 os_mutex_exit(os_file_count_mutex);
23242324
2325 if (innobase_get_slow_log() && trx && trx->take_stats && start_time)2325 if (UNIV_UNLIKELY(start_time != 0))
2326 {2326 {
2327 ut_usectime(&sec, &ms);2327 ut_usectime(&sec, &ms);
2328 finish_time = (ib_uint64_t)sec * 1000000 + ms;2328 finish_time = (ib_uint64_t)sec * 1000000 + ms;
@@ -2376,7 +2376,7 @@
2376 os_n_pending_reads--;2376 os_n_pending_reads--;
2377 os_mutex_exit(os_file_count_mutex);2377 os_mutex_exit(os_file_count_mutex);
23782378
2379 if (innobase_get_slow_log() && trx && trx->take_stats && start_time)2379 if (UNIV_UNLIKELY(start_time != 0)
2380 {2380 {
2381 ut_usectime(&sec, &ms);2381 ut_usectime(&sec, &ms);
2382 finish_time = (ib_uint64_t)sec * 1000000 + ms;2382 finish_time = (ib_uint64_t)sec * 1000000 + ms;
23832383
=== modified file 'Percona-Server/storage/innobase/srv/srv0srv.c'
--- Percona-Server/storage/innobase/srv/srv0srv.c 2013-02-20 06:41:32 +0000
+++ Percona-Server/storage/innobase/srv/srv0srv.c 2013-02-26 09:04:21 +0000
@@ -1448,7 +1448,7 @@
1448 ut_ad(!sync_thread_levels_nonempty_trx(trx->has_search_latch));1448 ut_ad(!sync_thread_levels_nonempty_trx(trx->has_search_latch));
1449#endif /* UNIV_SYNC_DEBUG */1449#endif /* UNIV_SYNC_DEBUG */
14501450
1451 if (innobase_get_slow_log() && trx->take_stats) {1451 if (UNIV_UNLIKELY(trx->take_stats)) {
1452 ut_usectime(&sec, &ms);1452 ut_usectime(&sec, &ms);
1453 start_time = (ib_uint64_t)sec * 1000000 + ms;1453 start_time = (ib_uint64_t)sec * 1000000 + ms;
1454 } else {1454 } else {
@@ -1463,7 +1463,7 @@
14631463
1464 trx->op_info = "";1464 trx->op_info = "";
14651465
1466 if (innobase_get_slow_log() && trx->take_stats && start_time) {1466 if (UNIV_UNLIKELY(start_time != 0)) {
1467 ut_usectime(&sec, &ms);1467 ut_usectime(&sec, &ms);
1468 finish_time = (ib_uint64_t)sec * 1000000 + ms;1468 finish_time = (ib_uint64_t)sec * 1000000 + ms;
1469 trx->innodb_que_wait_timer += (ulint)(finish_time - start_time);1469 trx->innodb_que_wait_timer += (ulint)(finish_time - start_time);
14701470
=== modified file 'Percona-Server/storage/innobase/trx/trx0trx.c'
--- Percona-Server/storage/innobase/trx/trx0trx.c 2012-08-16 13:44:06 +0000
+++ Percona-Server/storage/innobase/trx/trx0trx.c 2013-02-26 09:04:21 +0000
@@ -236,7 +236,7 @@
236236
237 mutex_exit(&kernel_mutex);237 mutex_exit(&kernel_mutex);
238238
239 if (innobase_get_slow_log() && trx->take_stats) {239 if (UNIV_UNLIKELY(trx->take_stats)) {
240 trx->distinct_page_access_hash = mem_alloc(DPAH_SIZE);240 trx->distinct_page_access_hash = mem_alloc(DPAH_SIZE);
241 memset(trx->distinct_page_access_hash, 0, DPAH_SIZE);241 memset(trx->distinct_page_access_hash, 0, DPAH_SIZE);
242 }242 }
@@ -1269,7 +1269,7 @@
1269 thr = UT_LIST_GET_FIRST(trx->wait_thrs);1269 thr = UT_LIST_GET_FIRST(trx->wait_thrs);
1270 }1270 }
12711271
1272 if (innobase_get_slow_log() && trx->take_stats) {1272 if (UNIV_UNLIKELY(trx->take_stats)) {
1273 ut_usectime(&sec, &ms);1273 ut_usectime(&sec, &ms);
1274 now = (ib_uint64_t)sec * 1000000 + ms;1274 now = (ib_uint64_t)sec * 1000000 + ms;
1275 trx->lock_que_wait_timer += (ulint)(now - trx->lock_que_wait_ustarted);1275 trx->lock_que_wait_timer += (ulint)(now - trx->lock_que_wait_ustarted);
@@ -1304,7 +1304,7 @@
1304 thr = UT_LIST_GET_FIRST(trx->wait_thrs);1304 thr = UT_LIST_GET_FIRST(trx->wait_thrs);
1305 }1305 }
13061306
1307 if (innobase_get_slow_log() && trx->take_stats) {1307 if (UNIV_UNLIKELY(trx->take_stats)) {
1308 ut_usectime(&sec, &ms);1308 ut_usectime(&sec, &ms);
1309 now = (ib_uint64_t)sec * 1000000 + ms;1309 now = (ib_uint64_t)sec * 1000000 + ms;
1310 trx->lock_que_wait_timer += (ulint)(now - trx->lock_que_wait_ustarted);1310 trx->lock_que_wait_timer += (ulint)(now - trx->lock_que_wait_ustarted);

Subscribers

People subscribed via source and target branches