Manik Surtani wrote:

On 25 Mar 2009, at 19:23, Galder Zamarreno wrote:

Hi,

I'm looking at RemoteCommandFactory in Infinispan as part as mapping the marshalling stuff to JBoss Marshalling and here's something that has confused me:

fromStream() method says:

* Creates an un-initialized command.  Un-initialized in the sense that parameters will be set, but any components

* specific to the cache in question will not be set.

* <p/>

* You would typically set these parameters using {@link org.infinispan.commands.CommandsFactory#initializeReplicableCommand(ReplicableCommand)}

* <p/>

And then you do the following:

       case StateTransferControlCommand.METHOD_ID:

          command = new StateTransferControlCommand();

          ((StateTransferControlCommand) command).init(rpcManager);

          break;

Shouldn't ((StateTransferControlCommand) command).init(rpcManager) go into org.infinispan.commands.CommandsFactory#initializeReplicableCommand(ReplicableCommand).

initializeReplicableCommand() javadoc says:

* Initializes a {@link org.infinispan.commands.ReplicableCommand} read from * a data stream with components specific to the target cache instance.

The rpcManager seems to be one those components?

No.  The RPCManager is scoped as GLOBAL:

   https://svn.jboss.org/repos/infinispan/trunk/src/main/java/org/infinispan/remoting/RPCManager.java Refer to the @Scope annotation on the interface.

But I understand your confusion here.  We should either:

1.  Change the Javadocs on fromStream() to be more explicit, i.e., only dependent components of GLOBAL scope are injected.  NAMED_CACHE scope components are only injected later with CommandsFactory.initializeReplicableCommand().

2.  Remove the injection in fromStream(); have CommandsFactory.initializeReplicableCommand() handle all of the injection, of GLOBAL and NAMED_CACHE scoped components.

I have no problem with 2, if it makes your integration easier.  In fact, I actually think 2 is probably cleaner.

I've looked into this and it looked simpler saying than actually doing it. The problem here is that StateTransferControlCommand is dealt in a very different way to the rest of ReplicableCommands that extend CacheRPCCommand.

CommandAwareRpcDispatcher.handle shows http://pastebin.com/m446036c4 and the problem is that perform() is called directly for StateTransferControlCommand before any initialisation could be done. That's why the RPCManager was set during unmarshalling.

I still think that StateTransferControlCommand, being a ReplicableCommand, should be allowed to go through CommandsFactory.initializeReplicableCommand() but CommandAwareRpcDispatcher.handle does not have access to command factory and neither the global registry of components.

Ideally, I'd like to see something this being doable: http://pastebin.com/m3f403e2

However, I dunno whether it's doable or make sense.

Currently, CommandsFactory.initializeReplicableCommand() call happens within InboundInvocationHandlerImpl.handle(CacheRPCCommand).

Thoughts? I suppose I could always fallback on 1 but I don't like different commands being treated in a different way. Ideally, all replicable commands received would be instantiated in the marshaller, and they would all be initialised in the same place.

Cheers

--

Manik Surtani

Lead, JBoss Cache

http://www.jbosscache.org

manik@jboss.org

--
Galder Zamarreņo
Sr. Software Maintenance Engineer
JBoss, a division of Red Hat