facebook / mysql-5.6

Facebook's branch of the Oracle MySQL database. This includes MyRocks.
http://myrocks.io
Other
2.47k stars 711 forks source link

add is_dd_engine_change variable #1378

Closed luqun closed 8 months ago

luqun commented 8 months ago

Summary: Innodb SE need to know whether DD engine change is in progress for its handler API to return correct data. such as get_se_private_data() should return mysql.dd_properties table data is DD engine change in in progress and also DDSE is rocksdb.

Expose is_dd_engine_change information in dd.h. also update other places to use is_dd_engine_change variable to check whether DD engine change is in progress.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

luqun commented 8 months ago

updated: fixed

luqun commented 8 months ago

@laurynas-biveinis , could you please take a look again?

facebook-github-bot commented 8 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

laurynas-biveinis commented 8 months ago

@luqun , I think I understand what happened with https://github.com/facebook/mysql-5.6/pull/1367 regressions, please hold merging this for short time

laurynas-biveinis commented 8 months ago

@luqun, so at least the part of what's going on is that my work branch has RocksDB as the default DDSE value. For tests, that had the effect of initializing the instance with RocksDB, and running RocksDB DDSE tests. Now on trunk, the test instance is initialized with InnoDB DDSE, and when the test tries to run with RocksDB DDSE, the first thing that happens is the crossgrade. Please go ahead with your PR, I'll adjust the tests to avoid this behavior too.

laurynas-biveinis commented 8 months ago

Another issue that the mysql_system_table.sql bit that prevented mysql.password being created in RocksDB got lost in its PR, while the code that was not, meaning that MyRocks DDSE instances will fail to initialize: https://github.com/facebook/mysql-5.6/pull/1381