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
=== modified file 'Percona-Server/storage/innobase/include/log0log.h'
--- Percona-Server/storage/innobase/include/log0log.h 2013-03-05 12:46:43 +0000
+++ Percona-Server/storage/innobase/include/log0log.h 2013-04-03 05:43:35 +0000
@@ -114,14 +114,22 @@
114void114void
115log_free_check(void);115log_free_check(void);
116/*================*/116/*================*/
117/**************************************************************************//**
118Locks the log mutex and opens the log for log_write_low. The log must be closed
119with log_close and released with log_release.
120@return start lsn of the log record */
121UNIV_INLINE
122lsn_t
123log_reserve_and_open(
124/*=================*/
125 ulint len); /*!< in: length of data to be catenated */
117/************************************************************//**126/************************************************************//**
118Opens the log for log_write_low. The log must be closed with log_close and127Opens the log for log_write_low. The log must be closed with log_close.
119released with log_release.
120@return start lsn of the log record */128@return start lsn of the log record */
121UNIV_INTERN129UNIV_INTERN
122lsn_t130lsn_t
123log_reserve_and_open(131log_open(
124/*=================*/132/*=====*/
125 ulint len); /*!< in: length of data to be catenated */133 ulint len); /*!< in: length of data to be catenated */
126/************************************************************//**134/************************************************************//**
127Writes to the log the string given. It is assumed that the caller holds the135Writes to the log the string given. It is assumed that the caller holds the
128136
=== modified file 'Percona-Server/storage/innobase/include/log0log.ic'
--- Percona-Server/storage/innobase/include/log0log.ic 2012-08-22 01:40:20 +0000
+++ Percona-Server/storage/innobase/include/log0log.ic 2013-04-03 05:43:35 +0000
@@ -333,10 +333,10 @@
333333
334 if (data_len >= OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE) {334 if (data_len >= OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE) {
335335
336 /* The string does not fit within the current log block336 /* The string does not fit within the current log block or the
337 or the log block would become full */337 log block would become full. Do not release the log mutex,
338338 because it has to be reacquired immediately for the "slow" write
339 mutex_exit(&log_sys->mutex);339 procedure via log_write_low(). */
340340
341 return(0);341 return(0);
342 }342 }
@@ -386,6 +386,21 @@
386 return(log_sys->lsn);386 return(log_sys->lsn);
387}387}
388388
389/**************************************************************************//**
390Locks the log mutex and opens the log for log_write_low. The log must be closed
391with log_close and released with log_release.
392@return start lsn of the log record */
393UNIV_INLINE
394ib_uint64_t
395log_reserve_and_open(
396/*=================*/
397 ulint len) /*!< in: length of data to be catenated */
398{
399 mutex_enter(&(log_sys->mutex));
400
401 return log_open(len);
402}
403
389/***********************************************************************//**404/***********************************************************************//**
390Releases the log mutex. */405Releases the log mutex. */
391UNIV_INLINE406UNIV_INLINE
392407
=== modified file 'Percona-Server/storage/innobase/log/log0log.cc'
--- Percona-Server/storage/innobase/log/log0log.cc 2013-03-05 12:46:43 +0000
+++ Percona-Server/storage/innobase/log/log0log.cc 2013-04-03 05:43:35 +0000
@@ -184,13 +184,12 @@
184}184}
185185
186/************************************************************//**186/************************************************************//**
187Opens the log for log_write_low. The log must be closed with log_close and187Opens the log for log_write_low. The log must be closed with log_close.
188released with log_release.
189@return start lsn of the log record */188@return start lsn of the log record */
190UNIV_INTERN189UNIV_INTERN
191lsn_t190lsn_t
192log_reserve_and_open(191log_open(
193/*=================*/192/*=====*/
194 ulint len) /*!< in: length of data to be catenated */193 ulint len) /*!< in: length of data to be catenated */
195{194{
196 log_t* log = log_sys;195 log_t* log = log_sys;
@@ -205,7 +204,6 @@
205204
206 ut_a(len < log->buf_size / 2);205 ut_a(len < log->buf_size / 2);
207loop:206loop:
208 mutex_enter(&(log->mutex));
209 ut_ad(!recv_no_log_write);207 ut_ad(!recv_no_log_write);
210208
211 /* Calculate an upper limit for the space the string may take in the209 /* Calculate an upper limit for the space the string may take in the
@@ -226,6 +224,8 @@
226224
227 ut_ad(++count < 50);225 ut_ad(++count < 50);
228226
227 mutex_enter(&(log->mutex));
228
229 goto loop;229 goto loop;
230 }230 }
231231
@@ -246,6 +246,8 @@
246246
247 ut_ad(++count < 50);247 ut_ad(++count < 50);
248248
249 mutex_enter(&(log->mutex));
250
249 goto loop;251 goto loop;
250 }252 }
251 }253 }
252254
=== modified file 'Percona-Server/storage/innobase/mtr/mtr0mtr.cc'
--- Percona-Server/storage/innobase/mtr/mtr0mtr.cc 2013-03-05 12:46:43 +0000
+++ Percona-Server/storage/innobase/mtr/mtr0mtr.cc 2013-04-03 05:43:35 +0000
@@ -265,12 +265,14 @@
265265
266 return;266 return;
267 }267 }
268 } else {
269 mutex_enter(&log_sys->mutex);
268 }270 }
269271
270 data_size = dyn_array_get_data_size(mlog);272 data_size = dyn_array_get_data_size(mlog);
271273
272 /* Open the database log for log_write_low */274 /* Open the database log for log_write_low */
273 mtr->start_lsn = log_reserve_and_open(data_size);275 mtr->start_lsn = log_open(data_size);
274276
275 if (mtr->log_mode == MTR_LOG_ALL) {277 if (mtr->log_mode == MTR_LOG_ALL) {
276278

Subscribers

People subscribed via source and target branches