InstructionScheduler should be a Registration

Unregistration is implied as not being possible to fail in its only
implementation. Exposed that to users so they know they do not have
to worry about exceptions.

JIRA: BGPCEP-983
Change-Id: I3edb6bf26a732798536d536c42374f27343a0f35
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
index 574ae9b694346ad7f056ce070aa0a4117d35e9d1..722eff13293980654151164c4ce1509e97636d11 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
@@ -12,7 +12,9 @@ import static java.util.Objects.requireNonNull;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.lock.qual.Holding;
 import org.eclipse.jdt.annotation.Nullable;
@@ -162,7 +164,6 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
         LOG.info("PCEP Topology Deployer closed");
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
     private static void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) {
         if (topology == null) {
             return;
@@ -170,10 +171,10 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
         LOG.info("Removing Topology {}", topologyId);
         try {
             topology.closeServiceInstance().get(TIMEOUT_NS, TimeUnit.NANOSECONDS);
-            topology.close();
-        } catch (final Exception e) {
+        } catch (final InterruptedException | TimeoutException | ExecutionException e) {
             LOG.error("Topology {} instance failed to close service instance", topologyId, e);
         }
+        topology.close();
     }
 
     private static boolean filterPcepTopologies(final @Nullable TopologyTypes topologyTypes) {

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderBean.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderBean.java
index 83cbfe361eec9cdf6c8bcf447072e7382a0174ad..9555c3bc31549d5388fab647fbc610fbb612adb9 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderBean.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderBean.java
@@ -70,7 +70,7 @@ final class PCEPTopologyProviderBean implements PCEPTopologyProviderDependencies
     }
 
     @Override
-    public synchronized void close() throws Exception {
+    public synchronized void close() {
         if (pcepTopoProviderCSS != null) {
             pcepTopoProviderCSS.close();
             pcepTopoProviderCSS = null;

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderSingleton.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderSingleton.java
index 31846f56fe45b3329d6217a8b2ebc865ff345ac0..34c209dc35f6b56e6ea2a448e7fcc27afdea87a6 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderSingleton.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderSingleton.java
@@ -53,13 +53,6 @@ final class PCEPTopologyProviderSingleton implements ClusterSingletonService, Au
         cssRegistration = cssp.registerClusterSingletonService(this);
     }
 
-    @SuppressModernizer
-    private static Dictionary<String, String> props(final PCEPTopologyConfiguration configDependencies) {
-        final Dictionary<String, String> properties = new Hashtable<>();
-        properties.put(PCEPTopologyProvider.class.getName(), configDependencies.getTopologyId().getValue());
-        return properties;
-    }
-
     @Override
     @SuppressWarnings("checkstyle:IllegalCatch")
     public synchronized void instantiateServiceInstance() {
@@ -88,7 +81,7 @@ final class PCEPTopologyProviderSingleton implements ClusterSingletonService, Au
     }
 
     @Override
-    public synchronized void close() throws Exception {
+    public synchronized void close() {
         if (cssRegistration != null) {
             cssRegistration.close();
             cssRegistration = null;
@@ -99,4 +92,11 @@ final class PCEPTopologyProviderSingleton implements ClusterSingletonService, Au
         }
         scheduler.close();
     }
+
+    @SuppressModernizer
+    private static Dictionary<String, String> props(final PCEPTopologyConfiguration configDependencies) {
+        final Dictionary<String, String> properties = new Hashtable<>();
+        properties.put(PCEPTopologyProvider.class.getName(), configDependencies.getTopologyId().getValue());
+        return properties;
+    }
 }
\ No newline at end of file

diff --git a/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionScheduler.java b/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionScheduler.java
index d96c9fd89d8712318324b246e9d26318d2e26405..9ca8facad395db7554217d9cfdb44d87f66c1afb 100644 (file)
--- a/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionScheduler.java
+++ b/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionScheduler.java
@@ -12,8 +12,9 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.singleton.common.api.ServiceGroupIdentifier;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.programming.rev150720.SubmitInstructionInput;
 import org.opendaylight.yangtools.concepts.Identifiable;
+import org.opendaylight.yangtools.concepts.Registration;
 
-public interface InstructionScheduler extends Identifiable<ServiceGroupIdentifier>, AutoCloseable {
+public interface InstructionScheduler extends Identifiable<ServiceGroupIdentifier>, Registration {
     /**
      * Schedule a new instruction for execution. This method tries to enqueue an instruction. It will return a Future
      * which represents the scheduling progress. When the future becomes successful, the requestor is expected to start

diff --git a/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionSchedulerFactory.java b/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionSchedulerFactory.java
index 14e7061058b8f233edafa2931dc6fd9b36cada11..8985769b98ab5b996089db073d8fbb746ea942bb 100644 (file)
--- a/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionSchedulerFactory.java
+++ b/programming/spi/src/main/java/org/opendaylight/bgpcep/programming/spi/InstructionSchedulerFactory.java
@@ -5,7 +5,6 @@
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  */
-
 package org.opendaylight.bgpcep.programming.spi;
 
 /**
@@ -15,10 +14,12 @@ public interface InstructionSchedulerFactory {
     /**
      * Creates a unique InstructionScheduler.
      *
-     * @param instructionId Unique Identifier, also to be used as part of
-     *                      Cluster Singleton Service Group Identifier
+     * @param instructionId Unique Identifier, also to be used as part of Cluster Singleton Service Group Identifier
      *                      as "instructionId"-service-group
      * @return InstructionScheduler
      */
+    // FIXME: this has weird lifecycle implications. We really should be exposing some sort of an ObjectRegistration
+    //        object, which exposes the SGI and allows unregistration, so that InstructionScheduler itself is in no way
+    //        tied to its lifecycle and/or Cluster Singleton Service
     InstructionScheduler createInstructionScheduler(String instructionId);
 }