Re: [infinispan-dev] Atomic operations and transactions

2011/7/5 Dan Berindei <dan.berindei(a)gmail.com&gt;: > On Tue, Jul 5, 2011 at 12:46 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: >> 2011/7/5 Galder Zamarreño <galder(a)redhat.com&gt;: >>> >>> >>> 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,

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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev