<div class="gmail_quote">On Tue, Oct 2, 2012 at 6:29 PM, Bela Ban <span dir="ltr"><<a href="mailto:bban@redhat.com" target="_blank">bban@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
On 10/2/12 1:43 PM, Dan Berindei wrote:<br>
<br>
>>> test threads are stuck in JGroupsTransport.waitForChannelToConnect so it<br>
>>> looks like a JGroups thing.<br>
>><br>
>><br>
>> JGroupsTransport.waitForChannelToConnect() is Infinispan code, not<br>
>> JGroups code. IMO unneeded code, too, as I don't understand why we're<br>
>> waiting on a latch until we get a view after calling JChannel.connect().<br>
>> When JChannel.connect() returns, you're guaranteed to have a view<br>
>> installed, so this code can (should !) be removed, as it makes things<br>
>> unnecessarily complex (and error prone ?).<br>
>><br>
><br>
> True, it's Infinispan code - my code, actually ;)<br>
<br>
<br>
</div>ouch :-)<br>
<div class="im"><br>
<br>
> The reason behind the latch was probably to avoid concurrent updates of the<br>
> members list from the main thread and from the JGroups thread that calls viewAccepted().<br>
<br>
<br>
</div>The members should be updated in viewAccepted(). When you connect a<br>
channel, then viewAccepted() will be called *before* JChannel.connect()<br>
returns, so the main thread won't have a chance of accessing members<br>
before that.<br>
<div class="im"><br></div></blockquote><div><br>Ok, if viewAccepted() is guaranteed to be called before connect() returns then you're right, the latch is superfluous. We only need to call viewAccepted() ourselves if the channel was injected (which we already do).<br>
<br>In this particular case, however, we don't get a viewAccepted() call or an exception from connect(), so I'm not sure it would have been better (easier to debug) without the latch. <br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
<br>
> I agree it's probably unnecessary, as we already use synchronization in our viewAccepted() implementation, but I don't see why it shouldn't work.<br>
<br>
</div>It's (unnecessarily) complex code that could be simplified; I'm not<br>
suggesting it's incorrect. More complexity == more bugs.<br>
<div class="im"><br></div></blockquote><div><br>True. I just wasn't sure whether viewAccepted() is guaranteed to be called during connect(), so I added a "safety net".<br><br>I know there was a NPE in JGroupsTransport that I thought at the time was caused by a missing viewAccepted() call, but I can't find the stack trace any more so I'm not sure if that was really the case.<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<br>
> We have attached the JGroupsTransport as a membership listener to the<br>
> MessageDispatcher (CommandAwareRpcDispatcher, actually) and we have<br>
> attached the MessageDispatcher as a channel listener to the channel before<br>
> calling connect(), so JGroups should call viewAccepted() for the initial<br>
> view just as it does for every view.<br>
<br>
<br>
</div>You mean MembershipListener ? Yes, if you create the channel, then the<br>
MessageDispatcher, then connect(), then you'll get the viewAccepted()<br>
callback *before* JChannel.connect() returns.<br>
<br>
Unless you deliver the view in a separate thread, but I don't think you<br>
do this, right ?<br>
<br></blockquote><div><br>I'm interested here strictly in the viewAccepted() callback in JGroupsTransport - I don't care how our other components that listen for our JGroups view events get their notifications, they usually all their version of viewAccepted() on startup.<br>
<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Which version is the stacktrace below with ? 3.0.x ? Taking a look now...<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br>3.2.0.Alpha2<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
<br>
> I just got the same test to hang on my machine, and I found this exception<br>
> in the log:<br>
><br>
> 12:26:25,098 DEBUG (CacheStarter-Cache4:nbst) [GMS]<br>
> exception=java.lang.IllegalStateException: channel is not connected,<br>
> retrying<br>
> java.lang.IllegalStateException: channel is not connected<br>
> at<br>
> org.jgroups.blocks.MessageDispatcher$ProtocolAdapter.down(MessageDispatcher.java:621)<br>
> at<br>
> org.jgroups.blocks.RequestCorrelator.handleRequest(RequestCorrelator.java:535)<br>
> at<br>
> org.jgroups.blocks.RequestCorrelator.receiveMessage(RequestCorrelator.java:390)<br>
> at<br>
> org.jgroups.blocks.RequestCorrelator.receive(RequestCorrelator.java:248)<br>
> at<br>
> org.jgroups.blocks.MessageDispatcher$ProtocolAdapter.up(MessageDispatcher.java:604)<br>
> at org.jgroups.JChannel.up(JChannel.java:715)<br>
> at org.jgroups.stack.ProtocolStack.up(ProtocolStack.java:1020)<br>
> at org.jgroups.protocols.FRAG2.up(FRAG2.java:181)<br>
> at org.jgroups.protocols.FC.up(FC.java:479)<br>
> at org.jgroups.protocols.pbcast.GMS.up(GMS.java:896)<br>
> at org.jgroups.protocols.pbcast.STABLE.up(STABLE.java:244)<br>
> at org.jgroups.protocols.UNICAST2.up(UNICAST2.java:432)<br>
> at org.jgroups.protocols.pbcast.NAKACK2.handleMessage(NAKACK2.java:722)<br>
> at org.jgroups.protocols.pbcast.NAKACK2.up(NAKACK2.java:570)<br>
> at<br>
> org.jgroups.protocols.pbcast.NAKACK2.flushBecomeServerQueue(NAKACK2.java:841)<br>
> at org.jgroups.protocols.pbcast.NAKACK2.down(NAKACK2.java:490)<br>
> at org.jgroups.protocols.UNICAST2.down(UNICAST2.java:523)<br>
> at org.jgroups.protocols.pbcast.STABLE.down(STABLE.java:307)<br>
> at org.jgroups.protocols.pbcast.GMS.installView(GMS.java:637)<br>
> at<br>
> org.jgroups.protocols.pbcast.ClientGmsImpl.installView(ClientGmsImpl.java:248)<br>
> at<br>
> org.jgroups.protocols.pbcast.ClientGmsImpl.joinInternal(ClientGmsImpl.java:182)<br>
> at<br>
> org.jgroups.protocols.pbcast.ClientGmsImpl.join(ClientGmsImpl.java:37)<br>
> at org.jgroups.protocols.pbcast.GMS.down(GMS.java:938)<br>
> at org.jgroups.protocols.FC.down(FC.java:435)<br>
> at org.jgroups.protocols.FRAG2.down(FRAG2.java:147)<br>
> at org.jgroups.stack.ProtocolStack.down(ProtocolStack.java:1025)<br>
> at org.jgroups.JChannel.down(JChannel.java:729)<br>
> at org.jgroups.JChannel.connect(JChannel.java:291)<br>
> at org.jgroups.JChannel.connect(JChannel.java:262)<br>
> at<br>
> org.infinispan.remoting.transport.jgroups.JGroupsTransport.startJGroupsChannelIfNeeded(JGroupsTransport.java:206)<br>
> at<br>
> org.infinispan.remoting.transport.jgroups.JGroupsTransport.start(JGroupsTransport.java:197)<br>
> at sun.reflect.GeneratedMethodAccessor113.invoke(Unknown Source)<br>
> at<br>
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)<br>
> at java.lang.reflect.Method.invoke(Method.java:601)<br>
> at<br>
> org.infinispan.util.ReflectionUtil.invokeAccessibly(ReflectionUtil.java:236)<br>
> at<br>
> org.infinispan.factories.AbstractComponentRegistry$PrioritizedMethod.invoke(AbstractComponentRegistry.java:900)<br>
> at<br>
> org.infinispan.factories.AbstractComponentRegistry.invokeStartMethods(AbstractComponentRegistry.java:650)<br>
> at<br>
> org.infinispan.factories.AbstractComponentRegistry.internalStart(AbstractComponentRegistry.java:639)<br>
> at<br>
> org.infinispan.factories.AbstractComponentRegistry.start(AbstractComponentRegistry.java:542)<br>
> at<br>
> org.infinispan.factories.GlobalComponentRegistry.start(GlobalComponentRegistry.java:218)<br>
> at<br>
> org.infinispan.manager.DefaultCacheManager.wireAndStartCache(DefaultCacheManager.java:680)<br>
> at<br>
> org.infinispan.manager.DefaultCacheManager.createCache(DefaultCacheManager.java:652)<br>
> at<br>
> org.infinispan.manager.DefaultCacheManager.getCache(DefaultCacheManager.java:548)<br>
> at<br>
> org.infinispan.statetransfer.JoiningNode.getCache(JoiningNode.java:54)<br>
> at<br>
> org.infinispan.statetransfer.StateTransferFunctionalTest$2.run(StateTransferFunctionalTest.java:234)<br>
> at java.lang.Thread.run(Thread.java:722)<br>
<br>
<br>
</div></div><div class="HOEnZb"><div class="h5">--<br>
Bela Ban, JGroups lead (<a href="http://www.jgroups.org" target="_blank">http://www.jgroups.org</a>)<br>
_______________________________________________<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>