<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 29, 2013 at 1:33 PM, Pedro Ruivo <span dir="ltr"><<a href="mailto:pedro@infinispan.org" target="_blank">pedro@infinispan.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5"><br>
<br>
On 10/29/2013 07:58 AM, Radim Vansa wrote:<br>
> On 10/25/2013 08:17 PM, Pedro Ruivo wrote:<br>
>> Hi guys.<br>
>><br>
>> I've open a JIRA to tack this: <a href="https://issues.jboss.org/browse/ISPN-3664" target="_blank">https://issues.jboss.org/browse/ISPN-3664</a><br>
>><br>
>> Suggestions/feedback is appreciated. This is probably be integrated in<br>
>> the next major (but no promises).<br>
>><br>
>> I was not cleared just ping me.<br>
>><br>
>> Have a nice weekend folks :)<br>
>><br>
>> Regards,<br>
>> Pedro<br>
>><br>
>> Description is the following:<br>
>><br>
>> Major refactorization of the write command with the following goals:<br>
>><br>
>> * Base WriteCommand: all the write command has the same workflow through<br>
>> the interceptor chain<br>
>> * Create a concrete WriteCommand for each operation (put, remove,<br>
>> replace, replaceIfEquals, removeIfEquals, putIfAbsent)<br>
>> * Extend the interceptor chain to process each one of the command and<br>
>> add a new "visitWriteCommand", that is invoked by the default visitX<br>
>> methods.<br>
>> * (minor) change the GetKeyValueCommand to ReadCommand to make name<br>
>> "compatible" with WriteCommand.<br>
>><br>
>> Note that most of the interceptor only needs to implement the<br>
>> visitWriteCommand because all the write command has the same execution<br>
>> flow. The basic flow of the write commands are: (non-tx) lock, fetch<br>
>> value (cachestore/remote), check condition and apply change. for tx<br>
>> mode, lock (if pessimistic), fetch value (cache loader, remote, etc),<br>
>> apply change and add it to the transaction (if successful)<br>
> I've been wondering for a while why the fetch part is a separate message<br>
> in the write flow. Is the return value of much use when it does not<br>
> return really the replaced value but only some of the previous values?<br>
> And this way you double* the latency. I think that returning the value<br>
> from primary owner as the response for write would make more sense.<br>
> Naturally, for optimistic txs you can only do the get, and for<br>
> pessimistic ones you'd have to return the value together with the<br>
> locking, but still, the operations would make more sense.<br>
<br>
</div></div>Actually, I think you are right. It makes no sense to fecth the value<br>
before the write operation for non-tx caches. I also like your idea of<br>
fetching the value when the key is locked :)<br>
<br>
Do you mind to create a new thread for it (I'm not sure if everybody<br>
reads this thread)?<br></blockquote><div><br>I don't think we actually fetch the value before the write operation for non-tx caches, at most we do an extra lookup in the local data container (in NonTxDistributionInterceptor.remoteGetBeforeWrite).<br>
<br></div><div>We do fetch the value explicitly in transactional caches, but we don't invoke the write command remotely in that case.<br></div><div><br></div><div>There is an opportunity for optimization in pessimistic tx caches, because we invoke both a remote get and a remote lock command for the same write command. Perhaps we can make LockControlCommand return the previous value (only if needed, of course).<br>
<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class=""><div class="h5"><br>
><br>
> *: OK, it's not doubling as with the write the primary owner replicates<br>
> the write to backups, but it's doubling the amount of communication<br>
> originating from the requestor node.<br>
><br>
>><br>
>> Also, another advantage is the simplification of the EntryFactory<br>
>> because if we think a little about it, independent of the write command<br>
>> we need to wrap the entry anyway.<br>
>><br>
>> Suggested implementation<br>
>><br>
>> class abstract WriteCommand,<br>
>> Object key, Object newValue<br>
>> boolen match(Object currentValue) //true by default<br>
>> boolean needsRemoteGetBeforeWrite() //true by default<br>
>> object perform() //common implementation like: if<br>
>> (match(entry.getValue()) then entry.setValue(newValue);<br>
>> entry.setChanged(true); entry.setRemoved(newValue == null)}<br>
>><br>
>> * Concrete implementations *<br>
>><br>
>> {PutCommand|RemoveCommand} extends WriteCommand<br>
>> ovewrite needsRemoteGetBeforeWrite() {return<br>
>> !flags.contains(IGNORE_RETURN_VALUE)}<br>
>><br>
>> ReplaceIfPresentCommand extends WriteCommand<br>
>> ovewrite match(Object currentValue) {return currentValue != null}<br>
>><br>
>> PutIfAbsentCommand extends WriteCommand<br>
>> ovewrite match(Object currentValue) {return currentValue == null}<br>
>><br>
>> * Special base class for operation with expected value to compare *<br>
>><br>
>> class abstract AdvancedWriteCommand extends WriteCommand<br>
>> Object expectedValue<br>
>> match(Object currentValue) {return currentValue.equals(expectedValue)}<br>
> I'd call that rather ConditionalWriteCommand - AdvancedWC sounds too<br>
> general to me.<br></div></div></blockquote><div><br></div><div>+1 for ConditionalWriteCommand<br><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class=""><div class="h5">
><br>
> My 2c.<br>
><br>
> Radim<br>
><br>
</div></div><div class=""><div class="h5">_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
</div></div></blockquote></div><br></div></div>