Permalink
Browse files

[MSC-131] always remove controller from all stability monitors if con…

…troller is transitioning from REMOVING to REMOVED

 * removed StabilityStatistics.getRemovedCount() because this statistics becomes invalid when this fix is applied
 * exposed ServiceContainer.isShutdown() method to be used instead of StabilityStatistics.getRemovedCount() in AS BootstrapListener
   in order to be able to detect ServiceContainer is going down.
  • Loading branch information...
1 parent cd683ef commit 3c02e4e0d1a1f8d98b6c8e254a43e829aa94f290 @ropalka ropalka committed Mar 27, 2013
@@ -157,6 +157,11 @@ public void shutdown() {
}
/** {@inheritDoc} */
+ public boolean isShutdown() {
+ throw new UnsupportedOperationException();
+ }
+
+ /** {@inheritDoc} */
public boolean isShutdownComplete() {
throw new UnsupportedOperationException();
}
@@ -30,13 +30,21 @@
* A service container which manages a set of running services.
*
* @author <a href="mailto:david.lloyd@redhat.com">David M. Lloyd</a>
+ * @author <a href="mailto:ropalka@redhat.com">Richard Opalka</a>
*/
public interface ServiceContainer extends ServiceTarget, ServiceRegistry {
/**
* Stop all services within this container.
*/
void shutdown();
+
+ /**
+ * Whether container have been shut down.
+ *
+ * @return {@code true} if container is shutting down
+ */
+ boolean isShutdown();
/**
* Determine whether the container is completely shut down.
@@ -499,7 +499,7 @@ public ServiceRegistry getServiceRegistry() {
return this;
}
- boolean isShutdown() {
+ public boolean isShutdown() {
return down;
}
@@ -682,6 +682,9 @@ void transition(final ArrayList<Runnable> tasks) {
case REMOVING_to_REMOVED: {
getListenerTasks(transition, tasks);
listeners.clear();
+ for (final StabilityMonitor monitor : monitors) {
+ monitor.removeControllerNoCallback(this);
+ }
break;
}
case REMOVING_to_DOWN: {
@@ -1453,7 +1456,6 @@ String dumpServiceDetails() {
void addMonitor(final StabilityMonitor stabilityMonitor) {
assert !holdsLock(this);
synchronized (this) {
- final Substate state = this.state;
if (monitors.add(stabilityMonitor) && !isStableRestState()) {
stabilityMonitor.incrementUnstableServices();
if (state == Substate.START_FAILED) {
@@ -105,10 +105,11 @@ public void addController(final ServiceController<?> controller) {
if (controller == null) return;
final ServiceControllerImpl<?> serviceController = (ServiceControllerImpl<?>) controller;
synchronized (controllersLock) {
- controllers.add(serviceController);
- // It is safe to call controller.addMonitor() under controllersLock because
- // controller.addMonitor() may callback only stabilityLock protected methods.
- serviceController.addMonitor(this);
+ if (controllers.add(serviceController)) {
+ // It is safe to call controller.addMonitor() under controllersLock because
+ // controller.addMonitor() may callback only stabilityLock protected methods.
+ serviceController.addMonitor(this);
+ }
}
}
@@ -132,10 +133,22 @@ public void removeController(final ServiceController<?> controller) {
if (controller == null) return;
final ServiceControllerImpl<?> serviceController = (ServiceControllerImpl<?>) controller;
synchronized (controllersLock) {
- // It is safe to call controller.addMonitor() under controllersLock because
- // controller.addMonitor() may callback only stabilityLock protected methods.
- serviceController.removeMonitor(this);
- controllers.remove(serviceController);
+ if (controllers.remove(serviceController)) {
+ // It is safe to call controller.removeMonitor() under controllersLock because
+ // controller.removeMonitor() may callback only stabilityLock protected methods.
+ serviceController.removeMonitor(this);
+ }
+ }
+ }
+
+ /**
+ * Unregister controller with this monitor but don't call serviceController.removeMonitor() at all.
+ *
+ * @param controller to be unregistered from stability detection.
+ */
+ void removeControllerNoCallback(final ServiceControllerImpl<?> controller) {
+ synchronized (controllersLock) {
+ controllers.remove(controller);
}
}
@@ -379,15 +392,14 @@ private void provideStatistics(final int failedCount, final int problemsCount, f
controllers = this.controllers.clone();
}
// collect statistics
- int active = 0, lazy = 0, onDemand = 0, never = 0, passive = 0, started = 0, remove = 0;
+ int active = 0, lazy = 0, onDemand = 0, never = 0, passive = 0, started = 0;
for (final ServiceController<?> controller : controllers) {
if (controller.getState() == UP) started++;
if (controller.getMode() == ACTIVE) active++;
else if (controller.getMode() == PASSIVE) passive++;
else if (controller.getMode() == ON_DEMAND) onDemand++;
else if (controller.getMode() == NEVER) never++;
else if (controller.getMode() == LAZY) lazy++;
- else if (controller.getMode() == REMOVE) remove++;
}
// report statistics
statistics.setActiveCount(active);
@@ -398,6 +410,5 @@ private void provideStatistics(final int failedCount, final int problemsCount, f
statistics.setPassiveCount(passive);
statistics.setProblemsCount(problemsCount);
statistics.setStartedCount(started);
- statistics.setRemovedCount(remove);
}
}
@@ -35,7 +35,6 @@
* <li>count of controllers in <b>PASSIVE</b> mode - see method {@link #getPassiveCount()}</li>
* <li>count of controllers that had <b>PROBLEM</b> to start - see method {@link #getProblemsCount()}</li>
* <li>count of controllers in <b>UP</b> state - see method {@link #getStartedCount()}</li>
- * <li>count of controllers in <b>REMOVE</b> mode - see method {@link #getRemovedCount()}</li>
* </ul>
*
* Sample usage:
@@ -59,7 +58,6 @@
private int passive;
private int problems;
private int started;
- private int removed;
/**
* Returns count of controllers registered with {@link StabilityMonitor} that are in
@@ -133,15 +131,6 @@ public int getStartedCount() {
return started;
}
- /**
- * Returns count of controllers registered with {@link StabilityMonitor} that are in
- * {@link ServiceController.Mode#REMOVE} mode.
- * @return count of <b>REMOVE</b> controllers
- */
- public int getRemovedCount() {
- return removed;
- }
-
void setActiveCount(final int count) {
active = count;
}
@@ -173,8 +162,4 @@ void setProblemsCount(final int count) {
void setStartedCount(final int count) {
started = count;
}
-
- void setRemovedCount(final int count) {
- removed = count;
- }
}
@@ -153,7 +153,6 @@ public Object getValue() throws IllegalStateException, IllegalArgumentException
assertTrue(statistics.getNeverCount() == 0);
assertTrue(statistics.getPassiveCount() == 0);
assertTrue(statistics.getProblemsCount() == 0);
- assertTrue(statistics.getRemovedCount() == 0);
stabilityMonitor.clear();
try {
stabilityMonitor.awaitStability(statistics);
@@ -168,7 +167,6 @@ public Object getValue() throws IllegalStateException, IllegalArgumentException
assertTrue(statistics.getNeverCount() == 0);
assertTrue(statistics.getPassiveCount() == 0);
assertTrue(statistics.getProblemsCount() == 0);
- assertTrue(statistics.getRemovedCount() == 0);
}
@Test
@@ -223,7 +221,6 @@ public Object getValue() throws IllegalStateException, IllegalArgumentException
assertTrue(statistics.getNeverCount() == 0);
assertTrue(statistics.getPassiveCount() == 0);
assertTrue(statistics.getProblemsCount() == 0);
- assertTrue(statistics.getRemovedCount() == 0);
stabilityMonitor.clear();
try {
stabilityMonitor.awaitStability(statistics);
@@ -238,6 +235,5 @@ public Object getValue() throws IllegalStateException, IllegalArgumentException
assertTrue(statistics.getNeverCount() == 0);
assertTrue(statistics.getPassiveCount() == 0);
assertTrue(statistics.getProblemsCount() == 0);
- assertTrue(statistics.getRemovedCount() == 0);
}
}

0 comments on commit 3c02e4e

Please sign in to comment.