On 5 Jul 2011, at 11:39, Sanne Grinovero wrote:
> 2011/7/5 Dan Berindei <dan.berin...@gmail.com>:
>> On Tue, Jul 5, 2011 at 12:46 PM, Sanne Grinovero <sa...@infinispan.org>
>> wrote:
>>> 2011/7/5 Galder Zamarreño <gal...@redhat.com>:
>>>>
>>>>
>>>> On Jul 4, 2011, at 11:25 AM, Sanne Grinovero wrote:
>>>>
>>>>> I agree they don't make sense, but only in the sense of exposed API
>>>>> during a transaction: some time ago I admit I was expecting them to
>>>>> just work: the API is there, nice public methods in the public
>>>>> interface with javadocs explaining that that was exactly what I was
>>>>> looking for, no warnings, no failures. Even worse, all works fine when
>>>>> running a local test because how the locks currently work they are
>>>>> acquired locally first, so unless you're running such a test in DIST
>>>>> mode, and happen to be *not* the owner of the being tested key, people
>>>>> won't even notice that this is not supported.
>>>>>
>>>>> Still being able to use them is very important, also in combination
>>>>> with transactions: I might be running blocks of transactional code
>>>>> (like a CRUD operation via OGM) and still require to advance a
>>>>> sequence for primary key generation. This needs to be an atomic
>>>>> operation, and I should really not forget to suspend the transaction.
>>>>
>>>> Fair point. At first glance, the best way to deal with this is suspending
>>>> the tx cos that guarantees the API contract while not forcing locks to be
>>>> acquired for too long.
>>>>
>>>> I'd advice though that whoever works on this though needs to go over
>>>> existing use cases and see if the end result could differ somehow if this
>>>> change gets applied. If any divergences are found and are to be expected,
>>>> these need to be thoroughly documented.
>>>>
>>>> I've gone through some cases and end results would not differ at first
>>>> glance if the atomic ops suspend the txs. The only thing that would change
>>>> would be the expectations of lock acquisition timeouts by atomic ops
>>>> within txs.
>>>>
>>>> For example:
>>>>
>>>> Cache contains: k1=galder
>>>>
>>>> 1. Tx1 does a cache.replace(k1, "galder", "sanne") -> suspends tx and
>>>> applies change -> k1=sanne now
>>>> 2. Tx2 does a cache.replace(k1, "galder", "manik") -> suspends tx and is
>>>> not able to apply change
>>>> 3. Tx2 commits
>>>> 4. Tx1 commits
>>>> End result: k1=sanne
>>>
>>> Right.
>>> To clarify, this is what would happen with the current implementation:
>>>
>>> 1. Tx2 does a cache.get(k1) -> it reads the value of k1, and is
>>> returned "galder"
>>> 2. Tx1 does a cache.replace(k1, "galder", "sanne") -> k1="sanne" in
>>> the scope of this transaction, but not seen by other tx
>>> 3. Tx2 does a cache.replace(k1, "galder", "manik") -> k1="manik" is
>>> assigned, as because of repeatable read we're still seeing "galder"
>>> 4. Tx2 & Tx1 commit
>>>
>>> ..and the end result depends on who commits first.
>>>
>>>>
>>>> 1. Tx1 does a cache.replace(k1, "galder", "sanne") -> acquires lock
>>>> 2. Tx2 does a cache.replace(k1, "galder", "manik") -> waits for lock
>>>> 3. Tx2 rollback -> times out acquiring lock
>>>> 4. Tx1 commits -> applies change
>>>> End result: k1=sanne
>>>
>>> I'm not sure we're on the same line here. 1) should apply the
>>> operation right away, so even if it might very briefly have to acquire
>>> a lock on it, it's immediately released (not at the end of the
>>> transaction), so why would TX2 have to wait for it to the point it
>>> needs to rollback?
>>>
>>
>> I think it would make sense to make atomic operations pessimistic by
>> default, so they would behave like in Galder's example.
>>
>> Then if you wanted to reduce contention you could suspend/resume the
>> transaction around your atomic operations and make them behave like
>> you're expecting them to.
>>
>> Here is a contrived example:
>>
>> 1. Start tx Tx1
>> 2. cache.get("k") -> "v0"
>> 3. cache.replace("k", "v0", "v1")
>> 4. gache.get("k") -> ??
>>
>> With repeatable read and suspend/resume around atomic operations, I
>> believe operation 4 would return "v0", and that would be very
>> surprising for a new user.
>> So I'd rather require explicit suspend/resume calls to make sure
>> anyone who uses atomic operations in a transaction understands what
>> results he's going to get.
>
> The problem is that as a use case it makes no sense to use an atomic
> operation without evaluating the return value.
> so 3) should actually read like
> `
> 3. boolean done = cache.replace("k", "v0", "v1")
> and based on this value, the application would branch in some way, and
> so acquiring locks and waiting for each other is not enough, we can
> only support this if write skew checks are enabled, and mandate the
> full operation to rollback in the end. That might be one option, but I
> really don't like to make it likely to rollback transactions,
then you can suspend the transaction and run the atomic operation out of tx's
scope.
> I'd
> prefer to have an alternative like a new flag which enforces a "fresh
> read" skipping the repeatable read guarantees. Of course this wouldn't
> work if we're not actually sending the operations to the key owners,
> so suspending the transaction is a much nicer approach from the user
> perspective. Though I agree this behaviour should be selectable.
>
> Cheers,
> Sanne
>
>>
>> Dan
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
