search

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

Always assume existence of persisted autoincrement values for MyRocks… #1329

Closed laurynas-biveinis closed 11 months ago

laurynas-biveinis commented 1 year ago

… table

The non-persistent autoincrement values upgrade to persistent ones was implemented in 2017, in 5.6. Thus no 8.0 instance should encounter non-persistent values, ever. If this assumption is incorrect, the upgrade is still possible by going through the latest 5.6 release first.

Thus remove myrocks_autoinc_upgrade debug code injection point, simplify the surrounding code, remove the tests that use it.

facebook-github-bot commented 1 year ago

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

facebook-github-bot commented 11 months ago

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

luqun commented 11 months ago

LGTM.. will commit soon.

luqun commented 11 months ago

Hi @laurynas-biveinis , got one code review from @lth: "in ha_rocksdb::load_auto_incr_value(), in case if we couldn't find data in DD(in case corrupt, or ..), use index_last() to fetch value." . I update the code with this and submitted.

laurynas-biveinis commented 11 months ago

Hi @laurynas-biveinis , got one code review from @lth: "in ha_rocksdb::load_auto_incr_value(), in case if we couldn't find data in DD(in case corrupt, or ..), use index_last() to fetch value." . I update the code with this and submitted.

This made the very title of this PR invalid, please keep me in loop next time :). It effectively changes the index_last from upgrade scenario to corruption scenario instead of removing it altogether.