MySQL Bugs: #65886: is the call to ibuf_merge_or_delete_for_page from buf_page_get

Bug #65886	is the call to ibuf_merge_or_delete_for_page from buf_page_get_gen redundant?
Submitted:	12 Jul 2012 19:24	Modified:	24 Dec 2012 19:11
Reporter:	Mark Callaghan	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: InnoDB Plugin storage engine	Severity:	S5 (Performance)
Version:	5.1.52, 5.1.64	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
When running sysbench to determine how many page reads per second I can get from InnoDB compressed tables performance is about 10% better (peak QPS) when the call to ibuf_merge_or_delete_for_page is removed. Given that the ibuf call is done from buf_page_io_complete, is the call from buf_page_get_gen redundant?

The problem is that this check is done on page decompression now rather than only on page reads.

How to repeat:
read the source

Suggested fix:
don't call it from buf_page_get_gen

Mark, sorry for not noticing this earlier. I was on vacation in July, and after that busy with MySQL 5.6 development.

For uncompressed pages, we call ibuf_merge_or_delete_from_page() from buf_page_io_complete().

If a compressed page is read from file without anyone explicitly asking for it, say, by read-ahead, we will not allocate an uncompressed page frame. In this case, the page will be decompressed in buf_page_get_gen(), and we may need to merge buffered changes.

I guess we could check some field in buf_page_t to determine whether this was the first buf_page_get_gen() request for the block. The ibuf merge is unnecessary as long as the block remained in the buffer pool since the last request. If it was a fresh read from file, we will have to do the ibuf merge.

I was thinking of a patch that would skip the call if buf_page_is_accessed() says that the page has been accessed before. This patch might depend on fixing
Bug#67476 Innodb_buffer_pool_read_ahead_evicted is inaccurate
if the inaccuracy is such that we return access_time!=0 for the first access. 

Returning access_time=0 for several accesses would not be hurting the correctness of the patch that I am thinking about, but it would not get rid of some redundant calls to ibuf_merge_or_delete_for_page().

Added to changelog for 5.1.68, 5.5.30, 5.6.10: 

Optimized read operations for compressed tables by skipping redundant
tests. The check for whether any related changes needed to be merged
from the insert buffer was being called more often than necessary.