-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
api: nested txns #8102
api: nested txns #8102
Conversation
dea3a03
to
950e33e
Compare
for k := range putsElse { | ||
if _, ok := puts[k]; ok { | ||
// if key is from putsThen, overlap is OK since | ||
// either then/else are mutually exclusive |
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.
So this means we reject
kv.Txn(context.TODO()).
If(clientv3.Compare(clientv3.Version("foo"), "=", 0)).
Then(
clientv3.OpPut("foo", "bar"),
clientv3.OpTxn(nil, []clientv3.Op{clientv3.OpPut("abc", "123")}, nil)).
Else(clientv3.OpPut("hello", "baz")).Commit()
Because "hello"
from putsElse
is not in putsThen
?
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.
Oh it's comparing within children
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.
if all the keys are unique, it shouldn't reject
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.
Ok this makes sense now. Thanks.
etcdserver/api/v3rpc/key.go
Outdated
// sets for recursive evaluation. | ||
func checkIntervals(reqs []*pb.RequestOp) (map[string]struct{}, adt.IntervalTree, error) { | ||
var dels adt.IntervalTree | ||
puts := make(map[string]struct{}) |
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.
Can we godoc this or move this before for _, req := range reqs {
?
for i := range reqs { | ||
resps[i] = a.applyUnion(txn, reqs[i]) | ||
} | ||
a.applyTxn(txn, rt, txnPath, txnResp) |
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.
Are we going to use the counts from applyTxn
and newTxnResp
later? I don't see them used in this PR?
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.
@gyuho the counts are used internally to track progress through txnPath
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.
@heyitsanthony Yeah I see them now. Nvm.
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.
lgtm. thanks!
950e33e
to
b10ea20
Compare
if !ok || tv.RequestTxn == nil { | ||
continue | ||
} | ||
txnPath = append(txnPath, compareToPath(rv, tv.RequestTxn)...) |
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.
What if the nested transaction's conditions become different after applying previous ops?
This code implements all check before any mutation but it's possible a nested compare happens after some previous mutation and the conditions result changed.
Fixes #7857