Re: [infinispan-dev] Retrieving value before write
On Tue, Oct 29, 2013 at 12:58 PM, Dan Berindei wrote: > > > > On Tue, Oct 29, 2013 at 3:56 PM, Mircea Markus wrote: >> >> >> On Oct 29, 2013, at 3:08 PM, William Burns wrote: >> >> > I agree, and I could have sworn there was a JIRA that detailed this, >> > but I can't seem to locate it now. I was hoping to get to it next >> > release. >> >> +1. >> >> > >> > This also is a very big improvement for non tx caches as well as the >> > updating cache only has to send a single message, possibly reducing >> > the overall amount of required JGroups messages by a third. >> >> We're already doing this for non-tx caches, at least since the lock >> delegation was implemented. >> >> > Although >> > the amount of bytes reduced is only a fixed amount less. >> > >> > Also it would make the consistency more correct since right now there >> > is technically a window where you could have 2 concurrent updates but >> > only 1 sees the correct returned value. >> >> +1 > > > I'm not sure about that... in non-tx caches we already return the previous > value from the write command, and in pessimistic caches we acquire the > remote lock *before* we retrieve the value. Yeah unfortunately what I said was thinking before lock delegation rewrite that Mircea mentioned. Looking at it again I would agree non tx and pessimistic would be fine. > >> >> >> > >> > On Tue, Oct 29, 2013 at 9:42 AM, Radim Vansa wrote: >> >> Hi, >> >> >> >> Pedro suggested moving the following idea to separate thread (from the >> >> 'Improve WriteCommand processing code...'). Re: >> >> >> >> 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. >> >> >> >> *: 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. >> >> >> >> WDYT? >> >> +1 >> Care to create a JIRA for this please? >> >> >> >> >> >> >> Radim >> >> ___ >> >> 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 >> >> Cheers, >> -- >> Mircea Markus >> Infinispan lead (www.infinispan.org) >> >> >> >> >> >> ___ >> 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
Re: [infinispan-dev] Retrieving value before write
On Tue, Oct 29, 2013 at 3:56 PM, Mircea Markus wrote: > > On Oct 29, 2013, at 3:08 PM, William Burns wrote: > > > I agree, and I could have sworn there was a JIRA that detailed this, > > but I can't seem to locate it now. I was hoping to get to it next > > release. > > +1. > > > > > This also is a very big improvement for non tx caches as well as the > > updating cache only has to send a single message, possibly reducing > > the overall amount of required JGroups messages by a third. > > We're already doing this for non-tx caches, at least since the lock > delegation was implemented. > > > Although > > the amount of bytes reduced is only a fixed amount less. > > > > Also it would make the consistency more correct since right now there > > is technically a window where you could have 2 concurrent updates but > > only 1 sees the correct returned value. > > +1 > I'm not sure about that... in non-tx caches we already return the previous value from the write command, and in pessimistic caches we acquire the remote lock *before* we retrieve the value. > > > > > On Tue, Oct 29, 2013 at 9:42 AM, Radim Vansa wrote: > >> Hi, > >> > >> Pedro suggested moving the following idea to separate thread (from the > >> 'Improve WriteCommand processing code...'). Re: > >> > >> 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. > >> > >> *: 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. > >> > >> WDYT? > > +1 > Care to create a JIRA for this please? > > > >> > >> Radim > >> ___ > >> 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 > > Cheers, > -- > Mircea Markus > Infinispan lead (www.infinispan.org) > > > > > > ___ > 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
Re: [infinispan-dev] Retrieving value before write
On Oct 29, 2013, at 3:08 PM, William Burns wrote: > I agree, and I could have sworn there was a JIRA that detailed this, > but I can't seem to locate it now. I was hoping to get to it next > release. +1. > > This also is a very big improvement for non tx caches as well as the > updating cache only has to send a single message, possibly reducing > the overall amount of required JGroups messages by a third. We're already doing this for non-tx caches, at least since the lock delegation was implemented. > Although > the amount of bytes reduced is only a fixed amount less. > > Also it would make the consistency more correct since right now there > is technically a window where you could have 2 concurrent updates but > only 1 sees the correct returned value. +1 > > On Tue, Oct 29, 2013 at 9:42 AM, Radim Vansa wrote: >> Hi, >> >> Pedro suggested moving the following idea to separate thread (from the >> 'Improve WriteCommand processing code...'). Re: >> >> 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. >> >> *: 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. >> >> WDYT? +1 Care to create a JIRA for this please? >> >> Radim >> ___ >> 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 Cheers, -- Mircea Markus Infinispan lead (www.infinispan.org) ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Retrieving value before write
I agree, and I could have sworn there was a JIRA that detailed this, but I can't seem to locate it now. I was hoping to get to it next release. This also is a very big improvement for non tx caches as well as the updating cache only has to send a single message, possibly reducing the overall amount of required JGroups messages by a third. Although the amount of bytes reduced is only a fixed amount less. Also it would make the consistency more correct since right now there is technically a window where you could have 2 concurrent updates but only 1 sees the correct returned value. On Tue, Oct 29, 2013 at 9:42 AM, Radim Vansa wrote: > Hi, > > Pedro suggested moving the following idea to separate thread (from the > 'Improve WriteCommand processing code...'). Re: > > 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. > > *: 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. > > WDYT? > > Radim > ___ > 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
Re: [infinispan-dev] Improve WriteCommand processing code and [possibly] performance
On Tue, Oct 29, 2013 at 1:33 PM, Pedro Ruivo wrote:
>
>
> 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)?
>
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).
We do fetch the value explicitly in transactional caches, but we don't
invoke the write command remotely in that case.
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).
> >
> > *: 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.
>
+1 for ConditionalWriteCommand
> >
> > My 2c.
> >
> > Radim
> >
> ___
> 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] Retrieving value before write
Hi, Pedro suggested moving the following idea to separate thread (from the 'Improve WriteCommand processing code...'). Re: 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. *: 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. WDYT? Radim ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
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@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Improve WriteCommand processing code and [possibly] performance
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.
*: 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
--
Radim Vansa
JBoss DataGrid QA
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
