Fix #1651657, failback from backup for legacy for small BP instance size #1262

ihanick · 2016-12-25T09:03:52Z

https://bugs.launchpad.net/percona-server/+bug/1651657
http://jenkins.percona.com/job/percona-server-5.6-param/1560/

Small buffer pool instance size and backoff algorithm causing deadlock, failback to legacy algorithm in such cases.

laurynas-biveinis

Please also check the Jenkins run for sys_vars.innodb_empty_free_list_algorithm_basic failure

laurynas-biveinis · 2016-12-27T09:45:22Z

mysql-test/t/percona_bug1651657.test

@@ -0,0 +1,11 @@
+--source include/have_innodb.inc
+--let $assert_text= Small buffer pool instance should use legacy


s/instance/instances

laurynas-biveinis · 2016-12-27T09:45:41Z

mysql-test/t/percona_bug1651657.test

+--source include/assert.inc
+
+call mtr.add_suppression("InnoDB: innodb_empty_free_list_algorithm = 'backoff' requires at least 20MB buffer pool instances.");
+--error 1231


Please use ER_ mnemonics

laurynas-biveinis · 2016-12-27T09:46:40Z

mysql-test/t/percona_bug1651657.test

@@ -0,0 +1,11 @@
+--source include/have_innodb.inc


Please move the testcase to the innodb suite

laurynas-biveinis · 2016-12-27T09:51:03Z

storage/innobase/handler/ha_innodb.cc

@@ -3880,7 +3880,8 @@ innobase_init(
 	if (!innodb_empty_free_list_algorithm_backoff_allowed(


There are some code smells in innodb_empty_free_list_algorithm_backoff_allowed: 1) the name hardcodes that it is "backoff", but the 1st arg passes the actual algorithm to verify; 2) the calculation innobase_buffer_pool_size / srv_page_size / innobase_buffer_pool_instance is the only correct way to pass the 2nd arg.

Thus:

Please rename to innodb_empty_free_list_algorithm_allowed

Please inline and remove the 2nd arg

ihanick · 2016-12-29T05:52:55Z

http://jenkins.percona.com/job/percona-server-5.6-param/1567/

renamed to innodb_empty_free_list_algorithm_allowed
moved number of pages in buffer pool calculation to this function
using new InnoDB header comment style
moved function declaration slightly above to avoid merge conflict
using --error ER_WRONG_VALUE_FOR_VAR in test
test now inside innodb suite
there is no limitation for minimal buffer pool size to have several instances (looks like a bug, because instance could be smaller than minimum single buffer pool size), forced sys_vars.innodb_empty_free_list_algorithm_basic to use single buffer pool instance.
changed innodb buffer pool size and number of instances to the same value as in 5.7-ps-bug1651657

laurynas-biveinis · 2016-12-30T04:58:52Z

storage/innobase/handler/ha_innodb.cc

@@ -1346,6 +1346,24 @@ innodb_enable_monitor_at_startup(
 /*=============================*/
 	char*	str);	/*!< in: monitor counter enable list */

+/** Checks if buffer pool is big enough to enable backoff algorithm.


"Check if enabling given empty free list algorithm is allowed"

The header comment slightly differs between this declaration and the actual definition. I'd just move definition higher in the file so that there is no need for the declaration

laurynas-biveinis · 2016-12-30T04:59:16Z

storage/innobase/handler/ha_innodb.cc

 @return	true if it's possible to enable backoff. */
-static
+static inline


Just static

laurynas-biveinis · 2016-12-30T04:59:50Z

storage/innobase/handler/ha_innodb.cc

 {
+	long long buf_pool_pages = srv_buf_pool_size / srv_page_size
+				/ (srv_buf_pool_instances > 1 ?


Why the ternary operator here?

ihanick · 2017-02-17T14:25:55Z

http://jenkins.percona.com/job/percona-server-5.6-param/1706/

removed declaration for innodb_empty_free_list_algorithm_backoff_allowed
removed ternary operator from buf_pool_pages calculation
removed "inline"

Upstream commit ID: facebook/mysql-5.6@c75bf30 PS-8755: Merge percona-202301 (https://jira.percona.com/browse/PS-8755) Summary: Commit 8a8708b71da379daea13fcae90e3bb1af5d60f32 (MyRocks to support both dynamic and static plugins) caused compiler error on Ubuntu 22.04 with gcc (Issue percona#1262). This diff fixes it. squash with: D41313174 Reviewed By: lth Differential Revision: D42646920 fbshipit-source-id: 16e77bc5b0cbcc464be7ce587b37da78b4b3de8f

Upstream commit ID: facebook/mysql-5.6@c75bf30 PS-8755: Merge percona-202301 (https://jira.percona.com/browse/PS-8755) Summary: Commit 8a8708b71da379daea13fcae90e3bb1af5d60f32 (MyRocks to support both dynamic and static plugins) caused compiler error on Ubuntu 22.04 with gcc (Issue #1262). This diff fixes it. squash with: D41313174 Reviewed By: lth Differential Revision: D42646920 fbshipit-source-id: 16e77bc5b0cbcc464be7ce587b37da78b4b3de8f

Upstream commit ID: facebook/mysql-5.6@c75bf30 PS-8755: Merge percona-202301 (https://jira.percona.com/browse/PS-8755) Summary: Commit 8a8708b71da379daea13fcae90e3bb1af5d60f32 (MyRocks to support both dynamic and static plugins) caused compiler error on Ubuntu 22.04 with gcc (Issue percona#1262). This diff fixes it. squash with: D41313174 Reviewed By: lth Differential Revision: D42646920 fbshipit-source-id: 16e77bc5b0cbcc464be7ce587b37da78b4b3de8f

ihanick mentioned this pull request Dec 25, 2016

Fix #1651657, failback from backup for legacy for small BP instance size - 5.7 #1263

Merged

laurynas-biveinis self-assigned this Dec 27, 2016

laurynas-biveinis suggested changes Dec 27, 2016

View reviewed changes

ihanick force-pushed the 5.6-ps-bug1651657 branch 4 times, most recently from 6c78a4b to d5b7071 Compare December 29, 2016 03:08

laurynas-biveinis suggested changes Dec 30, 2016

View reviewed changes

Fix #1651657, failback from backup for legacy for small BP instance size

4d70ecc

ihanick force-pushed the 5.6-ps-bug1651657 branch from d5b7071 to 4d70ecc Compare February 17, 2017 13:35

laurynas-biveinis approved these changes Feb 20, 2017

View reviewed changes

ihanick merged commit ca4ba8d into percona:5.6 Mar 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1651657, failback from backup for legacy for small BP instance size #1262

Fix #1651657, failback from backup for legacy for small BP instance size #1262

ihanick commented Dec 25, 2016

laurynas-biveinis left a comment

laurynas-biveinis Dec 27, 2016

laurynas-biveinis Dec 27, 2016

laurynas-biveinis Dec 27, 2016

laurynas-biveinis Dec 27, 2016

ihanick commented Dec 29, 2016

laurynas-biveinis Dec 30, 2016

laurynas-biveinis Dec 30, 2016

laurynas-biveinis Dec 30, 2016

ihanick commented Feb 17, 2017

		@@ -0,0 +1,11 @@
		--source include/have_innodb.inc
		--let $assert_text= Small buffer pool instance should use legacy

		@@ -3880,7 +3880,8 @@ innobase_init(
		if (!innodb_empty_free_list_algorithm_backoff_allowed(

Fix #1651657, failback from backup for legacy for small BP instance size #1262

Fix #1651657, failback from backup for legacy for small BP instance size #1262

Conversation

ihanick commented Dec 25, 2016

laurynas-biveinis left a comment

Choose a reason for hiding this comment

laurynas-biveinis Dec 27, 2016

Choose a reason for hiding this comment

laurynas-biveinis Dec 27, 2016

Choose a reason for hiding this comment

laurynas-biveinis Dec 27, 2016

Choose a reason for hiding this comment

laurynas-biveinis Dec 27, 2016

Choose a reason for hiding this comment

ihanick commented Dec 29, 2016

laurynas-biveinis Dec 30, 2016

Choose a reason for hiding this comment

laurynas-biveinis Dec 30, 2016

Choose a reason for hiding this comment

laurynas-biveinis Dec 30, 2016

Choose a reason for hiding this comment

ihanick commented Feb 17, 2017