MySQL Bugs: #69146: Needless log flush order mutex acquisition in buf_pool_get_oldest

Bug #69146	Needless log flush order mutex acquisition in buf_pool_get_oldest_modification
Submitted:	5 May 2013 7:50	Modified:	3 Jun 2015 10:36
Reporter:	Laurynas Biveinis (OCA)	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S5 (Performance)
Version:	5.5, 5.6, 5.7	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	buffer pool, innodb, mutexes

Description:
buf_pool_get_oldest_modification() loops over all buffer pool instances, acquiring the flush list mutex for each of them.

Before the loop it enters the log flush order mutex:

	/* When we traverse all the flush lists we don't want another
	thread to add a dirty page to any flush list. */
	log_flush_order_mutex_enter();

In the case of srv_buf_pool_instances == 1 this is unnecessary as the flush list mutex will take care of it

How to repeat:
By code reading.

Suggested fix:
Guard log flush order mutex acquisition by srv_buf_pool_instances == 1 check. Yasufumi added this patch to XtraDB 5.5 starting from day one with no ill effects.

I'll have Yasufumi take a look.

Actually, taking log flush order mutex in this function seems to be completely redundant for any number of buffer pool instances. All the relevant mutexes are released by the time function quits, making its result potentially outdated. Even without that, if some instance will receive a new dirty page, its first modification LSN will not be older than the result of this function.

Adjusting title

Bug 69146 patch for 5.7 (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug69146.patch (application/octet-stream, text), 1.02 KiB.

The single thread case I can see benefiting in some way but the log order mutex is required for correctness. If XtrDB doesn't do the same thing then it has divine blessing :-) is all I can say, or does something which is different from Oracle InnoDB :-)

"r6987 | sbains | 2010-04-14 00:14:13 +0300 (Wed, 14 Apr 2010) | 12 lines
      Changed paths:
         M /branches/innodb+multibp/buf/buf0buf.c
    
      branches/innodb+multibp: When calculating the oldest_lsn we can have a
      situation where we've iterated to say buffer pool 3 and another thread
      adds two new dirty pages, the first to buffer pool 1 and the second to
      buffer pool 4. Up to say buffer pool 3 the oldest_lsn was 0. Now, we will
      end up returning the lsn at buffer pool 4 as the oldest LSN. We prevent this
      by acquiring the flush order mutex.
    
      One other future option is to calculate the min_lsn when flushing pages
      from the list and maintaining a running total using atomics. That way
      we can get rid of this function altogether. The atomics will only really
      be required when we do parallel flushing.
"

Should have been "single buffer case" not "single thread case"

You are right, this function needs the log flush order mutex if running with more than one instance.