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
Triage: Needs Triage: D3 (Medium)

[12 Jul 2012 19:24] Mark Callaghan
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
[3 Oct 2012 12:21] Marko Mäkelä
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.
[12 Nov 2012 14:49] Marko Mäkelä
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().
[24 Dec 2012 19:11] John Russell
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.