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

[Alexey Kopytov]

  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/

[Laurynas Biveinis (laurynas-biveinis)]

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

review: Approve

=== 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 @@
 void
 log_free_check(void);
 /*================*/
+/**************************************************************************//**
+Locks the log mutex and opens the log for log_write_low. The log must be closed
+with log_close and released with log_release.
+@return start lsn of the log record */
+UNIV_INLINE
+lsn_t
+log_reserve_and_open(
+/*=================*/
+	ulint	len);	/*!< in: length of data to be catenated */
 /************************************************************//**
-Opens the log for log_write_low. The log must be closed with log_close and
-released with log_release.
+Opens the log for log_write_low. The log must be closed with log_close.
 @return	start lsn of the log record */
 UNIV_INTERN
 lsn_t
-log_reserve_and_open(
-/*=================*/
+log_open(
+/*=====*/
 	ulint	len);	/*!< in: length of data to be catenated */
 /************************************************************//**
 Writes to the log the string given. It is assumed that the caller holds the
 
=== 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 @@
 
 	if (data_len >= OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE) {
 
-		/* The string does not fit within the current log block
-		or the log block would become full */
-
-		mutex_exit(&log_sys->mutex);
+		/* The string does not fit within the current log block or the
+		log block would become full. Do not release the log mutex,
+		because it has to be reacquired immediately for the "slow" write
+		procedure via log_write_low(). */
 
 		return(0);
 	}
@@ -386,6 +386,21 @@
 	return(log_sys->lsn);
 }
 
+/**************************************************************************//**
+Locks the log mutex and opens the log for log_write_low. The log must be closed
+with log_close and released with log_release.
+@return start lsn of the log record */
+UNIV_INLINE
+ib_uint64_t
+log_reserve_and_open(
+/*=================*/
+	ulint	len)	/*!< in: length of data to be catenated */
+{
+	mutex_enter(&(log_sys->mutex));
+
+	return log_open(len);
+}
+
 /***********************************************************************//**
 Releases the log mutex. */
 UNIV_INLINE
 
=== 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 @@
 }
 
 /************************************************************//**
-Opens the log for log_write_low. The log must be closed with log_close and
-released with log_release.
+Opens the log for log_write_low. The log must be closed with log_close.
 @return	start lsn of the log record */
 UNIV_INTERN
 lsn_t
-log_reserve_and_open(
-/*=================*/
+log_open(
+/*=====*/
 	ulint	len)	/*!< in: length of data to be catenated */
 {
 	log_t*	log			= log_sys;
@@ -205,7 +204,6 @@
 
 	ut_a(len < log->buf_size / 2);
 loop:
-	mutex_enter(&(log->mutex));
 	ut_ad(!recv_no_log_write);
 
 	/* Calculate an upper limit for the space the string may take in the
@@ -226,6 +224,8 @@
 
 		ut_ad(++count < 50);
 
+		mutex_enter(&(log->mutex));
+
 		goto loop;
 	}
 
@@ -246,6 +246,8 @@
 
 			ut_ad(++count < 50);
 
+			mutex_enter(&(log->mutex));
+
 			goto loop;
 		}
 	}
 
=== 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 @@
 
 			return;
 		}
+	} else {
+		mutex_enter(&log_sys->mutex);
 	}
 
 	data_size = dyn_array_get_data_size(mlog);
 
 	/* Open the database log for log_write_low */
-	mtr->start_lsn = log_reserve_and_open(data_size);
+	mtr->start_lsn = log_open(data_size);
 
 	if (mtr->log_mode == MTR_LOG_ALL) {