BUG-7222: Improve ClusterSingletonService error handling.

The applications that are managed by singleton service
may throw an unchecked exception while handling the calls from
the service. We also need to handle these so that they don't prevent
other services from being started/stopped.

Change-Id: I711b8c1e8b2fcb8e9a9699ba96fa3b0c2daff594
Signed-off-by: Tomas Cere <tcere@cisco.com>

diff --git a/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java b/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
index f49dd5c67f1fe3a904522774d2dbd185b94ab429..0d4920bed3da77739960b054104b817382a0fbaf 100644 (file)
--- a/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
+++ b/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
@@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
+import org.opendaylight.mdsal.eos.common.api.CandidateAlreadyRegisteredException;
 import org.opendaylight.mdsal.eos.common.api.EntityOwnershipChangeState;
 import org.opendaylight.mdsal.eos.common.api.GenericEntity;
 import org.opendaylight.mdsal.eos.common.api.GenericEntityOwnershipCandidateRegistration;
@@ -118,7 +119,12 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             final List<ListenableFuture<Void>> serviceCloseFutureList = new ArrayList<>();
             if (hasOwnership) {
                 for (final ClusterSingletonServiceRegistrationDelegator service : serviceGroup) {
-                    serviceCloseFutureList.add(service.closeServiceInstance());
+                    try {
+                        serviceCloseFutureList.add(service.closeServiceInstance());
+                    } catch (final RuntimeException e) {
+                        LOG.warn("Unexpected exception while closing service: {}, resuming with next..",
+                                service.getIdentifier());
+                    }
                 }
                 hasOwnership = false;
             }
@@ -142,7 +148,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             Verify.verify(!hasOwnership);
             Verify.verify(serviceEntityCandidateReg == null);
             serviceEntityCandidateReg = entityOwnershipService.registerCandidate(serviceEntity);
-        } catch (final Exception e) {
+        } catch (final RuntimeException | InterruptedException | CandidateAlreadyRegisteredException e) {
             LOG.debug("Unexpected error by registration service Provider {}", clusterSingletonGroupIdentifier, e);
             needCloseProviderInstance = true;
             throw new RuntimeException(e);
@@ -168,7 +174,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             if (hasOwnership) {
                 service.instantiateServiceInstance();
             }
-        } catch (final Exception e) {
+        } catch (final RuntimeException | InterruptedException e) {
             LOG.debug("Unexpected error by registration service Provider {}", clusterSingletonGroupIdentifier, e);
             needCloseProviderInstance = true;
             throw new RuntimeException(e);
@@ -197,7 +203,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             } else {
                 needCloseProviderInstance = true;
             }
-        } catch (final Exception e) {
+        } catch (final RuntimeException | InterruptedException e) {
             LOG.debug("Unexpected error by registration service Provider {}", clusterSingletonGroupIdentifier, e);
             needCloseProviderInstance = true;
             throw new RuntimeException(e);
@@ -302,7 +308,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             } else {
                 LOG.debug("Service {} is closed, so don't take leadership", clusterSingletonGroupIdentifier);
             }
-        } catch (final Exception e) {
+        } catch (final RuntimeException | InterruptedException e) {
             LOG.error("Unexpected exception state for service Provider {} in TakeLeadership",
                     clusterSingletonGroupIdentifier, e);
             needCloseProviderInstance = true;
@@ -328,7 +334,12 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
             if (hasOwnership) {
                 Verify.verify(asyncCloseEntityCandidateReg != null);
                 for (final ClusterSingletonServiceRegistrationDelegator service : serviceGroup) {
-                    serviceCloseFutureList.add(service.closeServiceInstance());
+                    try {
+                        serviceCloseFutureList.add(service.closeServiceInstance());
+                    } catch (final RuntimeException e) {
+                        LOG.error("Unexpected exception while closing service: {}, resuming with next..",
+                                service.getIdentifier());
+                    }
                 }
                 hasOwnership = false;
             }
@@ -349,7 +360,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
              * instance has fully closed prior to relinquishing service ownership.
              */
             needReleaseLock = false;
-        } catch (final Exception e) {
+        } catch (final InterruptedException e) {
             LOG.error("Unexpected exception state for service Provider {} in LostLeadership",
                     clusterSingletonGroupIdentifier, e);
             needCloseProviderInstance = true;