facebook / mysql-5.6

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

Make InnoDB work when it's not the DDSE #1367

Closed laurynas-biveinis closed 9 months ago

laurynas-biveinis commented 10 months ago

No functional changes when InnoDB is the DDSE. When InnoDB is not the DDSE:

laurynas-biveinis commented 10 months ago

@luqun as discussed, this is the mysql.dd_properties_placeholder patch.

While the PR is not complete in that InnoDB is not fully debugged yet, it is safe to merge

laurynas-biveinis commented 9 months ago

Added support for the missing innodb_page_size values (8k, 32k, & 64k), and rebased

laurynas-biveinis commented 9 months ago

Added a check to innobase_commit to avoid starting an already-prepared transaction if GTIDs need to be persisted explicitly, added a regression testcase for this change rocksdb_dd_innodb.rpl_mts_spco_alter_table_analyze_partition_nobinlog, and rebased.

laurynas-biveinis commented 9 months ago

@luqun and others, I have now follow-up changes to InnoDB, which, if force-pushed to this PR, might complicate any in-progress code reviews. Should I wait and make a 2nd PR or a force push would not be a problem?

facebook-github-bot commented 9 months ago

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

luqun commented 9 months ago

As discussed, committed this PR first.

luqun commented 9 months ago

@laurynas-biveinis, looks like this PR cause a lot(or all) rocksdb MRR failure when myrocks as DDSE. such as rocksdb.1st, maybe in ha_innobase::get_se_private_data(), we should also check whether DD is upgrading. if yes, still use dd_properties?

Could you take a look https://github.com/facebook/mysql-5.6/pull/1378? or ?

laurynas-biveinis commented 9 months ago

@laurynas-biveinis, looks like this PR cause a lot(or all) rocksdb MRR failure when myrocks as DDSE. such as rocksdb.1st, maybe in ha_innobase::get_se_private_data(), we should also check whether DD is upgrading. if yes, still use dd_properties?

Could you take a look #1378? or ?

Can you provide more details about the regressions? The DDSE MyRocks tests could not have been running without this PR in the first place