Merge lp:~akopytov/percona-server/bug1163439-5.6 into lp:percona-server/5.6

Proposed by Alexey Kopytov
Status: Merged
Approved by: Laurynas Biveinis
Approved revision: 326
Merged at revision: 328
Proposed branch: lp:~akopytov/percona-server/bug1163439-5.6
Merge into: lp:percona-server/5.6
Diff against target: 137 lines (+41/-14)
4 files modified
Percona-Server/storage/innobase/include/log0log.h (+12/-4)
Percona-Server/storage/innobase/include/log0log.ic (+19/-4)
Percona-Server/storage/innobase/log/log0log.cc (+7/-5)
Percona-Server/storage/innobase/mtr/mtr0mtr.cc (+3/-1)
To merge this branch: bzr merge lp:~akopytov/percona-server/bug1163439-5.6
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Approve
Review via email: mp+156740@code.launchpad.net

Description of the change

  Bug #1163439: Unnecessary log_sys->mutex reacquisition in
                mtr_log_reserve_and_write()

  mtr_log_reserve_and_write() implements the following logic with respect
  to log_sys->mutex: if the mini-transaction log contains a single block,
  it calls log_reserve_and_write_fast() which acquires log_sys->mutex and
  does a "fast" write by appending the new record to the current log
  block. If the record does not fit in the current log block,
  log_reserve_and_write_fast() releases log_sys->mutex and returns 0, in
  which case mtr_log_reserve_and_write() immediately reacquires
  log_sys->mutex by calling log_reserve_and_open() and proceeds with the
  "slow" write procedure.

  It doesn't make sense to release a mutex and reacquire it immediately
  and benchmarks show that avoiding this helps to reduce log_sys->mutex
  contention in some write-intensive workloads.

  Fixed by implementing an alternative to log_reserve_and_open() which
  doesn't acquire the mutex (log_open()), removing mutex_exit() from
  log_reserve_and_write_fast() when the log record does not fit in the
  current block, and using log_open() instead of log_reserve_and_open() in
  mtr_log_reserve_and_write().

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/73/

To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Looks good, the only question is whether an upstream bug should be logged, asked on the bug report itself.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/storage/innobase/include/log0log.h'
2--- Percona-Server/storage/innobase/include/log0log.h 2013-03-05 12:46:43 +0000
3+++ Percona-Server/storage/innobase/include/log0log.h 2013-04-03 05:43:35 +0000
4@@ -114,14 +114,22 @@
5 void
6 log_free_check(void);
7 /*================*/
8+/**************************************************************************//**
9+Locks the log mutex and opens the log for log_write_low. The log must be closed
10+with log_close and released with log_release.
11+@return start lsn of the log record */
12+UNIV_INLINE
13+lsn_t
14+log_reserve_and_open(
15+/*=================*/
16+ ulint len); /*!< in: length of data to be catenated */
17 /************************************************************//**
18-Opens the log for log_write_low. The log must be closed with log_close and
19-released with log_release.
20+Opens the log for log_write_low. The log must be closed with log_close.
21 @return start lsn of the log record */
22 UNIV_INTERN
23 lsn_t
24-log_reserve_and_open(
25-/*=================*/
26+log_open(
27+/*=====*/
28 ulint len); /*!< in: length of data to be catenated */
29 /************************************************************//**
30 Writes to the log the string given. It is assumed that the caller holds the
31
32=== modified file 'Percona-Server/storage/innobase/include/log0log.ic'
33--- Percona-Server/storage/innobase/include/log0log.ic 2012-08-22 01:40:20 +0000
34+++ Percona-Server/storage/innobase/include/log0log.ic 2013-04-03 05:43:35 +0000
35@@ -333,10 +333,10 @@
36
37 if (data_len >= OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE) {
38
39- /* The string does not fit within the current log block
40- or the log block would become full */
41-
42- mutex_exit(&log_sys->mutex);
43+ /* The string does not fit within the current log block or the
44+ log block would become full. Do not release the log mutex,
45+ because it has to be reacquired immediately for the "slow" write
46+ procedure via log_write_low(). */
47
48 return(0);
49 }
50@@ -386,6 +386,21 @@
51 return(log_sys->lsn);
52 }
53
54+/**************************************************************************//**
55+Locks the log mutex and opens the log for log_write_low. The log must be closed
56+with log_close and released with log_release.
57+@return start lsn of the log record */
58+UNIV_INLINE
59+ib_uint64_t
60+log_reserve_and_open(
61+/*=================*/
62+ ulint len) /*!< in: length of data to be catenated */
63+{
64+ mutex_enter(&(log_sys->mutex));
65+
66+ return log_open(len);
67+}
68+
69 /***********************************************************************//**
70 Releases the log mutex. */
71 UNIV_INLINE
72
73=== modified file 'Percona-Server/storage/innobase/log/log0log.cc'
74--- Percona-Server/storage/innobase/log/log0log.cc 2013-03-05 12:46:43 +0000
75+++ Percona-Server/storage/innobase/log/log0log.cc 2013-04-03 05:43:35 +0000
76@@ -184,13 +184,12 @@
77 }
78
79 /************************************************************//**
80-Opens the log for log_write_low. The log must be closed with log_close and
81-released with log_release.
82+Opens the log for log_write_low. The log must be closed with log_close.
83 @return start lsn of the log record */
84 UNIV_INTERN
85 lsn_t
86-log_reserve_and_open(
87-/*=================*/
88+log_open(
89+/*=====*/
90 ulint len) /*!< in: length of data to be catenated */
91 {
92 log_t* log = log_sys;
93@@ -205,7 +204,6 @@
94
95 ut_a(len < log->buf_size / 2);
96 loop:
97- mutex_enter(&(log->mutex));
98 ut_ad(!recv_no_log_write);
99
100 /* Calculate an upper limit for the space the string may take in the
101@@ -226,6 +224,8 @@
102
103 ut_ad(++count < 50);
104
105+ mutex_enter(&(log->mutex));
106+
107 goto loop;
108 }
109
110@@ -246,6 +246,8 @@
111
112 ut_ad(++count < 50);
113
114+ mutex_enter(&(log->mutex));
115+
116 goto loop;
117 }
118 }
119
120=== modified file 'Percona-Server/storage/innobase/mtr/mtr0mtr.cc'
121--- Percona-Server/storage/innobase/mtr/mtr0mtr.cc 2013-03-05 12:46:43 +0000
122+++ Percona-Server/storage/innobase/mtr/mtr0mtr.cc 2013-04-03 05:43:35 +0000
123@@ -265,12 +265,14 @@
124
125 return;
126 }
127+ } else {
128+ mutex_enter(&log_sys->mutex);
129 }
130
131 data_size = dyn_array_get_data_size(mlog);
132
133 /* Open the database log for log_write_low */
134- mtr->start_lsn = log_reserve_and_open(data_size);
135+ mtr->start_lsn = log_open(data_size);
136
137 if (mtr->log_mode == MTR_LOG_ALL) {
138

Subscribers

People subscribed via source and target branches