basho / riak-erlang-client

The Riak client for Erlang.
Apache License 2.0
311 stars 189 forks source link

Mas i1847 conditionalput (#410) #411

Closed martinsumner closed 1 year ago

martinsumner commented 1 year ago

Tries to mirror vtag as defined by the riak HTTP API. The vtag is the vtag on a singleton metadata object, otherwise it is an bas62 encoded version of the vector clock

aramallo commented 1 year ago

Hi @martinsumner this is great! I guess this doesn't mean that we can use ETAGs to do conditional GETs/PUTs via riak_pb_socket client, as I do not see support for passing the ETAG in the options, right?

I have not check the Riak KV code but is this something supported by the server and never exposed via the riak_pb_socket client API or would we need to implement the support on the RIAK KV PB socket side too?

aramallo commented 1 year ago

Oh sorry this was already working with get option {if_modified, VClock} right?

martinsumner commented 1 year ago

Have a walk through of the issue and the test.

This is a bit of a hack to provide the functionality to the HTTP client so that it can use the standard HTTP if_match, if_none_match headers on PUTs. The matching is based on vtags, so something to generate the vtags client side was required (although this has some imperfections, and probably shouldn't be used this way).

There was already the support for if_not_modified option on PUTs via the PB API, which has now been added to the HTTP API using the X-Riak-If-Not-Modified header. This is based on vector clocks, not vtags, so is really what should be used in both APIs for conditional PUTs. So now if_not_modified works in both clients not just PB.

It is a weak condition - relying on a GET before a PUT - the object could still be modified in a race with a parallel update.

I haven't looked at using the headers on GETs in either client. The actual PB functionality is unchanged, I've only modified behaviour on the HTTP side.

martinsumner commented 1 year ago

Just had a quick peek, and yes, it looks like there is already if_modified on GET, as well as if_not_modified on PUT as the options for making requests conditional on the vector clock.

Note that other than saving on the round-trip of the object body back to the client, there is no special efficiency in these processes.

For the GET - it still does the GET in full under the hood - it doesn't do anything smart to avoid lifting the object off disk. Likewise with the conditional PUT, it is just doing a full GET before the PUT and aborting the PUT if the GET reveals a modification of the clock.

aramallo commented 1 year ago

@martinsumner Thanks. I got mixed up, cause I was thinking about the CRDT PB API, which does not support any of those. I will have a peek at the CRDT backend side to see if we can implement something similar.