ISPN-7377 RPCs to non-members throw spurious TimeoutException

[danberindei]

… members

https://issues.jboss.org/browse/ISPN-7377

MessageDispatcher.cast() removes missing members from the destinations list

[rvansa]

I don't really understand when this happens: is dest strict subset of addresses in results? Does this mean that there is a result from node we have not sent the message to? On the other hand, it seems that an address from dests could be missing in results so I don't understand why are you comparing the size of the sets this way...

[danberindei]

Yeah, the condition should have been the other way around, and that's why the fix didn't really work.

[rvansa]

Is it possible to add an unit test?

[danberindei]

I fixed the comparison and added a test.

[rvansa]

@danberindei Okay, looks better :) However, I would rather like to see this as a fix to JGroups than a workaround like this. Or, even better, we could get rid of the RspList completely, as this is quite heavy allocation-wise (it's a HashMap), pushing Responses to JGroups. There are quite a few translations from JGroups APIs to different sorts of maps for each request.

[belaban]

I've been thinking about changing this to use a more reactive pattern like onNext() for each response, terminated by onCompletion() / onError().
However, this is an API change and I don't want to introduce this so late in the game. Besides, Infinispan will move from MessageDispatcher to JChannel and therefore this is (or will be) moot.

[rvansa]

@danberindei How do we remember to remove this burden once Infinispan stops using MD, then?

[danberindei]

@rvansa If we stop using MD then we automatically stop using RspList as well, we'll keep only the HashMap in Responses :)

I've already started working on this, and for now I'm still using Rsp, but I completely replaced RspList with Responses. Before I open the PR I intend to remove the usage of Rsp as well. I have also considered changing the API, but I'm not convinced about using lambdas for this.

@belaban Still, @rvansa is right, it would be better if we didn't need this workaround, and MessageDispatcher.cast() created a Rsp instance for each recipient when the collection of addresses is not null.

[belaban]

Not sure I follow... why do you still need Rsp or Responses (from JGroups?) if you use the channel directly?

[danberindei]

@belaban Responses is our own class, but for some reason @rvansa wants to push it to JGroups ;)
I'm still using Rsp at the current stage of the migration to JChannel, but I'm removing those usages soon.

[rvansa]

@danberindei I don't want to push it into JGroups if that wouldn't be of use; I want to avoid RspList -> Responses transformations.

[galderz]

So, what's the conclusion to this?

[danberindei]

I think it's ready to go in, and we'll resume the Responses discussion when I finish https://issues.jboss.org/browse/ISPN-6971 :)

[slaskawi]

Integrated thanks!