api: txn with success/fail op list per key

[Sheph]

Hi.

I'm currently using etcd transactions and need the following functionality:
suppose I have 3 keys: a, b and c. I'm a client and I'm watching those keys and I've received latest values and mod revisions, so suppose a=1,rev=3; b=2,rev=5; c=3,rev=2
now I want to change all 3 values atomically, but only if those values are still "fresh", i.e. I want a transaction such as:

if (rev(a) == 3) { a = 4; } else {}
if (rev(b) == 5) { b = 5; } else {}
if (rev(c) == 2) { c = 6; } else {}

from etcd code I see that transaction processing is done like this:

checkRequests(txn, rt, txnPath, a.checkRange);
...
func newTxnResp(rt *pb.TxnRequest, txnPath []bool) (txnResp *pb.TxnResponse, txnCount int) {
	reqs := rt.Success
	if !txnPath[0] {
		reqs = rt.Failure
	}
	resps := make([]*pb.ResponseOp, len(reqs))
	txnResp = &pb.TxnResponse{
		Responses: resps,
		Succeeded: txnPath[0],
		Header:    &pb.ResponseHeader{},
	}
	for i, req := range reqs {
                ...
	}
	return txnResp, txnCount
}

so it looks like it's not very hard to implement what I want, I just need to check conditions per key and compose txnResp from different ops in success and failure lists depending on comparison outcome. I would like to know if it's the right thing to do, like, is it possible that it'll break something ? And if this approach is ok, is there any interest in pull request from your side ?

Thanks.

[jingyih]

Just trying to clarify, is the following code equivalent to your use case:

if (rev(a) == 3 AND rev(b) == 5 AND rev(c) == 2) {
  a = 4; b = 5; c = 6;
} else {
  // do nothing
}

From the function signature, this should be already supported by current implementation. Can you try to pass all the conditions to If().

etcd/clientv3/txn.go

Lines 38 to 41 in a621d80

	// If takes a list of comparison. If all comparisons passed in succeed,
	// the operations passed into Then() will be executed. Or the operations
	// passed into Else() will be executed.
	If(cs ...Cmp) Txn

[Sheph]

Actually, I need something different, i.e. instead of

if (rev(a) == 3 AND rev(b) == 5 AND rev(c) == 2) {
  a = 4; b = 5; c = 6;
} else {
  // do nothing
}

which is how it's currently working, I need:

if (rev(a) == 3) { a = 4; }
if (rev(b) == 5) { b = 5; }
if (rev(c) == 2) { c = 6; }

e.g. let's suppose that rev(a) == 3 and rev(b) == 5, but rev(c) is 3, not 2. the former case will
fail and nothing will be updated, but latter case will update a and b, but not c.

[jingyih]

From the pseudo code you provided, it looks like 3 separate transactions. But I am aware that you also stated you want the updates of all three keys to be atomic. At the moment, it is not clear to me how this fits into the currently provided transaction semantics. Hopefully someone more familiar with etcd transaction implementation could jump in and help.

[Sheph]

It looks like currently provided transactions can't satisfy me requirements, but the server code that I've mentioned above looks like the right place to make the change. So I'm planning to make such custom transaction type, I'm just wondering if it's possible to make it so that it can be merged upstream ? i.e. I really don't want to end up with custom patched etcd daemon that I'll have to fix with every etcd release

[jingyih]

My personally opinion is that adding feature to upstream is preferred, as long as it does not break the current Txn semantics.

The code you pasted is just pre allocating the response for Txn, not where the Txn is applied based on the 'IF' conditions.

For your reference:
The compare logic is here:

etcd/etcdserver/apply.go

Line 427 in c7c6894

func applyCompares(rv mvcc.ReadView, cmps []*pb.Compare) bool {

The Txn is applied here:

etcd/etcdserver/apply.go

Line 508 in c7c6894

func (a *applierV3backend) applyTxn(txn mvcc.TxnWrite, rt *pb.TxnRequest, txnPath []bool, tresp *pb.TxnResponse) (txns int) {

It is not obvious to me how to implement your Txn semantics while preserving backward compatibility.

[Sheph]

Yes, but as in preallocation code it does:

reqs := rt.Success
if !txnPath[0] {
	reqs = rt.Failure
}

and then just walks the reqs list and applies (correct me if I'm wrong, I'm not very familiar with etcd source code), so it looks like we just need to compose the reqs list based on conditions instead of taking one of the lists. What is not clear to me is how to fit this in current transaction model, like the easiest solution is just making a new type of transaction, i.e. new type of request, but the right solution is probably modifying current transaction request, preserving back compatibility. It's just that there may be more transaction logic extensions, i.e. what if someone will want "OR" instead of "AND" compare logic, what if someone would like to make crazy things such as "transaction should fail if at least N compares fail", etc. It's starting to look like there's a need for transaction language of some sort...

[Sheph]

I'm writing this just in case if someone else will need such functionality, it turned out etcd supports nested transactions: #7857
So using nested transactions it's possible to implement such logic