Mercurial > hg > release > thermostat-0.13
changeset 1095:8b19c4771db1
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"); }