[jboss-cvs] JBossRemoting/src/main/org/jboss/remoting ...

Ovidiu Feodorov ovidiu.feodorov at jboss.com
Tue Jan 23 04:39:02 EST 2007


  User: ovidiu  
  Date: 07/01/23 04:39:02

  Modified:    src/main/org/jboss/remoting      Tag: remoting_2_x
                        ConnectionValidator.java InvocationRequest.java
                        InvocationResponse.java
                        MicroRemoteClientInvoker.java ServerInvoker.java
  Log:
  various reformatting and logging improvments
  
  Revision  Changes    Path
  No                   revision
  
  
  No                   revision
  
  
  1.13.2.9  +203 -182  JBossRemoting/src/main/org/jboss/remoting/ConnectionValidator.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: ConnectionValidator.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/ConnectionValidator.java,v
  retrieving revision 1.13.2.8
  retrieving revision 1.13.2.9
  diff -u -b -r1.13.2.8 -r1.13.2.9
  --- ConnectionValidator.java	21 Jan 2007 11:24:13 -0000	1.13.2.8
  +++ ConnectionValidator.java	23 Jan 2007 09:39:02 -0000	1.13.2.9
  @@ -40,147 +40,154 @@
    */
   public class ConnectionValidator extends TimerTask implements StoppableTimerTask
   {
  +   // Constants ------------------------------------------------------------------------------------
  +
  +   private static final Logger log = Logger.getLogger(ConnectionValidator.class.getName());
  +
      /**
       * Default ping period. Value is 2 seconds.
       */
      public static final long DEFAULT_PING_PERIOD = 2000;
      
  -   private List listeners = new ArrayList();
  -   private Client client = null;
  -   private InvokerLocator locator;
  -   private Map configMap;
  -
  -   private long pingPeriod = DEFAULT_PING_PERIOD;
  +   // Static ---------------------------------------------------------------------------------------
   
  -   protected static final Logger log = Logger.getLogger(ConnectionValidator.class.getName());
      private static boolean trace = log.isTraceEnabled();
   
  -   private ClientInvoker clientInvoker;
  -   private Object lock = new Object();
  -   private volatile boolean stopped = false;
  -
  -
  -   public ConnectionValidator(Client client)
  -   {
  -      this(client, (int)DEFAULT_PING_PERIOD);
  -   }
  -
  -   public ConnectionValidator(Client client, int pingPeriod)
  +   /**
  +    * Will make $PING$ invocation on server. If sucessful, will return true. Otherwise, will throw
  +    * an exception.
  +    *
  +    * @param locator - locator for the server to ping
  +    * @param config  - any configuration needed for server
  +    * @return true if alive, false if not
  +    */
  +   public static boolean checkConnection(InvokerLocator locator, Map config) throws Throwable
      {
  -      this.client = client;
  -      this.pingPeriod = pingPeriod;
  -
  -      log.debug(this + " created");
  -   }
  +      boolean pingWorked = false;
   
  -   private void start()
  -   {
  -      if (client.getConfiguration() == null)
  +      Map configMap = null;
  +      if (config == null)
         {
            configMap = new HashMap();
         }
         else
         {
  -         configMap = new HashMap(client.getConfiguration());
  +         configMap = new HashMap(config);
         }
         configMap.put("connection_checker", "true");
         configMap.put("timeout", "1000");
         configMap.put("NumberOfRetries", "1");
  -      locator = client.getInvoker().getLocator();
  +      ClientInvoker innerClientInvoker = null;
         
         try
         {
  -         clientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
  -      }
  -      catch (Exception e)
  -      {
  -         log.error("Unable to create client invoker for locator: " + locator);
  -         throw new RuntimeException("Unable to create client invoker for locator: " + locator, e);
  -      }
  +         innerClientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
         
  -      if (!clientInvoker.isConnected())
  +         if (!innerClientInvoker.isConnected())
         {
            if (trace) { log.trace("inner client invoker not connected, connecting ..."); }
  -         clientInvoker.connect();
  +            innerClientInvoker.connect();
         }
         
  -      TimerUtil.schedule(this, pingPeriod);
  -      stopped = false;
  -
  -      log.debug(this + " started");
  +         pingWorked = doCheckConnection(innerClientInvoker);
      }
  -   
  -   public void stop()
  +      catch (Throwable throwable)
      {
  -      if (stopped)
  +         log.debug("ConnectionValidator to connect to server " +
  +            innerClientInvoker.getLocator().getProtocol() + "://" +
  +            innerClientInvoker.getLocator().getHost() + ":" +
  +            innerClientInvoker.getLocator().getPort(), throwable);
  +      }
  +      finally
         {
  -         return;
  +         if (innerClientInvoker != null)
  +         {
  +            InvokerRegistry.destroyClientInvoker(locator, configMap);
         }
  -      
  -      doStop();
      }
   
  -   public boolean cancel()
  -   {
  -      return doStop();
  +      return pingWorked;
      }
   
  -   protected boolean doStop()
  +   private static boolean doCheckConnection(ClientInvoker clientInvoker) throws Throwable
      {
  -      synchronized(lock)
  +      boolean pingWorked = false;
  +
  +      if (trace)
         {  
  -         if (!listeners.isEmpty())
  +         log.trace("ConnectionValidator pinging " +
  +               clientInvoker.getLocator().getProtocol() + "://" +
  +               clientInvoker.getLocator().getHost() + ":" +
  +               clientInvoker.getLocator().getPort());
  +      }
  +
  +      try
            {
  -            listeners.clear();
  +         // Sending null client id as don't want to trigger lease on server side. This also means
  +         // that client connection validator will NOT impact client lease, so can not depend on it
  +         // to maintain client lease with the server.
  +         InvocationRequest ir =
  +            new InvocationRequest(null, Subsystem.SELF, "$PING$", null, null, null);
  +
  +         clientInvoker.invoke(ir);
  +
  +         if (trace) { log.trace("ConnectionValidator got successful ping");}
  +
  +         pingWorked = true;
            }
  -         stopped = true;
  +      catch (Throwable throwable)
  +      {
  +         log.debug("ConnectionValidator failed to ping server " +
  +            clientInvoker.getLocator().getProtocol() + "://" +
  +            clientInvoker.getLocator().getHost() + ":" +
  +            clientInvoker.getLocator().getPort(), throwable);
         }
   
  -      if (clientInvoker != null)
  -      {
  -         InvokerRegistry.destroyClientInvoker(locator, configMap);
  +      return pingWorked;
         }
         
  -      TimerUtil.unschedule(this);
  +   // Attributes -----------------------------------------------------------------------------------
   
  -      boolean result = super.cancel();
  -      log.debug(this + " stopped, returning " + result);
  -      return result;
  -   }
  +   private Client client;
  +   private long pingPeriod;
  +   private InvokerLocator locator;
  +   private Map configMap;
  +   private List listeners;
  +   private ClientInvoker clientInvoker;
  +   private Object lock = new Object();
  +   private volatile boolean stopped;
   
  +   // Constructors ---------------------------------------------------------------------------------
   
  -   public void addConnectionListener(ConnectionListener listener)
  -   {
  -      if (listener != null)
  -      {
  -         synchronized (listeners)
  -         {
  -            if (listeners.size() == 0)
  +   public ConnectionValidator(Client client)
               {
  -               start();
  -            }
  -            listeners.add(listener);
  -         }
  -      }
  +      this(client, DEFAULT_PING_PERIOD);
      }
   
  -   public boolean removeConnectionListener(ConnectionListener listener)
  +   public ConnectionValidator(Client client, long pingPeriod)
      {
  -      boolean isRemoved = false;
  -      if (listener != null)
  -      {
  -         synchronized (listeners)
  +      this.client = client;
  +      this.pingPeriod = pingPeriod;
  +      this.listeners = new ArrayList();
  +      this.stopped = false;
  +
  +      log.debug(this + " created");
  +   }
  +
  +   // StoppableTimerTask implementation ------------------------------------------------------------
  +
  +   public void stop()
            {
  -            isRemoved = listeners.remove(listener);
  -            if (listeners.size() == 0)
  +      if (stopped)
               {
  -               stop();
  -            }
  -         }
  +         return;
         }
  -      return isRemoved;
  +
  +      doStop();
      }
   
  +   // TimerTask overrides --------------------------------------------------------------------------
  +
      /**
       * The action to be performed by this timer task.
       */
  @@ -212,136 +219,150 @@
         }
      }
   
  -   public long getPingPeriod()
  -   {
  -      if (stopped)
  +   public boolean cancel()
         {
  -         return -1;
  -      }
  -
  -      return pingPeriod;
  +      return doStop();
      }
      
  +   // Public ---------------------------------------------------------------------------------------
      
  -   private void notifyListeners(Throwable thr)
  +   public void addConnectionListener(ConnectionListener listener)
  +   {
  +      if (listener != null)
      {
  -      final Throwable t = thr;
         synchronized (listeners)
         {
  -         ListIterator itr = listeners.listIterator();
  -         while (itr.hasNext())
  +            if (listeners.size() == 0)
            {
  -            final ConnectionListener listener = (ConnectionListener) itr.next();
  -            new Thread()
  +               start();
  +            }
  +            listeners.add(listener);
  +         }
  +      }
  +   }
  +
  +   public boolean removeConnectionListener(ConnectionListener listener)
               {
  -               public void run()
  +      boolean isRemoved = false;
  +      if (listener != null)
                  {
  -                  listener.handleConnectionException(t, client);
  +         synchronized (listeners)
  +         {
  +            isRemoved = listeners.remove(listener);
  +            if (listeners.size() == 0)
  +            {
  +               stop();
                  }
  -            }.start();
            }
         }
  -      stop();
  -      listeners.clear();
  +      return isRemoved;
      }
   
  -   /**
  -    * Will make $PING$ invocation on server.  If sucessful, will return true.  Otherwise,
  -    * will throw an exception.
  -    *
  -    * @param locator - locator for the server to ping
  -    * @param config  - any configuration needed for server
  -    * @return true if alive, false if not
  -    * @throws Throwable
  -    */
  -   public static boolean checkConnection(InvokerLocator locator, Map config) throws Throwable
  +   public long getPingPeriod()
      {
  -      boolean pingWorked = false;
  +      if (stopped)
  +      {
  +         return -1;
  +      }
   
  -      Map configMap = null;
  -      if (config == null)
  +      return pingPeriod;
  +   }
  +
  +   public String toString()
  +   {
  +      InvokerLocator locator = client.getInvoker().getLocator();
  +      return "ConnectionValidator[" + locator.getProtocol() + "://" + locator.getHost() + ":" +
  +         locator.getPort() + ", pingPeriod=" + pingPeriod + " ms]";
  +   }
  +
  +   // Package protected ----------------------------------------------------------------------------
  +
  +   // Protected ------------------------------------------------------------------------------------
  +
  +   // Private --------------------------------------------------------------------------------------
  +
  +   private void start()
  +   {
  +      if (client.getConfiguration() == null)
         {
            configMap = new HashMap();
         }
         else
         {
  -         configMap = new HashMap(config);
  +         configMap = new HashMap(client.getConfiguration());
         }
         configMap.put("connection_checker", "true");
         configMap.put("timeout", "1000");
         configMap.put("NumberOfRetries", "1");
  -      ClientInvoker innerClientInvoker = null;
  +      locator = client.getInvoker().getLocator();
   
         try
         {
  -         innerClientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
  +         clientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
  +      }
  +      catch (Exception e)
  +      {
  +         log.error("Unable to create client invoker for locator: " + locator);
  +         throw new RuntimeException("Unable to create client invoker for locator: " + locator, e);
  +      }
   
  -         if (!innerClientInvoker.isConnected())
  +      if (!clientInvoker.isConnected())
            {
               if (trace) { log.trace("inner client invoker not connected, connecting ..."); }
  -            innerClientInvoker.connect();
  +         clientInvoker.connect();
            }
   
  -         pingWorked = doCheckConnection(innerClientInvoker);
  +      TimerUtil.schedule(this, pingPeriod);
  +      stopped = false;
  +
  +      log.debug(this + " started");
         }
  -      catch (Throwable throwable)
  +
  +   private boolean doStop()
         {
  -         log.debug("ConnectionValidator to connect to server " +
  -            innerClientInvoker.getLocator().getProtocol() + "://" +
  -            innerClientInvoker.getLocator().getHost() + ":" +
  -            innerClientInvoker.getLocator().getPort(), throwable);
  -      }
  -      finally
  +      synchronized(lock)
         {
  -         if (innerClientInvoker != null)
  +         if (!listeners.isEmpty())
            {
  -            InvokerRegistry.destroyClientInvoker(locator, configMap);
  +            listeners.clear();
            }
  +         stopped = true;
         }
   
  -      return pingWorked;
  +      if (clientInvoker != null)
  +      {
  +         InvokerRegistry.destroyClientInvoker(locator, configMap);
      }
      
  +      TimerUtil.unschedule(this);
      
  -   protected static boolean doCheckConnection(ClientInvoker clientInvoker) throws Throwable
  -   {
  -      boolean pingWorked = false;
  -
  -      if (trace)
  -      {
  -         log.trace("ConnectionValidator pinging " +
  -               clientInvoker.getLocator().getProtocol() + "://" +
  -               clientInvoker.getLocator().getHost() + ":" +
  -               clientInvoker.getLocator().getPort());
  +      boolean result = super.cancel();
  +      log.debug(this + " stopped, returning " + result);
  +      return result;
         }
         
  -      try
  +   private void notifyListeners(Throwable thr)
         {
  -         /**
  -          * Sending null client id as don't want to trigger lease on server side.
  -          * This also means that client connection validator will NOT impact client
  -          * lease, so can not depend on it to maintain client lease with the server.
  -          */
  -         clientInvoker.invoke(new InvocationRequest(null, Subsystem.SELF, "$PING$", null, null, null));
  -         pingWorked = true;
  -      }
  -      catch (Throwable throwable)
  +      final Throwable t = thr;
  +      synchronized (listeners)
         {
  -         log.debug("ConnectionValidator failed to ping server " +
  -            clientInvoker.getLocator().getProtocol() + "://" +
  -            clientInvoker.getLocator().getHost() + ":" +
  -            clientInvoker.getLocator().getPort(), throwable);
  +         ListIterator itr = listeners.listIterator();
  +         while (itr.hasNext())
  +         {
  +            final ConnectionListener listener = (ConnectionListener) itr.next();
  +            new Thread()
  +            {
  +               public void run()
  +               {
  +                  listener.handleConnectionException(t, client);
         }
  -
  -      return pingWorked;
  +            }.start();
      }
  -   
  -   
  -   public String toString()
  -   {
  -      InvokerLocator locator = client.getInvoker().getLocator();
  -      return "ConnectionValidator[" + locator.getProtocol() + "://" + locator.getHost() + ":" +
  -         locator.getPort() + ", pingPeriod=" + pingPeriod + " ms]";
      }
  +      stop();
  +      listeners.clear();
  +   }
  +
  +   // Inner classes --------------------------------------------------------------------------------
   
   }
  \ No newline at end of file
  
  
  
  1.3.10.2  +8 -5      JBossRemoting/src/main/org/jboss/remoting/InvocationRequest.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: InvocationRequest.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/InvocationRequest.java,v
  retrieving revision 1.3.10.1
  retrieving revision 1.3.10.2
  diff -u -b -r1.3.10.1 -r1.3.10.2
  --- InvocationRequest.java	13 Jan 2007 12:42:34 -0000	1.3.10.1
  +++ InvocationRequest.java	23 Jan 2007 09:39:02 -0000	1.3.10.2
  @@ -26,11 +26,11 @@
   import java.util.Map;
   
   /**
  - * InvocationRequest is passed to ServerInvocationHandler which encapsulates the
  - * unmarshalled method invocation parameters from the ServerInvoker.
  + * InvocationRequest is passed to ServerInvocationHandler which encapsulates the unmarshalled method
  + * invocation parameters from the ServerInvoker.
    *
    * @author <a href="mailto:jhaynie at vocalocity.net">Jeff Haynie</a>
  - * @version $Revision: 1.3.10.1 $
  + * @version $Revision: 1.3.10.2 $
    */
   //TODO: Need to remove Serializable if not going to pass InvocationRequest as the callback object -TME
   public class InvocationRequest implements Serializable
  @@ -45,7 +45,8 @@
      private Map returnPayload;
      private InvokerLocator locator;
   
  -   public InvocationRequest(String sessionId, String subsystem, Object arg, Map requestPayload, Map returnPayload, InvokerLocator locator)
  +   public InvocationRequest(String sessionId, String subsystem, Object arg,
  +                            Map requestPayload, Map returnPayload, InvokerLocator locator)
      {
         this.sessionId = sessionId;
         this.subsystem = subsystem;
  @@ -122,6 +123,8 @@
   
      public String toString()
      {
  -      return "InvocationRequest[" + Integer.toHexString(hashCode()) + "]";
  +      return "InvocationRequest[" + Integer.toHexString(hashCode()) +
  +         (subsystem != null ? ", " + subsystem : "") +
  +         (arg != null ? ", " + arg : ", EMPTY") + "]";
      }
   }
  
  
  
  1.2.10.2  +5 -4      JBossRemoting/src/main/org/jboss/remoting/InvocationResponse.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: InvocationResponse.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/InvocationResponse.java,v
  retrieving revision 1.2.10.1
  retrieving revision 1.2.10.2
  diff -u -b -r1.2.10.1 -r1.2.10.2
  --- InvocationResponse.java	13 Jan 2007 12:42:34 -0000	1.2.10.1
  +++ InvocationResponse.java	23 Jan 2007 09:39:02 -0000	1.2.10.2
  @@ -28,11 +28,11 @@
   
   /**
    * InvocationResponse is a return object from a call to a remote Server Invoker.
  - * The InvocationResponse may contain either an Exception or a result value (which may be
  - * null in the case the user returns null)
  + * The InvocationResponse may contain either an Exception or a result value (which may be null in
  + * the case the user returns null)
    *
    * @author <a href="mailto:tom.elrod at jboss.com">Tom Elrod</a>
  - * @version $Revision: 1.2.10.1 $
  + * @version $Revision: 1.2.10.2 $
    */
   public class InvocationResponse implements Serializable
   {
  @@ -73,7 +73,8 @@
   
      public String toString()
      {
  -      return "InvocationResponse[" + Integer.toHexString(hashCode()) + "]";
  +      return "InvocationResponse[" + Integer.toHexString(hashCode()) + ", " +
  +         (result == null ? "EMPTY" : result) + "]";
      }
   
   }
  
  
  
  1.7.2.9   +2 -2      JBossRemoting/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: MicroRemoteClientInvoker.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java,v
  retrieving revision 1.7.2.8
  retrieving revision 1.7.2.9
  diff -u -b -r1.7.2.8 -r1.7.2.9
  --- MicroRemoteClientInvoker.java	23 Jan 2007 05:08:21 -0000	1.7.2.8
  +++ MicroRemoteClientInvoker.java	23 Jan 2007 09:39:02 -0000	1.7.2.9
  @@ -27,7 +27,7 @@
    *
    * @author <a href="mailto:jhaynie at vocalocity.net">Jeff Haynie</a>
    * @author <a href="mailto:telrod at e2technologies.net">Tom Elrod</a>
  - * @version $Revision: 1.7.2.8 $
  + * @version $Revision: 1.7.2.9 $
    */
   public abstract class MicroRemoteClientInvoker extends AbstractInvoker implements ClientInvoker
   {
  @@ -60,7 +60,7 @@
         Object returnValue = null;
         int invokeCount = 0;
   
  -      if (trace) { log.trace(this + "(" + (++invokeCount) + ") invoking " + invocationReq + " with parameter " + invocationReq.getParameter()); }
  +      if (trace) { log.trace(this + "(" + (++invokeCount) + ") invoking " + invocationReq); }
   
         Marshaller marshaller = getMarshaller();
         UnMarshaller unmarshaller = getUnMarshaller();
  
  
  
  1.52.2.22 +18 -5     JBossRemoting/src/main/org/jboss/remoting/ServerInvoker.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: ServerInvoker.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/ServerInvoker.java,v
  retrieving revision 1.52.2.21
  retrieving revision 1.52.2.22
  diff -u -b -r1.52.2.21 -r1.52.2.22
  --- ServerInvoker.java	23 Jan 2007 05:08:21 -0000	1.52.2.21
  +++ ServerInvoker.java	23 Jan 2007 09:39:02 -0000	1.52.2.22
  @@ -65,7 +65,7 @@
    * @author <a href="mailto:tom.elrod at jboss.com">Tom Elrod</a>
    * @author <a href="mailto:ovidiu at jboss.org">Ovidiu Feodorov</a>
    *
  - * @version $Revision: 1.52.2.21 $
  + * @version $Revision: 1.52.2.22 $
    */
   public abstract class ServerInvoker extends AbstractInvoker implements ServerInvokerMBean
   {
  @@ -593,10 +593,12 @@
            Object param = invocation.getParameter();
            Object result = null;
   
  +         if (trace) { log.trace(this + " received " + param); }
  +
            // check to see if this is a is alive ping
            if ("$PING$".equals(param))
            {
  -            //if checking lease, need to update lease flag
  +            // if checking lease, need to update lease flag
               if (leaseManagement)
               {
                  //NOTE we only update the lease when we receive a PING, not for
  @@ -607,8 +609,13 @@
               // if this is an invocation ping, just pong back
               Map responseMap = new HashMap();
               responseMap.put(CLIENT_LEASE_PERIOD, new Long(leasePeriod));
  -            return new InvocationResponse(invocation.getSessionId(), new Boolean(leaseManagement),
  +
  +            InvocationResponse ir = new InvocationResponse(invocation.getSessionId(),
  +                                                           new Boolean(leaseManagement),
                                             false, responseMap);
  +
  +            if (trace) { log.trace(this + " returning " + ir); }
  +            return ir;
            }
   
            if ("$DISCONNECT$".equals(param))
  @@ -617,6 +624,8 @@
               {
                  terminateLease(invocation);
               }
  +
  +            if (trace) { log.trace(this + " returning null"); }
               return null;
            }
   
  @@ -644,6 +653,7 @@
                  // subsystem not specified, so will hope for a default one being set
                  if (!handlers.isEmpty())
                  {
  +                  if (trace) { log.trace(this + " handling invocation with no subsystem explicitely specified, using the default handler"); }
                     handler = (ServerInvocationHandler)handlers.values().iterator().next();
                  }
               }
  @@ -659,7 +669,7 @@
                  if (handler == null)
                  {
                     throw new InvalidConfigurationException(
  -                     "Can not handle invocation request for subsystem(" + subsystem + ") because " +
  +                     "Can not handle invocation request for subsystem '" + subsystem + "' because " +
                        "there are no matching ServerInvocationHandlers registered. Please add via " +
                        "xml configuration or via the Connector's addInvocationHandler() method.");
                  }
  @@ -1482,6 +1492,9 @@
         // The oneway invocation should contain the real param as it's only param in parameter array
         Object realParam = objs[0];
         invocation.setParameter(realParam);
  +
  +      if(trace) { log.trace(this + " handling oneway " + invocation); }
  +
         final InvocationRequest newInvocation = invocation;
   
         ThreadPool executor = getOnewayThreadPool();
  
  
  



More information about the jboss-cvs-commits mailing list