Prevent accessing freed XA-committed RocksDB transactions #1513
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With MyRocks DDSE, a XA transaction followed by a DDL statement in the same connection results in the following: - The XA transaction in RocksDB is committed by rocksdb_commit_by_xid, which then deletes the RocksDB transaction object. - The DDL statement in the same thread accesses the deleted object. - If it does not crash first, the next RocksDB commit on that transaction object fails due to the transaction being in unexpected state (COMMITTED instead of STARTED). Fix by: - No longer deleting transactions in rocksdb_commit_by_xid and rocksdb_rollback_by_xid, which was suitable only for crash recovery. - Prevent memory leaks in crash recovery by making the recovered transaction pointer vector in rocksdb_recover public, and deleting all the transactions there in rocksdb_post_recover. - New method Rdb_transaction::restart_if_committed_by_xid with implementation in Rdb_transaction_impl that syncs the MyRocks transaction state with RocksDB by restarting the RocksDB transaction if needed.
|
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With MyRocks DDSE, a XA transaction followed by a DDL statement in the same
connection results in the following:
then deletes the RocksDB transaction object.
fails due to the transaction being in unexpected state (COMMITTED instead of
STARTED).
Fix by:
rocksdb_rollback_by_xid, which was suitable only for crash recovery.
pointer vector in rocksdb_recover public, and deleting all the transactions
there in rocksdb_post_recover.
Rdb_transaction_impl that syncs the MyRocks transaction state with RocksDB by
restarting the RocksDB transaction if needed.