Closed laurynas-biveinis closed 1 year ago
Please review only the last commit of the three, the first two will disappear once their branch is merged
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@laurynas-biveinis , could you rebase this PR? it will easy to do testing after rebase.
@laurynas-biveinis , could you rebase this PR? it will easy to do testing after rebase.
Will do
@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.
@luqun Rebased
Found a minor issue after rebasing the whole prototype branch
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.
Ready for review. It is independent from https://github.com/facebook/mysql-5.6/pull/1323, although trivial text conflicts might emerge between the two.
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.
Updated & rebased
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
The storage engine requirement for supporting attachable transaction is being able to handle vanished THD::ha_data field in the middle of transaction. The server layer swaps its existing data with a nullptr before calling the SE, and expects the SE to allocate a new transaction object and to invalidate any cached info depending on the previous one. Once the server is done with the attachable transaction, it swaps back the original SE data pointer.
MyRocks already happens to work correctly in this case, so the main change is to advertise the attachable transaction support by adding HA_ATTACHABLE_TRX_COMPATIBLE to ha_rocksdb::table_flags return value.
Since the attachable transactions are AC-NL-RO-RC (AutoCommit-NonLocking-ReadOnly-ReadCommitted) at this point, the rest of code patch introduces invariants asserting these properties for Rdb_transcation and child class objects and minor changes such as not setting lock timeouts as no locks can be taken. Specifically, such transaction is indicated by m_max_row_locks == 0. This is decided in set_params which now handles both regular and attachable transactions.
Make m_ddl_transaction a private field with getters and setters so that the invariants could be checked.
Factor out is_autocommit and get_ha_data_or_null helper functions.
At the same time make cleanups in the touched code:
[[nodiscard]]
.For testing, convert the existing InnoDB attachable_trx.test to an include file that is shared by both engine tests.
At a later point the attachable transactions may become read-write (if i.e. mysql.gtid_executed is converted to MyRocks) and read-uncommitted, if this isolation level will be implemented for the DDSE. In this case the invariant will have to be split.
Reference: