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

Ovidiu Feodorov ovidiu.feodorov at jboss.com
Wed Jan 10 05:30:33 EST 2007


  User: ovidiu  
  Date: 07/01/10 05:30:33

  Modified:    src/main/org/jboss/remoting     Tag: remoting_2_x
                        ConnectionValidator.java InvokerRegistry.java
                        LeasePinger.java MicroRemoteClientInvoker.java
  Log:
  Proposed fix for http://jira.jboss.org/jira/browse/JBREM-662
  Even if functionality changes will be refactored, I would kindly request
  that the extra log statements to be left in place.
  
  Revision  Changes    Path
  No                   revision
  
  
  No                   revision
  
  
  1.13.2.3  +52 -10    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.2
  retrieving revision 1.13.2.3
  diff -u -b -r1.13.2.2 -r1.13.2.3
  --- ConnectionValidator.java	12 Dec 2006 14:41:16 -0000	1.13.2.2
  +++ ConnectionValidator.java	10 Jan 2007 10:30:33 -0000	1.13.2.3
  @@ -35,6 +35,7 @@
   
   /**
    * @author <a href="mailto:tom.elrod at jboss.com">Tom Elrod</a>
  + * @author <a href="mailto:ovidiu at jboss.org">Ovidiu Feodorov</a>
    */
   public class ConnectionValidator extends TimerTask
   {
  @@ -44,6 +45,7 @@
      private long pingPeriod = DEFAULT_PING_PERIOD;
   
      protected static final Logger log = Logger.getLogger(ConnectionValidator.class.getName());
  +   private static boolean trace = log.isTraceEnabled();
   
      private Object lock = new Object();
      private boolean stopped = false;
  @@ -55,19 +57,23 @@
   
      public ConnectionValidator(Client client)
      {
  -      this.client = client;
  +      this(client, (int)DEFAULT_PING_PERIOD);
      }
   
      public ConnectionValidator(Client client, int pingPeriod)
      {
         this.client = client;
         this.pingPeriod = pingPeriod;
  +
  +      log.debug(this + " created");
      }
   
      private void start()
      {
         TimerUtil.schedule(this, pingPeriod);
         stopped = false;
  +
  +      log.debug(this + " started");
      }
   
      public void stop()
  @@ -81,6 +87,8 @@
            }
            stopped = true;
         }
  +
  +      log.debug(this + " stopped");
      }
   
   
  @@ -127,14 +135,21 @@
            {
               try
               {
  -               boolean isValid = checkConnection(client.getInvoker().getLocator(), client.getConfiguration());
  +               if (trace) { log.trace(this + " pinging ..."); }
  +
  +               boolean isValid =
  +                  checkConnection(client.getInvoker().getLocator(), client.getConfiguration());
  +
                  if (!isValid)
                  {
  -                  notifyListeners(new Exception("Could not connect to server."));
  +                  log.debug(this + "'s connections is invalid");
  +
  +                  notifyListeners(new Exception("Could not connect to server!"));
                  }
               }
               catch (Throwable thr)
               {
  +               log.debug(this + " got throwable while pinging", thr);
                  notifyListeners(thr);
               }
            }
  @@ -188,37 +203,64 @@
   
         try
         {
  -
            innerClientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
   
            if (!innerClientInvoker.isConnected())
            {
  +            if (trace) { log.trace("inner client invoker not connected, connecting ..."); }
               innerClientInvoker.connect();
            }
  +
  +         if (trace)
  +         {
  +            log.trace("ConnectionValidator pinging " +
  +               innerClientInvoker.getLocator().getProtocol() + "://" +
  +               innerClientInvoker.getLocator().getHost() + ":" +
  +               innerClientInvoker.getLocator().getPort());
  +         }
  +
            /**
             * 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.
             */
  -         Object o = innerClientInvoker.invoke(new InvocationRequest(null, Subsystem.SELF,
  -                                                                    "$PING$", null, null, null));
  +         innerClientInvoker.
  +            invoke(new InvocationRequest(null, Subsystem.SELF, "$PING$", null, null, null));
  +
            pingWorked = true;
         }
         catch (Throwable throwable)
         {
  -         log.debug("ConnectionValidator could not successfully ping server (" + innerClientInvoker.getLocator());
  +         log.debug("ConnectionValidator failed to ping server " +
  +            innerClientInvoker.getLocator().getProtocol() + "://" +
  +            innerClientInvoker.getLocator().getHost() + ":" +
  +            innerClientInvoker.getLocator().getPort(), throwable);
         }
         finally
         {
            if (innerClientInvoker != null)
            {
  +            if (pingWorked)
  +            {
               InvokerRegistry.destroyClientInvoker(locator, configMap);
            }
  +            else
  +            {
  +               // need to get rid of the client invoker that keeps spinning
  +               InvokerRegistry.reallyDestroyClientInvoker(locator, configMap);
  +            }
  +         }
         }
   
         return pingWorked;
  -
      }
   
   
  +   public String toString()
  +   {
  +      InvokerLocator locator = client.getInvoker().getLocator();
  +      return "ConnectionValidator[" + locator.getProtocol() + "://" + locator.getHost() + ":" +
  +         locator.getPort() + ", pingPeriod=" + pingPeriod + " ms]";
  +   }
  +
   }
  \ No newline at end of file
  
  
  
  1.34.4.1  +98 -51    JBossRemoting/src/main/org/jboss/remoting/InvokerRegistry.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: InvokerRegistry.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/InvokerRegistry.java,v
  retrieving revision 1.34
  retrieving revision 1.34.4.1
  diff -u -b -r1.34 -r1.34.4.1
  --- InvokerRegistry.java	20 Jul 2006 16:58:31 -0000	1.34
  +++ InvokerRegistry.java	10 Jan 2007 10:30:33 -0000	1.34.4.1
  @@ -48,13 +48,15 @@
    *
    * @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.34 $
  + * @version $Revision: 1.34.4.1 $
    */
   public class InvokerRegistry
   {
   
      private static final Logger log = Logger.getLogger(InvokerRegistry.class);
   
  +   private static boolean isTraceEnabled = log.isTraceEnabled();
  +
      private static final Map clientLocators = new HashMap();
      private static final Map serverLocators = new HashMap();
   
  @@ -67,8 +69,6 @@
   
      /**
       * return an array of InvokerLocators that are local to this VM (server invokers)
  -    *
  -    * @return
       */
      public static synchronized final InvokerLocator[] getRegisteredServerLocators()
      {
  @@ -80,7 +80,6 @@
       * a compatible transport
       *
       * @param remote
  -    * @return
       */
      public static synchronized InvokerLocator getSuitableServerLocatorForRemote(InvokerLocator remote)
      {
  @@ -99,8 +98,6 @@
   
      /**
       * return an array of String of the registered transports
  -    *
  -    * @return
       */
      public static final String[] getRegisteredInvokerTransports()
      {
  @@ -114,8 +111,6 @@
   
      /**
       * return an array of ClientInvokers that are connected
  -    *
  -    * @return
       */
      public static final ClientInvoker[] getClientInvokers()
      {
  @@ -147,8 +142,6 @@
   
      /**
       * return an array of ServerInvokers that are connected
  -    *
  -    * @return
       */
      public static final ServerInvoker[] getServerInvokers()
      {
  @@ -198,7 +191,6 @@
       * returns true if the client invoker is registered in the local JVM for a given locator
       *
       * @param locator
  -    * @return
       */
      public static boolean isClientInvokerRegistered(InvokerLocator locator)
      {
  @@ -209,19 +201,27 @@
      }
   
      /**
  -    * called to destroy any cached RemoteClientInvoker copies inside the registry. this method
  +    * Called to destroy any cached RemoteClientInvoker copies inside the registry. This method
       * must be called when it is determined that a remote server (via the locator) is no
       * longer available.
  -    *
  -    * @param locator
       */
      public static void destroyClientInvoker(InvokerLocator locator, Map configuration)
      {
         synchronized(clientLock)
         {
  +         if (isTraceEnabled)
  +         {
  +            log.trace("destroying client invoker " + locator + ", config " + configuration);
  +         }
  +
            ClientInvoker invoker = decrementClientInvokerCounter(locator, configuration);
  +
            if(invoker != null)
            {
  +            if (isTraceEnabled)
  +            {
  +               log.trace("disconnecting " + invoker);
  +            }
               invoker.disconnect();
               invoker = null;
            }
  @@ -229,6 +229,43 @@
      }
   
      /**
  +    * (Temporary) method that fixes http://jira.jboss.org/jira/browse/JBREM-662.
  +    * Must be refactored (renamed, to start with).
  +    */
  +   public static void reallyDestroyClientInvoker(InvokerLocator locator, Map configuration)
  +   {
  +      synchronized(clientLock)
  +      {
  +         if (isTraceEnabled)
  +         {
  +            log.trace("really destroying client invoker " + locator + ", config " + configuration);
  +         }
  +
  +         // loop until last reference; really dangerous if the invoker is not in registry, because
  +         // it will loop forever. decrementClientInvokerCounter() semantics must change to clearly
  +         // indicate when it was a invoker and when it wasn't an invoker in registry.
  +
  +         int loops = 0;
  +         ClientInvoker invoker = null;
  +         while((invoker = decrementClientInvokerCounter(locator, configuration)) == null)
  +         {
  +            // loop, dangerous, a precautionary hack
  +            if (++loops == 50)
  +            {
  +               throw new IllegalStateException("got into a hopeless loop!");
  +            }
  +         }
  +
  +         if (isTraceEnabled)
  +         {
  +            log.trace("disconnecting " + invoker);
  +         }
  +         invoker.disconnect();
  +         invoker = null;
  +      }
  +   }
  +
  +   /**
        * create a ClientInvoker instance, using the specific InvokerLocator, which is just a client-side
       * invoker to a remote server.  Will use the default configuration values for the transport.
       *
  @@ -363,7 +400,6 @@
       * Note, this will also increment the internal reference count for the invoker
       * @param locator
       * @param configuration
  -    * @return
       */
      private static ClientInvoker getRegisteredClientInvoker(InvokerLocator locator, Map configuration)
      {
  @@ -480,7 +516,6 @@
       * returns true if the server invoker is registered in the local JVM for a given locator/handler pair
       *
       * @param locator
  -    * @return
       */
      public static boolean isServerInvokerRegistered(InvokerLocator locator)
      {
  @@ -547,11 +582,17 @@
   
      private static ClientInvoker decrementClientInvokerCounter(InvokerLocator locator, Map configuration)
      {
  -      ClientInvoker clientInvoker = null;
  -      ClientInvokerHolder holder = null;
  -      List holderList = (List) clientLocators.get(locator);
  -      if (holderList != null)
  +      List holderList = (List)clientLocators.get(locator);
  +
  +      if (holderList == null)
         {
  +         log.debug("Could not decrement client invoker counter for locator " + locator +
  +            " as does not exist in invoker registry.");
  +         return null;
  +      }
  +
  +      ClientInvokerHolder holder = null;
  +
            // now look for specific invoker by configuration map
            for(int x = 0; x < holderList.size(); x++)
            {
  @@ -573,30 +614,31 @@
               }
            }
   
  -         if(holder != null)
  +      if (holder == null)
            {
  +         log.debug("Could not decrement client invoker counter for locator " + locator +
  +                   "as does not exist in invoker registry with matching configuraion map.");
  +         return null;
  +      }
  +
  +      ClientInvoker clientInvoker =  null;
               holder.decrementCount();
  +
               if(holder.getCount() == 0)
               {
  -               // is the last reference to invoker
  -               // need to remove all references
                  clientInvoker = holder.getClientInvoker();
                  holderList.remove(holder);
                  if(holderList.isEmpty())
                  {
                     clientLocators.remove(locator);
                  }
  -            }
  -         }
  -         else
  -         {
  -            log.debug("Could not remove client invoker for locator (" + locator + ") " +
  -                      "as does not exist in invoker registry with matching configuraion map.");
  -         }
  +
  +         log.debug("removed " + clientInvoker + " from registry");
         }
         else
         {
  -         log.debug("Could not remove client invoker for locator (" + locator + ") as does not exist in invoker registry.");
  +         log.debug("decremented " + holder.getClientInvoker() +
  +            "'s count, current count " + holder.getCount());
         }
   
         return clientInvoker;
  @@ -673,6 +715,11 @@
         return isSSLSupported;
      }
   
  +   public String toString()
  +   {
  +      return "InvokerRegistry[" + Integer.toHexString(hashCode()) + "]";
  +   }
  +
      private static class ClientInvokerHolder
      {
         private ClientInvoker invoker = null;
  
  
  
  1.8.2.2   +11 -11    JBossRemoting/src/main/org/jboss/remoting/LeasePinger.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: LeasePinger.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/LeasePinger.java,v
  retrieving revision 1.8.2.1
  retrieving revision 1.8.2.2
  diff -u -b -r1.8.2.1 -r1.8.2.2
  --- LeasePinger.java	12 Dec 2006 03:48:04 -0000	1.8.2.1
  +++ LeasePinger.java	10 Jan 2007 10:30:33 -0000	1.8.2.2
  @@ -41,7 +41,7 @@
      {
         if(isTraceEnabled)
         {
  -         log.trace("Starting lease timer for client invoker (session id " + invokerSessionId + ") with ping period of " + pingPeriod);
  +         log.trace("Starting lease timer for client invoker with session ID " + invokerSessionId + " with ping period of " + pingPeriod);
         }
         timerTask = new LeaseTimerTask();
         TimerUtil.schedule(timerTask, pingPeriod);
  @@ -51,7 +51,7 @@
      {
         if(isTraceEnabled)
         {
  -         log.trace("Stopping lease timer for client invoker (session id " + invokerSessionId + ")");
  +         log.trace("Stopping lease timer for client invoker with session ID " + invokerSessionId);
         }
         if (timerTask != null)
         {
  @@ -90,7 +90,7 @@
                        clientSessionIds = clientSessionIds + h.getSessionId() + "\n";
                     }
                  }
  -               log.trace("Sending ping to server for client invoker (session id " + invokerSessionId + ".  " +
  +               log.trace("Sending ping to server for client invoker with session ID " + invokerSessionId + ".  " +
                            "Currently managing lease for following clients:\n" + clientSessionIds);
               } // end trace
   
  @@ -102,7 +102,7 @@
            }
            catch (Throwable throwable)
            {
  -            log.warn("Error sending lease ping to server for client invoker (session id " + invokerSessionId + ".");
  +            log.warn("Error sending lease ping to server for client invoker with session ID " + invokerSessionId);
            }
         }
      }
  @@ -116,8 +116,8 @@
   
         if(isTraceEnabled)
         {
  -         log.trace("Adding new client to lease for client invoker (session id " + invokerSessionId + ") where " +
  -                   "client session id is " + sessionId + " and lease period is " + leasePeriod);
  +         log.trace("Adding new client to lease for client invoker with session ID " + invokerSessionId + " where " +
  +                   "client session ID is " + sessionId + " and lease period is " + leasePeriod);
         }
         ClientHolder newClient = new ClientHolder(sessionId, configuration, leasePeriod);
         clients.put(sessionId, newClient);
  @@ -144,7 +144,7 @@
   
         if(isTraceEnabled)
         {
  -         log.trace("Removing client (session id " + sessionId + ") from lease for client invoker (session id " + invokerSessionId + ")");
  +         log.trace("Removing client with session ID " + sessionId + " from lease for client invoker with session ID " + invokerSessionId);
         }
         ClientHolder holder = (ClientHolder) clients.remove(sessionId);
         if (holder != null)
  @@ -157,17 +157,17 @@
               client.invoke(new InvocationRequest(invokerSessionId, null, "$DISCONNECT$", clientMap, null, null));
               if(isTraceEnabled)
               {
  -               log.trace("Sent out disconnect message to server for lease tied to client session id " + sessionId);
  +               log.trace("Sent out disconnect message to server for lease tied to client session ID " + sessionId);
               }
            }
            catch (Throwable throwable)
            {
  -            log.warn("Error sending disconnect for client lease where client session id is " + sessionId);
  +            log.warn("Error sending disconnect for client lease where client session ID is " + sessionId);
            }
         }
         else
         {
  -         log.warn("Tried to remove lease for client (session id " + sessionId + "), but did not exist for client invoker lease (session id " + invokerSessionId);
  +         log.warn("Tried to remove lease for client with session ID " + sessionId + ", but did not exist for client invoker lease (session ID " + invokerSessionId + ")");
         }
   
         if (clients.isEmpty())
  @@ -175,7 +175,7 @@
            isLastClientLease = true;
            if(isTraceEnabled)
            {
  -            log.trace("There are no more client leases tied to this client invoker's lease (session id " + invokerSessionId);
  +            log.trace("There are no more client leases tied to this client invoker's lease (session ID " + invokerSessionId + ")");
            }
         }
         else
  
  
  
  1.7.2.1   +9 -4      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
  retrieving revision 1.7.2.1
  diff -u -b -r1.7 -r1.7.2.1
  --- MicroRemoteClientInvoker.java	21 Sep 2006 18:35:26 -0000	1.7
  +++ MicroRemoteClientInvoker.java	10 Jan 2007 10:30:33 -0000	1.7.2.1
  @@ -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 $
  + * @version $Revision: 1.7.2.1 $
    */
   public abstract class MicroRemoteClientInvoker extends AbstractInvoker implements ClientInvoker
   {
  @@ -67,7 +67,7 @@
   
         if (isTraceEnabled)
         {
  -         log.trace((++invokeCount) + ") invoking =>" + invocationReq + " with parameter: " + invocationReq.getParameter());
  +         log.trace(this + "(" + (++invokeCount) + ") invoking =>" + invocationReq + " with parameter: " + invocationReq.getParameter());
         }
   
         Marshaller marshaller = getMarshaller();
  @@ -288,10 +288,10 @@
       */
      public synchronized void disconnect()
      {
  +      if (isTraceEnabled) { log.trace(this + " disconnecting ..."); }
  +
         if (connected)
         {
  -         log.debug("disconnect called for: " + this);
  -
            connected = false;
            handleDisconnect();
            ClassLoader classLoader = getClassLoader();
  @@ -299,6 +299,11 @@
            {
               ((ClassByteClassLoader) classbyteloader).destroy();
            }
  +         if (isTraceEnabled) { log.trace(this + " disconnected"); }
  +      }
  +      else
  +      {
  +         if (isTraceEnabled) { log.trace(this + " is not connected!"); }
         }
      }
   
  
  
  



More information about the jboss-cvs-commits mailing list