-
Notifications
You must be signed in to change notification settings - Fork 142
CORTX-33652: Add min_success parameter for index operations #1991
Conversation
|
Thanks for your contribution in opening this pull request! Now you can be rewarded with a CORTX sticker by requesting cortx sticker |
nikitadanilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed elsewhere it makes sense to rename "quorum" to something like "min_replicas" throughout to avoid confusion.
|
References to "quorum" have been replaced with "min_replicas" |
|
Thanks for your contribution! |
|
Thanks for your contribution! |
sergey-shilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please reword the commit message since we decided to get rid of "quorum" word and use something like "min_success" or "minimum number of successful CAS ops" and so on.
|
This issue/pull request has been marked as |
|
@trshaffer : I think following ST failures are related to this change, |
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Signed-off-by: Tim Shaffer <[email protected]>
Jenkins CI Result : Motr#1640Motr Test Summary
CppCheck SummaryCppcheck: No new warnings found 👍 |
Jenkins CI Result : Motr#1649Motr Test Summary
CppCheck SummaryCppcheck: No new warnings found 👍 |
Jenkins CI Result : Motr#1650Motr Test Summary
CppCheck SummaryCppcheck: No new warnings found 👍 |
Signed-off-by: Tim Shaffer <[email protected]>
Jenkins CI Result : Motr#1662Motr Test Summary
CppCheck SummaryCppcheck: No new warnings found 👍 |
|
retest this please |
|
Since hare and motr job had failed, I retriggered it manually, run 436 waiting for results. |
|
Thanks for your contribution to CORTX! 🎉 |

Problem Statement
This PR adds a per-operation setting for the minimum number of successful CAS operations for distributed index operations. Initially, all CAS operations were required to succeed for the operation as a whole to succeed. A previous commit (f2ba0d1) changed the behavior to reduce IO errors during degraded mode. Currently, a single successful operation is sufficient to consider the parent operation successful. This can lead to consistency issues, however. In the read-after-write case, stale data may be returned from successful index operations if the child operations succeed on disjoint sets of CAS services.
Design
This PR changes adds a
setsockopt-like function (m0_idx_op_setoption) for tuning parameters of index operations. The only option initially available isM0_DIX_MIN_REPLICA_QUORUM. This defaults to (N+K)/2 + 1 to prevent the situation above (disjoint sets of CAS services). Clients may choose a more lax requirement if they care more about availability than consistency. Clients (RGW) may want to add configuration options for controlling this quorum requirement, or could use this functionality as part of S3 storage tiers, e.g. setting a bucket to use reduced consistency operations. Note that ensuring correctness will also require a mechanism to compare versions of replies from CAS services, which is not in the scope of this PR.Coding
Checklist for Author
Testing
Checklist for Author
Impact Analysis
Checklist for Author/Reviewer/GateKeeper
Review Checklist
Checklist for Author
Documentation
Checklist for Author