-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Consistent reads #432
Merged
Merged
Consistent reads #432
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
…nlock inside the hashtable
…logic in transaction_lock_for_write
…initialization and resize
…re read transactions which in turn require a worker context, the worker context needs to be set for the tests to run properly
…nd transaction_upgrade_lock_for_write
Codecov ReportAttention:
... and 2 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
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.
This PR implements a mechanism to guarantee consistent reads adding support for read-only transactions and leveraging this new transaction type to synchronize the read and write operations.
A major advantage of this approach is that meanwhile before commands like MSET or MSETNX would have allowed a client to read a value of a sequence being set, breaking the ABI with the Redis interface (where MSET, MSETNX and all the other commands that operate on the same set of keys are atomic), now the new mechanism implemented directly into the transaction logic and leveraged internally by the hashtable, ensures that the reads will wait any write transaction in progress.
Meanwhile this is causing a performance penalty, the major advantage is that now cachegrand is much more ACID compliant (infact when used with the on-disk storage cachegrand is fully ACID compliant) providing transaction isolation.
Another major advantage is that now a lot of complexity implemented to allow reads with writes in progress can be dropped, which will lead to (a) an esamplification of the code and (b) some performance improvements.
These changes have room for improvement, the locks now might last for several seconds, depending on the type of operation being carried out, and there is no reason for a fiber to wait spinning, it makes much more sense to yield after some spinning to let other fibers to carry out their own operations.
Also, so far all the operation that required access to multiple keys were forced to use read-write transaction but for several it's not really required and they can switch to read-only transaction.
Because there is (plenty) of room for improvement, no benchmark is included to compare the performances.