1:17 p.m.
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)
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)}
{RemoveIfEquals|ReplaceIfEquals} extends AdvancedWriteCommand //no
different implementation needed.
5:55 a.m.
Hi Pedro,
I like the idea. Reducing the number of visitX commands for write commands would be a good
thing from the POV of adding new interceptors.
What would happen with existing visitPutKey…etc methods? Would this dissapear or would
they be left there still for those interceptors that want to do something specific to each
command?
Cheers,
On Oct 25, 2013, at 7:17 PM, Pedro Ruivo <pedro(a)infinispan.org> wrote:
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
6:04 a.m.
Hi,
This was discussed in the last day in Farnborough. The idea is to keep
the visitX methods for each type of operations (something like):
* visitPutCommand
* visitRemoveCommand
* visitReplaceCommand
* visitRemoveIfEqualsCommand
* visitReplaceIfEqualsCommand
* visitPutIfAbsentCommand
and finally a last method:
* visitWriteCommand
In our current stack implementation, the visitX will only invoke the
last one (because the logic is the same for all types of commands) but
for CustomInterceptors we decide to keep them if a user may want to
intercept only a type of commands.
Cheers,
Pedro
On 10/28/2013 10:55 AM, Galder Zamarreño wrote:
7:58 a.m.
On Oct 28, 2013, at 10:55 AM, Galder Zamarreño <galder(a)redhat.com> wrote:
+1. It would also enforce code reuse between operations that amend the cache.
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
2:58 a.m.
On 10/25/2013 08:17 PM, Pedro Ruivo wrote:
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.
I'd call
that rather ConditionalWriteCommand - AdvancedWC sounds too
general to me.
My 2c.
Radim
--
Radim Vansa <rvansa(a)redhat.com>
JBoss DataGrid QA
8:33 a.m.
On 10/29/2013 07:58 AM, Radim Vansa wrote:
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)?
9:42 a.m.
New subject: 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
10:08 a.m.
New subject: 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 <rvansa(a)redhat.com> wrote:
10:56 a.m.
New subject: Retrieving value before write
On Oct 29, 2013, at 3:08 PM, William Burns <mudokonman(a)gmail.com> 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.
We're already doing this for non-tx caches, at least since the lock delegation was
implemented.
+1
+1
Care to create a JIRA for this please?
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
12:58 p.m.
New subject: Retrieving value before write
On Tue, Oct 29, 2013 at 3:56 PM, Mircea Markus <mmarkus(a)redhat.com> wrote:
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.
1:06 p.m.
New subject: Retrieving value before write
On Tue, Oct 29, 2013 at 12:58 PM, Dan Berindei <dan.berindei(a)gmail.com> wrote:
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.
10:02 a.m.
On Tue, Oct 29, 2013 at 1:33 PM, Pedro Ruivo <pedro(a)infinispan.org> wrote:
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).
+1 for ConditionalWriteCommand
4174
days inactive
4178
days old
infinispan-dev@lists.jboss.org
11 comments
6 participants
participants (6)
-
Dan Berindei -
Galder Zamarreño -
Mircea Markus -
Pedro Ruivo -
Radim Vansa -
William Burns