#502 - Concurrency Usage stats update and custom CAS in one transaction support

[anandrgitnirman]

Update usage stats in concurrent requests Support for custon CAS operations in concurrency

[coveralls]

Coverage decreased (-3.4%) to 64.243% when pulling 6e7af86176a63ad315458c281bacf4b27cabf7f8 on anandrgitnirman:master into 1a5298472c615f6b155317c7f540427b20360931 on singnet:master.

[vsbogd]

@anandrgitnirman , I have thought about API from the PR and two things bother me:

• presence of Version in KeyValueData means that API exposes thing which is specific for ETCD implementation only. Version doesn't exist in MemoryStorage for example and one who will try implement this API for memory storage will face this issue.
• AdditionalParameters in CASRequest actually are required by caller implementation only and should stay on caller side and not to be exposed via API.

In fact getting rid of this two things required me to rethink an API conception. The best option I see is to divide transaction on two calls. StartTransaction call to get current state of the database and allow storage keep all information it needs to guarantee that this state is not changed. CompleteTransaction (not very correct name btw, may be startCAS and endCAS would be better) which checks that state was not changed and applies update. Thus in between of these calls client can do any logic it wants assuming conditionValues are not changed during transaction.

The whole solution is different from current one so I implemented it in own branch. You can look at results at https://github.com/anandrgitnirman/snet-daemon/pull/43. I didn't implemented tests and tried to keep as large part of original code as I can. But I also implemented few of my comments in this PR.

Please take a look and let me know what do you think. Probably it will be simpler to look in IDE not GitHub as change is large enough.

[vsbogd]

Here is another option to merge both StartTransaction and CompleteTransaction methods into one, lets say ExecuteTransaction with the following signature:

ExecuteTransaction(transaction Transaction) (ok bool, err error)

type Transaction struct {
  ConditionKeys []string
  UpdateFunc func (conditionValues []*KeyValueData) (update []*KeyValueData, err error)
  RetryTillSuccessOrError bool
}

May be this API is preferable because allows implementing indefinite loop of calling CompleteTransaction in storage implementation itself.

[anandrgitnirman]

Here is another option to merge both StartTransaction and CompleteTransaction methods into one, lets say ExecuteTransaction with the following signature:

ExecuteTransaction(transaction Transaction) (ok bool, err error)

type Transaction struct {
  ConditionKeys []string
  UpdateFunc func (conditionValues []*KeyValueData) (update []*KeyValueData, err error)
  RetryTillSuccessOrError bool
}

May be this API is preferable because allows implementing indefinite loop of calling CompleteTransaction in storage implementation itself.

Thanks @vsbogd , will cover the test cases and I like the sugeestion above , will try to wrap the core API we have built later today or by first thing tomorrow morning, this will pave the way for standrdizing the updates we do on the storage and also keeping it agnostic to the type of storage ( we should be easily move to a different one) and hence the abstraction is even more important than just a working version :)

[vsbogd]

@anandrgitnirman thank you for taking care of this. BTW the simplest way to test the code I think is implement PutIfAbsent and CompareAndSwap via transaction API and then TestEtcdCAS and TestEtcdNilValue tests should stay green.

[anandrgitnirman]

hi @vsbogd , thanks for the approval , however I would like to bring in some more test cases , will get to them over the weekend , will merge after we have increased the coverage Thanks again for all the help