Some of the API methods' return values may not be as expected when using DIST. Just wanted to run this by you guys to see what you thought. Specifically, what I am referring to is methods like put(), putIfAbsent(), and the 2 versions of remove() and replace(). Now when these are executed in any other cache mode (LOCAL, REPL, INVAL) return values are as expected from their java.util.Map contracts. However with DIST, there are 2 separate ways this could go. 1. The key in question is mapped to the local cache on which the method is invoked. This case behaves normally as well as per the Map contract. 2. The key in question is *not* mapped to the local cache. E.g., K is mapped to nodes A and B, and on C we do C.remove(). I can stick with the map contract if each of these methods first does a remote lookup of the key and stores this in L1 (or even in the invocation context and then throw it away if there is no L1) but this is unnecessarily wasteful IMO. The other approach is to realise the command pertains to a key for which the local cache is not the owner, and replicate the call to the remote nodes. What happens next is also a matter of tuning: we could a) replicate + ignore responses. Then the retval to these methods are indeterminate, and probably unusable. b) replicate + wait for response. This will adhere to the Map contract, albeit at a cost. Probably not possible if we use DIST_ASYNC. c) L1: we could either "correct" the L1 cache with the change, or remove the key forcing another lookup for the next time around. What do people think? Maybe offer all strategies? Or will this then become configuration hell? :) Cheers -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

Ok, after discussing this on IRC and the clustering conf call, here is what we will do: 1. Non-transactional * Only support DIST in SYNC mode. Throw a configuration exception if DIST and ASYNC are detected. * Allow DIST_ASYNC if an unsafe flag is set to true (breakApi=true?) in the config, so that the user is explicitly aware that API return values will be unreliable. * Make sure this is documented in the Javadocs as well as the FAQs 2. Transactional - Async * As above, do not support unless breakApi = true. In which case this is simple - no return values, only broadcast modifications in prepare as we do with REPL_SYNC. 3. Transactional - Sync * Offer 2 options here. Option 1 is simple and we already have this. Option 2 will need to be written. Option 1: break API. This will behave the same as async, in that we don't care about return values, but we still use a 2-phase commit protocol for the transaction boundary. Users will still need to specify breakApi = true so this is explicit. Option 2: Before any write RPC call is broadcast, a remote get is performed. This pulls back the necessary values so that a reliable return value is available. Cheers Manik On 17 Apr 2009, at 12:18, Manik Surtani wrote: > This gets even more interesting when we have transactions in play. > E.g.: > > K resides on {A, B} in a cluster of {A, B, C} > > On C, we do: > > 1. tx.begin() > 2. retval = C.replace(K, V, V2) > 3. // do something with retval > 4. tx.commit() > > It is one thing to set the expectation of unreliable return values > for async mode as defined below, but the above would be tricky even > in the case of sync mode. On line 2 we would have to start the tx > on remote nodes and start processing the commands and pull back > results. And on 4, the prepare logic would have to change to > assume that all writes have already been executed. > > On the plus side, this means we have txs that will fail fast(er) if > remote locks cannot be be acquired... > > Thoughts and comments? > > - Manik > > On 16 Apr 2009, at 14:33, Manik Surtani wrote: > >> Some of the API methods' return values may not be as expected when >> using DIST. Just wanted to run this by you guys to see what you >> thought. >> >> Specifically, what I am referring to is methods like put(), >> putIfAbsent(), and the 2 versions of remove() and replace(). >> >> Now when these are executed in any other cache mode (LOCAL, REPL, >> INVAL) return values are as expected from their java.util.Map >> contracts. >> >> However with DIST, there are 2 separate ways this could go. >> >> 1. The key in question is mapped to the local cache on which the >> method is invoked. >> >> This case behaves normally as well as per the Map contract. >> >> 2. The key in question is *not* mapped to the local cache. E.g., >> K is mapped to nodes A and B, and on C we do C.remove(). >> >> I can stick with the map contract if each of these methods first >> does a remote lookup of the key and stores this in L1 (or even in >> the invocation context and then throw it away if there is no L1) >> but this is unnecessarily wasteful IMO. >> >> The other approach is to realise the command pertains to a key for >> which the local cache is not the owner, and replicate the call to >> the remote nodes. >> >> What happens next is also a matter of tuning: we could >> >> a) replicate + ignore responses. Then the retval to these methods >> are indeterminate, and probably unusable. >> b) replicate + wait for response. This will adhere to the Map >> contract, albeit at a cost. Probably not possible if we use >> DIST_ASYNC. >> c) L1: we could either "correct" the L1 cache with the change, or >> remove the key forcing another lookup for the next time around. >> >> What do people think? Maybe offer all strategies? Or will this >> then become configuration hell? :) >> >> Cheers >> -- >> Manik Surtani >> manik(a)jboss.org >> Lead, Infinispan >> Lead, JBoss Cache >> http://www.infinispan.org >> http://www.jbosscache.org >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Manik Surtani > manik(a)jboss.org > Lead, Infinispan > Lead, JBoss Cache > http://www.infinispan.org > http://www.jbosscache.org > > > > -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

Can't we reuse cacheMode (REPL_SYNC, REPL_ASYNC) ?

Manik Surtani wrote: > And in terms of configuration - this is how I see it looking: > > <!-- This will allow you to use ASYNC with DIST --> > <!-- If used with SYNC + DIST, it will optimise away the remote get > before a write --> > <unsafe unreliableReturnValues="true" /> > > <cluster mode="dist"> > <async /> > </cluster> > > Reckon this is clear and intuitive enough for users? > > Cheers > Manik > > > On 21 Apr 2009, at 11:41, Manik Surtani wrote: > >> Ok, after discussing this on IRC and the clustering conf call, >> here is what we will do: >> >> 1. Non-transactional >> >> * Only support DIST in SYNC mode. Throw a configuration exception >> if DIST and ASYNC are detected. >> * Allow DIST_ASYNC if an unsafe flag is set to true >> (breakApi=true?) in the config, so that the user is explicitly >> aware that API return values will be unreliable. >> * Make sure this is documented in the Javadocs as well as the FAQs >> >> 2. Transactional - Async >> >> * As above, do not support unless breakApi = true. In which case >> this is simple - no return values, only broadcast modifications in >> prepare as we do with REPL_SYNC. >> >> 3. Transactional - Sync >> >> * Offer 2 options here. Option 1 is simple and we already have >> this. Option 2 will need to be written. >> >> Option 1: break API. This will behave the same as async, in that >> we don't care about return values, but we still use a 2-phase >> commit protocol for the transaction boundary. Users will still >> need to specify breakApi = true so this is explicit. >> >> Option 2: Before any write RPC call is broadcast, a remote get is >> performed. This pulls back the necessary values so that a >> reliable return value is available. >> >> Cheers >> Manik >> >> On 17 Apr 2009, at 12:18, Manik Surtani wrote: >> >>> This gets even more interesting when we have transactions in >>> play. E.g.: >>> >>> K resides on {A, B} in a cluster of {A, B, C} >>> >>> On C, we do: >>> >>> 1. tx.begin() >>> 2. retval = C.replace(K, V, V2) >>> 3. // do something with retval >>> 4. tx.commit() >>> >>> It is one thing to set the expectation of unreliable return >>> values for async mode as defined below, but the above would be >>> tricky even in the case of sync mode. On line 2 we would have to >>> start the tx on remote nodes and start processing the commands >>> and pull back results. And on 4, the prepare logic would have to >>> change to assume that all writes have already been executed. >>> >>> On the plus side, this means we have txs that will fail fast(er) >>> if remote locks cannot be be acquired... >>> >>> Thoughts and comments? >>> >>> - Manik >>> >>> On 16 Apr 2009, at 14:33, Manik Surtani wrote: >>> >>>> Some of the API methods' return values may not be as expected >>>> when using DIST. Just wanted to run this by you guys to see >>>> what you thought. >>>> >>>> Specifically, what I am referring to is methods like put(), >>>> putIfAbsent(), and the 2 versions of remove() and replace(). >>>> >>>> Now when these are executed in any other cache mode (LOCAL, >>>> REPL, INVAL) return values are as expected from their >>>> java.util.Map contracts. >>>> >>>> However with DIST, there are 2 separate ways this could go. >>>> >>>> 1. The key in question is mapped to the local cache on which >>>> the method is invoked. >>>> >>>> This case behaves normally as well as per the Map contract. >>>> >>>> 2. The key in question is *not* mapped to the local cache. >>>> E.g., K is mapped to nodes A and B, and on C we do C.remove(). >>>> >>>> I can stick with the map contract if each of these methods first >>>> does a remote lookup of the key and stores this in L1 (or even >>>> in the invocation context and then throw it away if there is no >>>> L1) but this is unnecessarily wasteful IMO. >>>> >>>> The other approach is to realise the command pertains to a key >>>> for which the local cache is not the owner, and replicate the >>>> call to the remote nodes. >>>> >>>> What happens next is also a matter of tuning: we could >>>> >>>> a) replicate + ignore responses. Then the retval to these >>>> methods are indeterminate, and probably unusable. >>>> b) replicate + wait for response. This will adhere to the Map >>>> contract, albeit at a cost. Probably not possible if we use >>>> DIST_ASYNC. >>>> c) L1: we could either "correct" the L1 cache with the change, >>>> or remove the key forcing another lookup for the next time around. >>>> >>>> What do people think? Maybe offer all strategies? Or will this >>>> then become configuration hell? :) >>>> >>>> Cheers >>>> -- >>>> Manik Surtani >>>> manik(a)jboss.org >>>> Lead, Infinispan >>>> Lead, JBoss Cache >>>> http://www.infinispan.org >>>> http://www.jbosscache.org >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >>> -- >>> Manik Surtani >>> manik(a)jboss.org >>> Lead, Infinispan >>> Lead, JBoss Cache >>> http://www.infinispan.org >>> http://www.jbosscache.org >>> >>> >>> >>> >> >> -- >> Manik Surtani >> manik(a)jboss.org >> Lead, Infinispan >> Lead, JBoss Cache >> http://www.infinispan.org >> http://www.jbosscache.org >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Manik Surtani > manik(a)jboss.org > Lead, Infinispan > Lead, JBoss Cache > http://www.infinispan.org > http://www.jbosscache.org > > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Bela Ban Lead JGroups / Clustering Team JBoss - a division of Red Hat

Manik Surtani wrote: > Ok, after discussing this on IRC and the clustering conf call, here > is what we will do: > > 1. Non-transactional > > * Only support DIST in SYNC mode. Throw a configuration exception > if DIST and ASYNC are detected. > * Allow DIST_ASYNC if an unsafe flag is set to true > (breakApi=true?) in the config, so that the user is explicitly > aware that API return values will be unreliable. > * Make sure this is documented in the Javadocs as well as the FAQs I like that the user needs to explicitly enable that, i.e. to ack that he/she won't rely in return values. One more thing (and I think this might be applied in 2 and 3) what if the user previously retrieved the values, in some sort of batch operation. I.e.: cache.retrieve("key1", "key2", "keyn"); //then operate on these keys only oldVal = cache.replace("key1","newVal");//at this point the data would already be in the context/L1 and the API is not broken. If user tries to use an "keyX" that was not fetched, then fail with an exception (unless breakApi is true).

the advantage of retreive operation would be that it minimize network round trips, i,e. "key1" and "key2" might be on the same node, so only one roundtrip. (we can use cache.startBatch() and cache.endBatch() instead of the cache.retrieve(), though I think latter is more descriptive).

> 2. Transactional - Async > > * As above, do not support unless breakApi = true. In which case > this is simple - no return values, only broadcast modifications in > prepare as we do with REPL_SYNC. > > 3. Transactional - Sync > > * Offer 2 options here. Option 1 is simple and we already have > this. Option 2 will need to be written. > > Option 1: break API. This will behave the same as async, in that > we don't care about return values, but we still use a 2-phase > commit protocol for the transaction boundary. Users will still > need to specify breakApi = true so this is explicit. > > Option 2: Before any write RPC call is broadcast, a remote get is > performed. This pulls back the necessary values so that a reliable > return value is available. guess this will also use a 2pc? Also, are we locking progressively or at commit time only, when broadcasting the modifications?

yes, indeed. that's what cache.retrieve("key1", "key2", "keyn"...); would do, fetch all the remote values at once (multiple keys mapped to one node will result in one aggregated get). > Sounds pretty clunky though ... Might be. I think it is easy to grasp though, and can have significant benefits for clients that know all the key set they will manipulate in one session.

Manik Surtani wrote: > > On 21 Apr 2009, at 18:01, Mircea Markus wrote: > >> yes, indeed. that's what cache.retrieve("key1", "key2", >> "keyn"...); would do, fetch all the remote values at once >> (multiple keys mapped to one node will result in one aggregated >> get). >>> Sounds pretty clunky though ... >> Might be. I think it is easy to grasp though, and can have >> significant benefits for clients that know all the key set they >> will manipulate in one session. > > But the keys retrieved could still be wiped out. > > 1. start tx > 2. retrieve(k1, k2, k3) > 3. // go make coffee; other processes changing stuff, which > removes keys from the L1, negating the effect of step 2 Isn't that exactly what happens now with read mvcc entries being held in context? This won't break neither read_committed nor repetable_read.

> 4. replace(k1, v1) // will return incorrect retval. Or will need > to do a remote get again at this point > 5. end tx > > -- > Manik Surtani > manik(a)jboss.org > Lead, Infinispan > Lead, JBoss Cache > http://www.infinispan.org > http://www.jbosscache.org > > > >

Thinking about this a bit more (and implementing tests, etc for this) I think even for the async case we need to do the remote get first (and as a side-effect this would provide reliable return values). The reason is that not doing this causes txs to behave very weird and would need a lot of hacks to behave cleanly without doing the eager get. Take this example (assuming dist-async) 1. tx.begin 2. putIfAbsent(k) // k exists elsewhere 3. get(k) // this will return the OLD val of k, since the tx hasnt completed and the owners haven't seen the WriteCommand in 2 yet! We could hack this to make a record of commands that will be executed later, but in the case of conditional writes (like putIfAbsent) we dont know if they will succeed. So we could do a get first as well, but in this case then we may as well stick with a get-before-write approach and thereby provide reliable retvals. The actual commit would still be 1-phase and async though. WDYT? Also, for consistency, I suggest the same for non-tx-writes. This then serves the added benefit of removing the weird unsafe flag, etc. Ok, so it means doing a remote get before a put, but the cost of this is mitigated because a) it is unicast to a small set of servers b) the RPC call returns as soon as we get the first valid response and c) with MVCC, the get is very quick - no locking needed on the remote end. Comments? On 21 Apr 2009, at 18:24, Manik Surtani wrote: > > On 21 Apr 2009, at 18:18, Mircea Markus wrote: > >> Manik Surtani wrote: >>> >>> On 21 Apr 2009, at 18:01, Mircea Markus wrote: >>> >>>> yes, indeed. that's what cache.retrieve("key1", "key2", >>>> "keyn"...); would do, fetch all the remote values at once >>>> (multiple keys mapped to one node will result in one aggregated get). >>>>> Sounds pretty clunky though ... >>>> Might be. I think it is easy to grasp though, and can have >>>> significant benefits for clients that know all the key set they >>>> will manipulate in one session. >>> >>> But the keys retrieved could still be wiped out. >>> >>> 1. start tx >>> 2. retrieve(k1, k2, k3) >>> 3. // go make coffee; other processes changing stuff, which >>> removes keys from the L1, negating the effect of step 2 >> Isn't that exactly what happens now with read mvcc entries being >> held in context? This won't break neither read_committed nor >> repetable_read. > > Not quite. The return value is calculated atomically when the > command is performed, even though the old value is cached in > context. E.g., locally, > > 1. tx.begin > 2. read K > 3. // go make coffee > 4. replace K. This command is atomic and the retval is extracted > from the datacontainer as this command is perform()'ed. So what this > invocation returns is accurate regardless of interleaving writes > between step 2 & 4 > 5. ... > >>> 4. replace(k1, v1) // will return incorrect retval. Or will need >>> to do a remote get again at this point >>> 5. end tx >>> >>> -- >>> Manik Surtani >>> manik(a)jboss.org >>> Lead, Infinispan >>> Lead, JBoss Cache >>> http://www.infinispan.org >>> http://www.jbosscache.org >>> >>> >>> >>> >> > > -- > Manik Surtani > manik(a)jboss.org > Lead, Infinispan > Lead, JBoss Cache > http://www.infinispan.org > http://www.jbosscache.org > > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 24 Apr 2009, at 06:52, Bela Ban wrote: > Whatever you do, I think quick'n'dirty fire&forget calls are > important: a user might want to push data into the grid at a very > high rate and does not care about return values (for now). > > Maybe an additional API ? > > void putAsync(key, value); > > > Or a batch API: > > void putBatch(Map<K,V> data); Well, putAll(Map<K, V> data) behaves like this since it returns void... > > > ? > > > > Manik Surtani wrote: >> Thinking about this a bit more (and implementing tests, etc for >> this) I think even for the async case we need to do the remote get >> first (and as a side-effect this would provide reliable return >> values). The reason is that not doing this causes txs to behave >> very weird and would need a lot of hacks to behave cleanly without >> doing the eager get. >> >> Take this example (assuming dist-async) >> >> 1. tx.begin >> 2. putIfAbsent(k) // k exists elsewhere >> 3. get(k) // this will return the OLD val of k, since the tx >> hasnt completed and the owners haven't seen the WriteCommand in 2 >> yet! >> >> We could hack this to make a record of commands that will be >> executed later, but in the case of conditional writes (like >> putIfAbsent) we dont know if they will succeed. So we could do a >> get first as well, but in this case then we may as well stick with >> a get-before-write approach and thereby provide reliable retvals. >> >> The actual commit would still be 1-phase and async though. >> >> WDYT? >> >> Also, for consistency, I suggest the same for non-tx-writes. This >> then serves the added benefit of removing the weird unsafe flag, >> etc. Ok, so it means doing a remote get before a put, but the >> cost of this is mitigated because a) it is unicast to a small set >> of servers b) the RPC call returns as soon as we get the first >> valid response and c) with MVCC, the get is very quick - no >> locking needed on the remote end. >> >> Comments? >> >> >> On 21 Apr 2009, at 18:24, Manik Surtani wrote: >> >>> >>> On 21 Apr 2009, at 18:18, Mircea Markus wrote: >>> >>>> Manik Surtani wrote: >>>>> >>>>> On 21 Apr 2009, at 18:01, Mircea Markus wrote: >>>>> >>>>>> yes, indeed. that's what cache.retrieve("key1", "key2", >>>>>> "keyn"...); would do, fetch all the remote values at once >>>>>> (multiple keys mapped to one node will result in one >>>>>> aggregated get). >>>>>>> Sounds pretty clunky though ... >>>>>> Might be. I think it is easy to grasp though, and can have >>>>>> significant benefits for clients that know all the key set >>>>>> they will manipulate in one session. >>>>> >>>>> But the keys retrieved could still be wiped out. >>>>> >>>>> 1. start tx >>>>> 2. retrieve(k1, k2, k3) >>>>> 3. // go make coffee; other processes changing stuff, which >>>>> removes keys from the L1, negating the effect of step 2 >>>> Isn't that exactly what happens now with read mvcc entries being >>>> held in context? This won't break neither read_committed nor >>>> repetable_read. >>> >>> Not quite. The return value is calculated atomically when the >>> command is performed, even though the old value is cached in >>> context. E.g., locally, >>> >>> 1. tx.begin >>> 2. read K >>> 3. // go make coffee >>> 4. replace K. This command is atomic and the retval is >>> extracted from the datacontainer as this command is >>> perform()'ed. So what this invocation returns is accurate >>> regardless of interleaving writes between step 2 & 4 >>> 5. ... >>> >>>>> 4. replace(k1, v1) // will return incorrect retval. Or will >>>>> need to do a remote get again at this point >>>>> 5. end tx >>>>> >>>>> -- >>>>> Manik Surtani >>>>> manik(a)jboss.org >>>>> Lead, Infinispan >>>>> Lead, JBoss Cache >>>>> http://www.infinispan.org >>>>> http://www.jbosscache.org >>>>> >>>>> >>>>> >>>>> >>>> >>> >>> -- >>> Manik Surtani >>> manik(a)jboss.org >>> Lead, Infinispan >>> Lead, JBoss Cache >>> http://www.infinispan.org >>> http://www.jbosscache.org >>> >>> >>> >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> Manik Surtani >> manik(a)jboss.org >> Lead, Infinispan >> Lead, JBoss Cache >> http://www.infinispan.org >> http://www.jbosscache.org >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Bela Ban > Lead JGroups / Clustering Team > JBoss - a division of Red Hat > -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

Manik Surtani wrote: > DIST will always do a get before a write, and this call will be > synchronous. (the actual write command will be executed remotely > either sync or async based on cache mode) This will slow down asynchronous PUTs a lot, which is bad. Especially if the putter doesn't care about the return values. I assume you'll only do this for put(K,V) which returns the prev value ?

So those folks who want async write only and don't care about prev values have to use putAll(Map) ? That's fine but this needs to be stated in the docs

Manik Surtani wrote: > Ok, so this is my final, final solution. :-) > > Both sync and async are supported in DIST. DIST will always do a > get before a write, and this call will be synchronous. (the actual > write command will be executed remotely either sync or async based > on cache mode). > > This will ensure proper return values as well as proper behaviour > of conditional invocations such as putIfAbsent, replace, etc. > > I will still hang on to the <unsafe unreliableReturnValues /> > option though, which will apply to both sync and async mode, which > will allow for skipping the get-before-write, but only where this > is unnecessary for accurate functioning of a method. what about also allowing this on a per invocation basis, through Flag?