[jboss-dev-forums] [Design of Clustering on JBoss (Clusters/JBoss)] - Re: Solution of JBREM-954 and ramifications into UnifiedInvo

[ron.sigal@jboss.com]

"galder.zamarreno at jboss.com" wrote : 
  | Client.invoke() is the interface Unified invokers and others deal with and this method throws Throwable so your users have to be prepared for anything anyway... 
  | 

Good point.  That relieves my unfounded anxiety.

"galder.zamarreno at jboss.com" wrote : 
  | By the way, how would you turn InterruptedException into an InterruptedIOException? You can't set a cause Throwable to InterruptedIOException, so you can't really wrap IE around a InterruptedIOException,
  | 

I was just going to do 


  | catch (InterruptedException e)
  | {
  |    throw new InterruptedException(e.getMessage);
  | }
  | 

I'm looking at JBossMessaging as a test case.  org.jboss.jms.client.delegate.DelegateSupport filters any Throwable thrown by org.jboss.remoting.Client.invoke() through the following code:


  |    public JMSException handleThrowable(Throwable t)
  |    {
  |       // ConnectionFailedException could happen during ConnectionFactory.createConnection.
  |       // IOException could happen during an interrupted exception.
  |       // CannotConnectionException could happen during a communication error between a connected
  |       // remoting client and the server (what means any new invocation).
  | 
  |       if (t instanceof JMSException)
  |       {
  |          return (JMSException)t;
  |       }
  |       else if (t instanceof CannotConnectException)
  |       {
  |       	boolean failover = true;
  |       	CannotConnectException cc = (CannotConnectException)t;
  |       	Throwable underlying = cc.getCause();
  |       	if (underlying != null && underlying instanceof SocketException)
  |       	{
  |       		//If remoting fails to find a connection because the client pool is full
  |       		//then it throws a SocketException! - in this case we DO NOT want to failover
  |       		//See http://jira.jboss.com/jira/browse/JBMESSAGING-1114
  |       		if (underlying.getMessage() != null &&
  |       			 underlying.getMessage().startsWith("Can not obtain client socket connection from pool"))
  |       		{
  |       			log.warn("Timed out getting a connection from the pool. Try increasing clientMaxPoolSize and/or numberOfRetries " +
  |       					   "attributes in remoting-xxx-service.xml");
  |       			failover = false;
  |       		}
  |       	}
  |       	if (failover)
  |       	{
  |       		return new MessagingNetworkFailureException(cc);
  |       	}      	      	      	      	      	
  |       }
  |       else if ((t instanceof IOException) || (t instanceof ConnectionFailedException))
  |       {
  |          return new MessagingNetworkFailureException((Exception)t);
  |       }
  |       //This can occur if failure happens when Client.connect() is called
  |       //Ideally remoting should have a consistent API
  |       else if (t instanceof RuntimeException)
  |       {
  |          RuntimeException re = (RuntimeException)t;
  |          
  |          Throwable initCause = re.getCause();
  |          
  |          if (initCause != null)
  |          {
  |             do
  |             {
  |                if ((initCause instanceof CannotConnectException) ||
  |                         (initCause instanceof IOException) ||
  |                         (initCause instanceof ConnectionFailedException))
  |                {
  |                   return new MessagingNetworkFailureException((Exception)initCause);
  |                }
  |                initCause = initCause.getCause();
  |             }
  |             while (initCause != null);
  |          }
  |       }
  |          
  |       return new MessagingJMSException("Failed to invoke", t);      
  |    }
  | 

I gather that a MessagingNetworkFailureException indicates the need to try to do a failover.

1. In its current state, this code would take an InterruptedException wrapped in a CannotConnectException and initiate a failover.

2. If Remoting replaced InterruptedExceptions by InterruptedIOExceptions, JBM would see an IOException and initiate a failover.

3. If Remoting wrapped InterruptedExceptions in RuntimeExceptions, JBM would return a MessagingJMSException and, presumably, not initiate a failover.

So, it turns out that strategy 3 is best for JBossMessaging.  But it looks like good luck more than anything else.  The treatment of CannotConnectException looks for one known anomaly (which, by the way, no longer exists) and does a failover by default.  The treatment of RuntimeException looks for three particular cases and *doesn't* do a failover by default.

The fact is that none of us anticipated an InterruptedException in these circumstances.  It seems to me that any existing code is equally likely to react well or badly in response to either wrapping InterruptedExceptions in RuntimeExceptions or replacing InterruptedExceptions by InterruptedExceptions.

The bottom line is that (1) something has to be done, and (2) there doesn't seem to be a solution that leaves the Remoting API unchanged and guarantees that no code suffers.  

Is there any reason to prefer one solution over the other?  One could argue that an interruptible EJB or JMS client *should* declare that it throws an InterruptedException, but that discussion might be more theological than technical.  I'm leaning towards wrapping in RuntimeExceptions, which seems more straightforward and avoids the UndeclaredThrowableException  problem.

There's one more concern.  Suppose a Remoting user detected the InterruptedException possibility and their code looks for InterruptedExceptions wrapped in CannotConnectExceptions.  Then replacing CannotConnectExceptions with RuntimeExceptions might break their code.  I'm leaning towards

1. for Remoting 2.2.2.SP8

a) leaving the current behavior in place as the default behavior, and

b) making the new behavior a configurable alternative.

2. for Remoting 2.4.0, replacing the current behavior with the new behavior.

Now WDYT?

View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4146373#4146373

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=4146373