[infinispan-dev] Fix or document? Concurrent replaceWithVersion w/ same value might all return true - ISPN-4972

[Dan Berindei]

On Wed, Nov 26, 2014 at 5:54 PM, Tristan Tarrant <ttarrant at redhat.com> wrote:
> It depends on the side-effects that replacing something with the same
> value has: listeners, cachestores, state transfer, etc.
> In general I'd say: no that's not what I want.

Listeners work with the embedded API, which can only replace based on
values. So I don't think that's a problem.

>
> Tristan
>
> On 26/11/14 16:43, Sanne Grinovero wrote:
>> On 26 November 2014 at 14:17, Dan Berindei <dan.berindei at gmail.com> wrote:
>>> Sanne, it will work as long as the previous value is not the same as
>>> the new value.
>>>
>>> If multiple threads read value 5 with version 5, and all of them want
>>> to replace it with value 6, only one of them will succeed.
>> Ok I see I might be confusing value and versions. I hope :)
>>
>>> But if multiple threads read value 5 with version 5, and want to
>>> replace it with value *5*, all of them might succeed.
>> This paragraph is confusing me more. What "value" are you referring to
>> at the third "5"? Is it even legal to replace an entry with a new
>> value but not incrementing its version?

I see you haven't been reading the HotRod API docs recently :D

The HotRod server is the one who increments the version, the client
can only supply the *expected* version. So "version 5" is the initial
version, "value 5" is the initial value, and the last "value 5" is the
new value.

>>
>> Thanks!
>> Sanne
>>
>>> Indeed, it's not atomic, but a basic counter will work. And it's all
>>> we can do with the actual core cache API (unless we want to go back to
>>> including the HotRod version in the value).
>>>
>>> Cheers
>>> Dan
>>>
>>>
>>> On Wed, Nov 26, 2014 at 2:33 PM, Sanne Grinovero <sanne at infinispan.org> wrote:
>>>> That's not Atomic. How can I implement a counter on this?
>>>>
>>>> Say the current version is 5, I read it, and then issue a "replace 5
>>>> with 6" command.
>>>> If I send a couple of such commands in parallel I need a guarantee
>>>> that only one succeeds, so that the other one can retry and get the
>>>> counter up to 7.
>>>>
>>>> Over Hot Rod I have no locking so I have no alternatives other than
>>>> atomic replacement commands, that's not unlikely to happen: that's a
>>>> critical showstopper for users.
>>>>
>>>> Sanne
>>>>
>>>>
>>>> On 20 November 2014 at 16:35, Dan Berindei <dan.berindei at gmail.com> wrote:
>>>>> I guess you could say this is a regression, this wouldn't have been possible
>>>>> when the version was part of the value :)
>>>>>
>>>>> But I agree an application is very unlikely call replaceWithVersion with the
>>>>> same value as before, so +1 to document it for now and implement
>>>>> replaceWithVersion/replaceWithPredicate in the embedded cache for 8.0.
>>>>>
>>>>> Cheers
>>>>> Dan
>>>>>
>>>>>
>>>>> On Thu, Nov 13, 2014 at 3:08 PM, Radim Vansa <rvansa at redhat.com> wrote:
>>>>>> I agree with Galder, fixing it is not worth the cost.
>>>>>>
>>>>>> Actually, there are often bugs that I'd call rather 'quirks', not
>>>>>> honoring the ConcurrentMap contract (recently we have discussed with Dan
>>>>>> [1] and [2]) which are quite complex to fix. Another one that's
>>>>>> considered not a bug is that a read does not have transactional semantics.
>>>>>> Galder, where will you document that? I think that special page in
>>>>>> documentation should accumulate such cases, linked to JIRAs for case
>>>>>> that eventually we'll resolve them (with that glorious MVCC). And of
>>>>>> course, link from javadoc to this document (though I am not sure whether
>>>>>> we can correctly keep that in sync with latest release. Could we have a
>>>>>> redirection from http://infinispan.org/docs/latest to
>>>>>> http://infinispan.org/docs/7.0.x/ ?
>>>>>>
>>>>>> Radim
>>>>>>
>>>>>> [1] https://issues.jboss.org/browse/ISPN-3918
>>>>>> [2] https://issues.jboss.org/browse/ISPN-4286
>>>>>>
>>>>>> On 11/13/2014 01:51 PM, Galder Zamarreño wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Re: https://issues.jboss.org/browse/ISPN-4972
>>>>>>>
>>>>>>> Embedded cache provides atomicity of a replace() call passing in the
>>>>>>> previous value. This limitation might be lifted when we adopt Java 8 and we
>>>>>>> can pass in a lambda or similar, which can be executed right when the value
>>>>>>> is compared now, and if it returns true it’s applied. The lambda could
>>>>>>> compare both value and metadata for example.
>>>>>>>
>>>>>>> Anyway, given the current status, I’m considering whether it’s worth
>>>>>>> fixing this particular issue. Fixing the issue would require adding some
>>>>>>> kind of locking in the Hot Rod server so that the version retrieval,
>>>>>>> comparison and replace call, can all happen atomically.
>>>>>>>
>>>>>>> This is not ideal, and on top of that, as Radim said, the chances of
>>>>>>> this happening in real life are limited, or more precisely it’s effects are
>>>>>>> minimal. In other words, if two concurrent threads call replace with the
>>>>>>> same value, the end result is that the new value would be stored, but as a
>>>>>>> result of the code, both replaces would return true which is not strictly
>>>>>>> right.
>>>>>>>
>>>>>>> I’d rather document this than add unnecessary locking in the Hot Rod
>>>>>>> server where it deals with the versioned replace call.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>> --
>>>>>>> Galder Zamarreño
>>>>>>> galder at redhat.com
>>>>>>> twitter.com/galderz
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> infinispan-dev mailing list
>>>>>>> infinispan-dev at lists.jboss.org
>>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>>
>>>>>> --
>>>>>> Radim Vansa <rvansa at redhat.com>
>>>>>> JBoss DataGrid QA
>>>>>>
>>>>>> _______________________________________________
>>>>>> infinispan-dev mailing list
>>>>>> infinispan-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> infinispan-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> infinispan-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> --
> Tristan Tarrant
> Infinispan Lead
> JBoss, a division of Red Hat
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev