Mercurial > hg > release > thermostat-0.9

changeset 1095:8b19c4771db1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Gracefully shutdown agent upon encountering an error This commit moves the connection error handling in AgentApplication from the ConnectionListener to a try/catch block. Currently, if the agent fails to connect, the callback in ConnectionListener is triggered but then a ConnectionException is thrown and we never catch it. When using the service command, this ConnectionException prevents storage from shutting down properly. I have also taken the opportunity to convert the logger to a private static field, as is common elsewhere in Thermostat. Also instead of passing the CountDownLatch to our SignalHandler, I converted the SignalHandler to a non-static nested class and use the latch field directly. Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-May/006585.html
author Elliott Baron <ebaron@redhat.com>
date Fri, 17 May 2013 14:08:49 -0400
parents 51540fc0849c
children e9abee8e0f3c
files agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java
diffstat 2 files changed, 23 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Fri May 17 14:08:07 2013 -0400
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Fri May 17 14:08:49 2013 -0400
@@ -67,6 +67,7 @@
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.storage.core.Connection.ConnectionListener;
 import com.redhat.thermostat.storage.core.Connection.ConnectionStatus;
+import com.redhat.thermostat.storage.core.ConnectionException;
 import com.redhat.thermostat.storage.core.DbService;
 import com.redhat.thermostat.storage.core.DbServiceFactory;
 import com.redhat.thermostat.storage.core.Storage;
@@ -76,6 +77,8 @@
 @SuppressWarnings("restriction")
 public final class AgentApplication extends AbstractStateNotifyingCommand {
 
+    private static final Logger logger = LoggingUtils.getLogger(AgentApplication.class);
+    
     private final BundleContext bundleContext;
     private final ConfigurationCreator configurationCreator;
 
@@ -86,7 +89,6 @@
     private ServiceTracker configServerTracker;
     private MultipleServiceTracker daoTracker;
     private final ExitStatus exitStatus;
-
     private CountDownLatch shutdownLatch;
 
     public AgentApplication(BundleContext bundleContext, ExitStatus exitStatus) {
@@ -110,7 +112,6 @@
         long startTime = System.currentTimeMillis();
         configuration.setStartTime(startTime);
         
-        final Logger logger = LoggingUtils.getLogger(AgentApplication.class);
 
         final DbService dbService = dbServiceFactory.createDbService(
                 configuration.getUsername(), configuration.getPassword(),
@@ -137,11 +138,10 @@
                             break;
                         case CONNECTED:
                             logger.fine("Connected to storage");
-                            handleConnected(configServer, logger, shutdownLatch);
+                            handleConnected(configServer);
                             break;
                         case FAILED_TO_CONNECT:
-                            logger.warning("Could not connect to storage.");
-                            shutdown();
+                            // ConnectionException will be thrown
                             break;
                         default:
                             logger.warning("Unfamiliar ConnectionStatus value: " + newStatus.toString());
@@ -151,7 +151,13 @@
 
                 dbService.addConnectionListener(connectionListener);
                 logger.fine("Connecting to storage...");
-                dbService.connect();
+                
+                try {
+                    dbService.connect();
+                } catch (ConnectionException e) {
+                    logger.log(Level.SEVERE, "Could not connect to storage.", e);
+                    shutdown();
+                }
                 
                 return configServer;
             }
@@ -204,20 +210,14 @@
         }
     }
     
-    // Does not need a reference of the enclosing type so lets declare this class static
-    private static class CustomSignalHandler implements SignalHandler {
-
+    private class CustomSignalHandler implements SignalHandler {
+        
         private Agent agent;
         private ConfigurationServer configServer;
-        private Logger logger;
-        private CountDownLatch shutdownLatch;
-        
-        CustomSignalHandler(Agent agent, ConfigurationServer configServer,
-                Logger logger, CountDownLatch latch) {
+
+        public CustomSignalHandler(Agent agent, ConfigurationServer configServer) {
             this.agent = agent;
             this.configServer = configServer;
-            this.logger = logger;
-            this.shutdownLatch = latch;
         }
         
         @Override
@@ -231,13 +231,12 @@
                 ex.printStackTrace();
             }
             logger.fine("Agent stopped.");       
-            shutdownLatch.countDown();
+            shutdown();
         }
         
     }
 
-    Agent startAgent(final Logger logger, final Storage storage,
-            AgentInfoDAO agentInfoDAO, BackendInfoDAO backendInfoDAO) {
+    Agent startAgent(final Storage storage, AgentInfoDAO agentInfoDAO, BackendInfoDAO backendInfoDAO) {
         BackendRegistry backendRegistry = null;
         try {
             backendRegistry = new BackendRegistry(bundleContext);
@@ -272,8 +271,7 @@
         return agent;
     }
     
-    private void handleConnected(final ConfigurationServer configServer,
-            final Logger logger, final CountDownLatch shutdownLatch) {
+    private void handleConnected(final ConfigurationServer configServer) {
         Class<?>[] deps = new Class<?>[] {
                 Storage.class,
                 AgentInfoDAO.class,
@@ -289,11 +287,8 @@
                 BackendInfoDAO backendInfoDAO = (BackendInfoDAO) services
                         .get(BackendInfoDAO.class.getName());
 
-                final Agent agent = startAgent(logger, storage,
-                        agentInfoDAO, backendInfoDAO);
-
-                SignalHandler handler = new CustomSignalHandler(agent,
-                        configServer, logger, shutdownLatch);
+                Agent agent = startAgent(storage, agentInfoDAO, backendInfoDAO);
+                SignalHandler handler = new CustomSignalHandler(agent, configServer);
                 Signal.handle(new Signal("INT"), handler);
                 Signal.handle(new Signal("TERM"), handler);
             }
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java	Fri May 17 14:08:07 2013 -0400
+++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java	Fri May 17 14:08:49 2013 -0400
@@ -51,7 +51,6 @@
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
-import java.util.logging.Logger;
 
 import org.junit.After;
 import org.junit.Before;
@@ -178,7 +177,7 @@
         final AgentApplication agent = new AgentApplication(context,
                 exitStatus, configCreator, dbServiceFactory);
         try {
-            agent.startAgent(mock(Logger.class), null, null, null);
+            agent.startAgent(null, null, null);
         } catch (RuntimeException e) {
             assertEquals(InvalidSyntaxException.class, e.getCause().getClass());
         }
@@ -202,7 +201,7 @@
         final AgentApplication agent = new AgentApplication(context,
                 exitStatus, configCreator, dbServiceFactory);
         try {
-            agent.startAgent(mock(Logger.class), null, null, null);
+            agent.startAgent(null, null, null);
         } catch (RuntimeException e) {
             fail("Should not have thrown RuntimeException");
         }