Spencerkimball/inconsistent reads#450
Conversation
e1c6318 to
e65cf68
Compare
kv/dist_sender_server_test.go
Outdated
There was a problem hiding this comment.
Comment appears to be outdated.
|
LGTM |
e65cf68 to
fe23ef4
Compare
|
Might want to take another look... |
There was a problem hiding this comment.
Should we remove the CONSENSUS option until it's implemented? (Do we even need it? When would someone choose one or the other?)
There was a problem hiding this comment.
To pass Jepsen tests which mess with clocks to foil read leases. I'd like to just leave it in as a placeholder.
There was a problem hiding this comment.
But is this ever a choice you'd want to make on a per-operation basis? Either you trust your clocks and choose between CONSISTENT and INCONSISTENT, or you don't and choose between CONSENSUS and INCONSISTENT. The choice between CONSISTENT and CONSENSUS seems to be best made at the level of the deployment instead of the operation.
888eabb to
c195cbf
Compare
There was a problem hiding this comment.
Why create an error variable here?
There was a problem hiding this comment.
Pete's code. Unrelated to my change.
|
LGTM |
MVCCGet and MVCCScan now take a "consistent" boolean which, if false, ignores intents. Moved the guts of MVCCScan into MVCCIterate and MVCCScan just calls into it. This change allows the removal of the MVCCIterateCommitted method. Provides a unittest for "deadloop" issue of encountering an extant intent at a meta indexing record. Fixes #343 Fixes #435
If read consistency isn't required, a timestamp is assigned using the local clock. This prevents cases where a different timestamp is used for each part of a multi-range op, each assigned to the respective node's clock at the time the RPC is received. Also, if read consistency isn't required, don't insist that ops spanning ranges be auto-wrapped in a transaction. Fixed some errors in dist_sender.go with error handling. In one case the error wasn't being cleared on the retry loop. In another, the response error wasn't being examined after calling the RPC.
Address pull request feedback and add new unittests for command queue / timestamp cache skipping functionality.
c195cbf to
2bae48c
Compare
…reads Spencerkimball/inconsistent reads
MVCCGet and MVCCScan now take a "consistent" boolean which, if false,
ignores intents. Moved the guts of MVCCScan into MVCCIterate and MVCCScan
just calls into it. This change allows the removal of the MVCCIterateCommitted
method.
Provides a unittest for "deadloop" issue of encountering an extant intent
at a meta indexing record.
Fixes #343
Fixes #435