Re: [infinispan-dev] Improve WriteCommand processing code and [possibly] performance

On 10/29/2013 07:58 AM, Radim Vansa wrote: > On 10/25/2013 08:17 PM, Pedro Ruivo wrote: >> Hi guys. >> >> I've open a JIRA to tack this: https://issues.jboss.org/browse/ISPN-3664 >> >> Suggestions/feedback is appreciated. This is probably be integrated in >> the next major (but no promises). >> >> I was not cleared just ping me. >> >> Have a nice weekend folks :) >> >> Regards, >> Pedro >> >> Description is the following: >> >> Major refactorization of the write command with the following goals: >> >> * Base WriteCommand: all the write command has the same workflow through >> the interceptor chain >> * Create a concrete WriteCommand for each operation (put, remove, >> replace, replaceIfEquals, removeIfEquals, putIfAbsent) >> * Extend the interceptor chain to process each one of the command and >> add a new "visitWriteCommand", that is invoked by the default visitX >> methods. >> * (minor) change the GetKeyValueCommand to ReadCommand to make name >> "compatible" with WriteCommand. >> >> Note that most of the interceptor only needs to implement the >> visitWriteCommand because all the write command has the same execution >> flow. The basic flow of the write commands are: (non-tx) lock, fetch >> value (cachestore/remote), check condition and apply change. for tx >> mode, lock (if pessimistic), fetch value (cache loader, remote, etc), >> apply change and add it to the transaction (if successful) > I've been wondering for a while why the fetch part is a separate message > in the write flow. Is the return value of much use when it does not > return really the replaced value but only some of the previous values? > And this way you double* the latency. I think that returning the value > from primary owner as the response for write would make more sense. > Naturally, for optimistic txs you can only do the get, and for > pessimistic ones you'd have to return the value together with the > locking, but still, the operations would make more sense. Actually, I think you are right. It makes no sense to fecth the value before the write operation for non-tx caches. I also like your idea of fetching the value when the key is locked :) Do you mind to create a new thread for it (I'm not sure if everybody reads this thread)?

> > *: OK, it's not doubling as with the write the primary owner replicates > the write to backups, but it's doubling the amount of communication > originating from the requestor node. > >> >> Also, another advantage is the simplification of the EntryFactory >> because if we think a little about it, independent of the write command >> we need to wrap the entry anyway. >> >> Suggested implementation >> >> class abstract WriteCommand, >> Object key, Object newValue >> boolen match(Object currentValue) //true by default >> boolean needsRemoteGetBeforeWrite() //true by default >> object perform() //common implementation like: if >> (match(entry.getValue()) then entry.setValue(newValue); >> entry.setChanged(true); entry.setRemoved(newValue == null)} >> >> * Concrete implementations * >> >> {PutCommand|RemoveCommand} extends WriteCommand >> ovewrite needsRemoteGetBeforeWrite() {return >> !flags.contains(IGNORE_RETURN_VALUE)} >> >> ReplaceIfPresentCommand extends WriteCommand >> ovewrite match(Object currentValue) {return currentValue != null} >> >> PutIfAbsentCommand extends WriteCommand >> ovewrite match(Object currentValue) {return currentValue == null} >> >> * Special base class for operation with expected value to compare * >> >> class abstract AdvancedWriteCommand extends WriteCommand >> Object expectedValue >> match(Object currentValue) {return currentValue.equals(expectedValue)} > I'd call that rather ConditionalWriteCommand - AdvancedWC sounds too > general to me.

> > My 2c. > > Radim > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev