Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add rocksdb as transactional engine #1189

Closed
wangzihuacool opened this issue Oct 20, 2022 · 11 comments
Closed

add rocksdb as transactional engine #1189

wangzihuacool opened this issue Oct 20, 2022 · 11 comments

Comments

@wangzihuacool
Copy link
Contributor

wangzihuacool commented Oct 20, 2022

gh-ost now treats innodb and tokudb as transactional engine.

We now use rocksdb as MySQL's storage engine, for its high compression ratio and write performance. Rocksdb engine also supports transactions and implements transaction concurrency control based on row locks.

I hope gh-ost will treat rocksdb as a transactional engine as well, by adding a case option to IsTransactionalTable() .

Thank you!

@wangzihuacool
Copy link
Contributor Author

@dm-2
Copy link
Contributor

dm-2 commented Oct 21, 2022

👋 @wangzihuacool thanks for the PR! Has this been tested? My main area of concern is whether gh-ost's atomic cut-over still works as expected with RocksDB.

If this all works fine in testing, I'm happy to accept the PR to allow gh-ost to run against RocksDB - but we won't provide any official support for RocksDB as we don't use it here at GitHub currently.

@timvaillancourt
Copy link
Collaborator

timvaillancourt commented Oct 21, 2022

Has this been tested? My main area of concern is whether gh-ost's atomic cut-over still works as expected with RocksDB.

I'm also curious about testing and the potential differences due to no Gap Lock in RocksDB

Are the functional-tests in localtests/ passing on RocksDB and would you be able to assist with adding a RocksDB-enabled MySQL server to the CI jobs that run these tests @wangzihuacool? This would help gain confidence of the compatibility

We are currently testing on a Oracle Community Edition 5.7 and 8.0 server, InnoDB only. I believe RocksDB can be tested using a Percona Server for MySQL server

@wangzihuacool
Copy link
Contributor Author

We are testing RocksDB with gh-ost in our test environment now , and I am glad to add a RocksDB-enabled MySQL server to the ci jobs.

@wangzihuacool
Copy link
Contributor Author

wangzihuacool commented Oct 28, 2022

Has this been tested? My main area of concern is whether gh-ost's atomic cut-over still works as expected with RocksDB.

I'm also curious about testing and the potential differences due to no Gap Lock in RocksDB

Are the functional-tests in localtests/ passing on RocksDB and would you be able to assist with adding a RocksDB-enabled MySQL server to the CI jobs that run these tests @wangzihuacool? This would help gain confidence of the compatibility

We are currently testing on a Oracle Community Edition 5.7 and 8.0 server, InnoDB only. I believe RocksDB can be tested using a Percona Server for MySQL server

I have tested RocksDB using a Percona Server for MySQL 8.0. Most of the functional-tests in localtests/ for release v1.1.5 are passed on RocksDB , except a few features that RocksDB not supportted. To be specific, RocksDB does not support Foreign Key, Generated Columns,GEOMETRY and Gap Locks, refre to MyRocks-limitations.

But tests for master branch are failed, due to the transaction isolation changed to REPEATABLE-READ in #1156. gh-ost can only run in READ_COMMITTED isolation on RocksDB, due to no support for gap locking within RocksDB. @timvaillancourt Can you please change the default isolation to READ_COMMITTED, or make the transaction isolation level user-configurable?

Since we have modified thousands of tables using gh-ost, under READ_COMMITTED isolation level. It suppose to be safe to use gh-ost under READ_COMMITTED isolation level.

@dm-2
Copy link
Contributor

dm-2 commented Nov 14, 2022

@wangzihuacool I think this is going to be a larger piece of work. Would you mind creating a PR with all of the changes needed in a single PR?

We can then review this PR to decide whether to include it in gh-ost - however if the changes are significant then it may be best maintained as a separate fork 😅

The approach that I would prefer for adding RocksDB support would be:

  • add a new storage-engine command line flag (defaults to innodb)
    • when storage-engine is set to rocksdb, this enables any changes necessary to support RocksDB (e.g. sets isolation level to READ_COMMITTED)
    • when storage-engine is set to innodb, gh-ost runs in the same way that it does currently
  • add RocksDB migration testing
    • update existing tests to exclude any that don't work with RocksDB

@wangzihuacool
Copy link
Contributor Author

I'm glad to do that, I'll create a PR with all the changes in a few days.

@wangzihuacool
Copy link
Contributor Author

wangzihuacool commented Nov 25, 2022

I have created a PR #14 in gh-ost-ci-env , adding PerconaServer-8.0.21 tarball so that gh-ost can run tests for RocksDB.

And a PR #1190 with all the changes needed to support RocksDB. Hope you take the time to review it @dm-2 @timvaillancourt

@dm-2
Copy link
Contributor

dm-2 commented Nov 25, 2022

Thanks @wangzihuacool1 I've merged github/gh-ost-ci-env#14 and left a review on #1190 👍

@dm-2
Copy link
Contributor

dm-2 commented Nov 29, 2022

PR merged 🚀

Thanks again for all of your work on this @wangzihuacool! ❤️

@dm-2 dm-2 closed this as completed Nov 29, 2022
@nccx
Copy link

nccx commented Nov 14, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants