Workaround the improper behavior of MySQL shell with respect to auto increment parameters

[gianlucaborello]

I work with an application that for some reason assumes that auto increment columns always start at 1, and increase sequentially after that.

When using InnoDB Cluster, this can create a problem if we're using multiple masters, because concurrent writes from different masters could cause a heavy contention rate in the incremental space, and that's the reason why we have options such as group_replication_auto_increment_increment, auto_increment_increment and auto_increment_offset which are recommended to be set to some >1 unique values for every node, when using multi primary configurations. However, if we are in single primary mode, there should be no risk, since the cluster itself makes sure that writes can happen on one master at a time, so concurrency is not an issue. For this reason, auto_increment_increment and auto_increment_offset can be safely kept to 1 (the MySQL default).

However, when using MySQL Shell to provision a cluster, Shell includes its own logic to reset the auto increment values to what might seem like more suitable defaults:

https://github.com/mysql/mysql-shell/blob/8.0.11/python/mysql_gadgets/common/group_replication.py#L2210

These parameters appear to be recommitted after every cluster membership command in Shell (https://github.com/mysql/mysql-shell/blob/8.0.11/python/mysql_gadgets/common/group_replication.py#L553), as shown in the general query log:

2018-06-15T23:05:11.316643Z        17 Query     SET PERSIST auto_increment_offset = 7
2018-06-15T23:05:11.329589Z        17 Query     SET PERSIST auto_increment_increment = 7

But the logic at https://github.com/mysql/mysql-shell/blob/8.0.11/python/mysql_gadgets/common/group_replication.py#L2210 is wrong for two reasons:

1. The options.get("single_primary", None) == "ON" simply doesn't get evaluated as true every time we are single primary mode, because in other parts of the code, an absence of "single_primary" in the dictionary signifies indeed single primary (https://github.com/mysql/mysql-shell/blob/8.0.11/modules/adminapi/mod_dba_provisioning_interface.cc#L722). For example, one of these instances is when a cluster is bootstrapped. In such cases, we enter the false branch for the node that bootstrap the cluster, whereas the other nodes that join as secondary end up in the true branch, leading to non-correlated set of values for the cluster, for example:

Node 1, that bootstrapped the cluster:
+--------------------------------------------+-------+
| Variable_name                              | Value |
+--------------------------------------------+-------+
| auto_increment_increment                   | 7     |
| auto_increment_offset                      | 7     |
| group_replication_auto_increment_increment | 7     |
+--------------------------------------------+-------+

Node 2, that joined as secondary:
+--------------------------------------------+-------+
| Variable_name                              | Value |
+--------------------------------------------+-------+
| auto_increment_increment                   | 1     |
| auto_increment_offset                      | 2     |
| group_replication_auto_increment_increment | 7     |
+--------------------------------------------+-------+

Node 3, that joined as secondary:
+--------------------------------------------+-------+
| Variable_name                              | Value |
+--------------------------------------------+-------+
| auto_increment_increment                   | 1     |
| auto_increment_offset                      | 2     |
| group_replication_auto_increment_increment | 7     |
+--------------------------------------------+-------+

It's definitely safe to say that this is not the correct range of settings for a single primary, nor for a multi primary.

2. Even if the logic didn't contain the bug as step 1), and the if would work as intended (meaning that all nodes would show increment=1, offset=2), it's still wrong to reset offset to 2 under the assumption that MySQL would otherwise reset it to the value of group_replication_auto_increment_increment, that behavior has been fixed in 8.0 (mysql/mysql-server@846ced2) and I think Shell should do a more thorough version check before messing with those parameters, penalizing users who are expecting a certain behavior on the latest version of MySQL.

Since my understanding is that this operator only supports MySQL 8, it should be safe to reset and persist the auto increment settings to 1/1 when in single primary mode, after any cluster membership operation, and obviously ultimately this behavior should be addressed by Shell in upstream.

Thanks

[prydie]

Looks good, thanks for the contribution 👍 .

My only concern is backwards compatibility with existing clusters. Have you considered the impact resetting the value would have on a production cluster? Not my area of expertise so might be absolutely fine but I'd be keen to know.

[gianlucaborello]

Thanks for reviewing.

In my opinion, and testing, everything works fine (but I also have an extremely limited experience with MySQL). I would understand your point if auto_increment_increment was set by Shell to a constant value across the whole cluster (e.g. always 7). But here, as proven above, values get essentially set pseudo-randomly, because the initial master will have 7 and the others will have 2, so if some application weirdly relies on that 7 to be always 7, that would stop being correct after a failover happens and a slave gets elected. So, in my opinion it's safe to have a new and consistent behavior.

That being said, I would certainly feel more safe if someone from the Shell team (I don't know if you have a way to ping them from your side?) could chime in and ack the issue.

[gianlucaborello]

Hi, was wondering if by any chance there are any updates on this? Let me know in case you'd like for me to open an issue to mysql shell or something and see if some developer from there agrees with my reasoning.

Thanks

[owainlewis]

Thanks for this @gianlucaborello. Both the code and analysis look good. We're chasing this up internally with the MySQL shell team to get their input on the issue.

[gianlucaborello]

Thank you for taking a lead on this, much appreciated!

[owainlewis]

FWIW I believe the bug is known internally and we are still waiting on further details around a fix. I'm looking to get a member of the MySQL shell team to review this PR so it may hang around for a little while until we get clarity.

[owainlewis]

Still waiting on further information from the MySQL shell team.