Bug #68970 fsp_reserve_free_extents switches from small to big tblspace handling too early
Submitted: 16 Apr 2013 9:18 Modified: 18 Dec 2014 19:54
Reporter: Laurynas Biveinis Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.6.10 OS:Any
Assigned to: CPU Architecture:Any
Tags: fsp, innodb

[16 Apr 2013 9:18] Laurynas Biveinis
Description:
fsp_reserve_free_extents() works by accident or extremely counter-intuitively for tablespaces between 32 and 64 pages.

First, all newly-created tablespaces have their FSP_FREE_LIMIT initialized to 64 in fsp_fill_free_list(). And it is explained next to FSP_FREE_LIMIT define that 

"note that in a single-table tablespace where size < 64 pages, this number is 64, i.e., we have initialized the space about the first extent, but have not physically allocted those pages to the file"

But fsp_reserve_free_extents has special handling for small tablespaces that checks for size < 32 pages only:

	if (size < FSP_EXTENT_SIZE / 2) {
		/* Use different rules for small single-table tablespaces */
		*n_reserved = 0;
		return(fsp_reserve_free_pages(space, space_header, size, mtr));
	}

This means that, whenever size > 32 and < 64, fsp_reserve_free_extents() will end up with very large positive values in its calculations because of  
n_free_up = (size - free_limit) / FSP_EXTENT_SIZE;
where free_limit = 64 and size < 64.

If that kind of unsigned math was intended there, it should be explained at least. But the code, including the call to fil_space_reserve_free_extents() with n_free_now close to ULINT_MAX does not seem support that.

Some sample var values with size between 32 and 64:

(gdb) print n_free_up
$3 = 288230376151711743

(gdb) print n_free_up
$4 = 270215977642229759

(gdb) print n_free
$6 = 270215977642229759

2808			if (n_free <= reserve + n_ext) {
(gdb) print reserve
$9 = 2
(gdb) print n_ext
$10 = 3
(gdb) n
2825		success = fil_space_reserve_free_extents(space, n_free, n_ext);

My first fix idea would be to cut off small tablespace handling at 64 pages instead of 32 pages. But I am not familiar with this code enough to suggest that this would be correct.

How to repeat:
innodb_wl6347_comp_indx_stat.

Note that my suggested fix perturbs the resulting size of tablespaces as checked by the testcase:

@@ -987,7 +987,7 @@
 AND table_name='tab5' AND database_name='test'
 AND index_name like 'idx%' ;
 compress_stat	1
-The size of the tab5.ibd file: 159744 
+The size of the tab5.ibd file: 163840 
 # fetch the compressed page and check the stats
 ===============
 Fetch Records
@@ -1013,7 +1013,7 @@
 AND table_name='tab5' AND database_name='test'
 AND index_name like 'idx%' ;
 compress_stat	1
-The size of the tab5.ibd file: 159744 
+The size of the tab5.ibd file: 163840 
 # fetch the compressed same page once again and check the stats
 # the stat figures should be same as above query
 ===============
@@ -1040,7 +1040,7 @@
 AND table_name='tab5' AND database_name='test'
 AND index_name like 'idx%' ;
 compress_stat	1
-The size of the tab5.ibd file: 159744 
+The size of the tab5.ibd file: 163840 
 #cleanup
 DROP TABLE IF EXISTS tab5;
 #reset the stat table before starting next testcase

Suggested fix:
=== modified file 'storage/innobase/fsp/fsp0fsp.cc'
--- storage/innobase/fsp/fsp0fsp.cc	2012-09-28 06:10:51 +0000
+++ storage/innobase/fsp/fsp0fsp.cc	2013-04-16 09:05:18 +0000
@@ -2681,7 +2681,7 @@
 	ulint	n_used;
 
 	ut_a(space != 0);
-	ut_a(size < FSP_EXTENT_SIZE / 2);
+	ut_a(size < FSP_EXTENT_SIZE);
 
 	descr = xdes_get_descriptor_with_space_hdr(space_header, space, 0,
 						   mtr);
@@ -2761,7 +2761,7 @@
 try_again:
 	size = mtr_read_ulint(space_header + FSP_SIZE, MLOG_4BYTES, mtr);
 
-	if (size < FSP_EXTENT_SIZE / 2) {
+	if (size < FSP_EXTENT_SIZE) {
 		/* Use different rules for small single-table tablespaces */
 		*n_reserved = 0;
 		return(fsp_reserve_free_pages(space, space_header, size, mtr));
@@ -2776,6 +2776,7 @@
 	some of them will contain extent descriptor pages, and therefore
 	will not be free extents */
 
+	ut_ad (size >= free_limit);
 	n_free_up = (size - free_limit) / FSP_EXTENT_SIZE;
 
 	if (n_free_up > 0) {
[20 Apr 2013 18:05] MySQL Verification Team
verified in debugger.  size was 33, free_limit was 64 and n_free_up was 288230376151711743
[7 Nov 2014 14:35] Laurynas Biveinis
Bug 68970 fix 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: bug68970.patch (application/octet-stream, text), 3.69 KiB.

[18 Dec 2014 19:31] Daniel Price
Posted by developer:
 
   Bug#16696906 - FSP_RESERVE_FREE_EXTENTS SWITCHES FROM SMALL TO BIG TABLESPAC
               HANDLING TOO EARLY
    
    The small tablespace criteria is inconsistent. At some places, we say 32
    pages is small tablespace and at other places, we say 64 pages is small
    tablespace.
    
    Fix:
    ----
    Use 64 pages as small tablespace criteria when reserving extents.
    This is because FSP_FREE_LIMIT is initialized to 64 for newly created
    tablespace.
    
    Reviewed-by: Kevin Lewis <kevin.lewis@oracle.com>
    RB: 7437
[18 Dec 2014 19:54] Daniel Price
Posted by developer:
 
Fixed as of the upcoming 5.7.6 release, and here's the changelog entry:

The criteria used to define a small tablespace was inconsistent. Thanks
to Laurynas Biveinis for the patch.

Thank you for the bug report.