Use a global Timer

We should not be instantiating a timer for each ServerSessionManager,
but rather keep a single instance for all of them. This allows us to
reuse the timer for other things as well.

Change-Id: I0ce8489d4fdba6c9e4d37d515462b0b15481afc1
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/PCEPTopologyProviderDependencies.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyProviderDependencies.java
index a83225a717d8bc9a06c7cbd39b8d5efa7f0c67be..2ff1f4690654ff02bcb8238add100d881a718106 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyProviderDependencies.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyProviderDependencies.java
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.bgpcep.pcep.topology.provider;
 
+import io.netty.util.Timer;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.opendaylight.bgpcep.pcep.server.PceServerProvider;
 import org.opendaylight.mdsal.binding.api.DataBroker;
@@ -52,4 +53,11 @@ interface PCEPTopologyProviderDependencies {
      * @return PceServerProvider
      */
     PceServerProvider getPceServerProvider();
+
+    /**
+     * Return the timer to use used for scheduling various timeouts.
+     *
+     * @return A Timer.
+     */
+    Timer getTimer();
 }

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyTracker.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyTracker.java
index 7a909bc4d6c05ec9829402f7ec9cc0f61af4b240..e5a8d4945ab5af9ed8d6ecf8f8793f1dd20e02b9 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyTracker.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyTracker.java
@@ -11,9 +11,15 @@ import static com.google.common.base.Verify.verifyNotNull;
 import static java.util.Objects.requireNonNull;
 import static org.opendaylight.bgpcep.pcep.topology.provider.TopologyUtils.friendlyId;
 
+import io.netty.util.HashedWheelTimer;
+import io.netty.util.Timeout;
+import io.netty.util.Timer;
+import io.netty.util.TimerTask;
 import java.util.Collection;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.TimeUnit;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.bgpcep.pcep.server.PceServerProvider;
@@ -55,6 +61,21 @@ public final class PCEPTopologyTracker
     private final @NonNull PCEPDispatcher pcepDispatcher;
     private final @NonNull DataBroker dataBroker;
 
+    // Timer used for RPC timeouts and session statistics scheduling
+    private final @NonNull HashedWheelTimer privateTimer = new HashedWheelTimer();
+    private final @NonNull Timer timer = new Timer() {
+        @Override
+        public Timeout newTimeout(final TimerTask task, final long delay, final TimeUnit unit) {
+            return privateTimer.newTimeout(task, delay, unit);
+        }
+
+        @Override
+        public Set<Timeout> stop() {
+            // Do not allow the timer to be shut down
+            throw new UnsupportedOperationException();
+        }
+    };
+
     // We are reusing our monitor as the universal lock. We have to account for three distinct threads competing for
     // our state:
     //   1) the typical DTCL callback thread invoking onDataTreeChanged()
@@ -119,6 +140,11 @@ public final class PCEPTopologyTracker
         return pceServerProvider;
     }
 
+    @Override
+    public Timer getTimer() {
+        return timer;
+    }
+
     @Override
     public synchronized void close() {
         if (reg == null) {
@@ -134,6 +160,13 @@ public final class PCEPTopologyTracker
         instances.values().forEach(PCEPTopologySingleton::destroy);
         // Second pass: wait for cleanup
         instances.values().forEach(PCEPTopologySingleton::awaitCleanup);
+
+        // Stop the timer
+        final var cancelledTasks = privateTimer.stop().size();
+        if (cancelledTasks != 0) {
+            LOG.warn("Stopped timer with {} remaining tasks", cancelledTasks);
+        }
+
         LOG.info("PCEP Topology tracker shut down");
     }
 

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
index 310f574da439aa8e97499decff7c4fa1f1f3b6f3..72831756be332c3393685f3df8b16d04f5e3a977 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
@@ -15,7 +15,6 @@ import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
-import io.netty.util.HashedWheelTimer;
 import io.netty.util.Timeout;
 import java.net.InetAddress;
 import java.util.HashMap;
@@ -70,7 +69,6 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
 
     private final @NonNull KeyedInstanceIdentifier<Topology, TopologyKey> topology;
     private final @NonNull PCEPTopologyProviderDependencies dependencies;
-    private final @NonNull HashedWheelTimer timer = new HashedWheelTimer();
 
     @VisibleForTesting
     final AtomicBoolean isClosed = new AtomicBoolean(false);
@@ -146,12 +144,6 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
         }
         state.clear();
 
-        // Stop the timer
-        final var cancelledTasks = timer.stop().size();
-        if (cancelledTasks != 0) {
-            LOG.warn("Stopped timer with {} remaining tasks", cancelledTasks);
-        }
-
         // Un-Register Pcep Topology into PCE Server
         final PceServerProvider server = dependencies.getPceServerProvider();
         if (server != null) {
@@ -283,7 +275,7 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
     final @Nullable Timeout newRpcTimeout(final RpcTimeout task, final SrpIdNumber requestId) {
         final short localTimeout = rpcTimeout;
         return localTimeout <= 0 ? null
-            : timer.newTimeout(ignored -> task.run(requestId), localTimeout, TimeUnit.SECONDS);
+            : dependencies.getTimer().newTimeout(ignored -> task.run(requestId), localTimeout, TimeUnit.SECONDS);
     }
 
     final void setRpcTimeout(final short rpcTimeout) {

diff --git a/pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java b/pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
index 783dd259e57e6ac6b9d80999af9bc6eaaad844e6..085ae973b2f0db9fe0994935e456fdbf5c3051b4 100644 (file)
--- a/pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
+++ b/pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
@@ -20,6 +20,8 @@ import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.EventLoop;
+import io.netty.util.HashedWheelTimer;
+import io.netty.util.Timer;
 import io.netty.util.concurrent.Promise;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
@@ -99,6 +101,8 @@ public abstract class AbstractPCEPSessionTest extends AbstractConcurrentDataBrok
     private PCEPTopologyProviderDependencies topologyDependencies;
     @Mock
     private Promise<PCEPSessionImpl> promise;
+
+    private final Timer timer = new HashedWheelTimer();
     private DefaultPCEPSessionNegotiator neg;
 
     @Before
@@ -128,6 +132,7 @@ public abstract class AbstractPCEPSessionTest extends AbstractConcurrentDataBrok
 
         doReturn(getDataBroker()).when(topologyDependencies).getDataBroker();
         doReturn(statsRegistry).when(topologyDependencies).getStateRegistry();
+        doReturn(timer).when(topologyDependencies).getTimer();
         doReturn(null).when(topologyDependencies).getPceServerProvider();
 
         manager = customizeSessionManager(new ServerSessionManager(TOPO_IID, topologyDependencies, RPC_TIMEOUT,
@@ -155,6 +160,7 @@ public abstract class AbstractPCEPSessionTest extends AbstractConcurrentDataBrok
     @After
     public void tearDown() {
         stopSessionManager();
+        timer.stop();
     }
 
     Ero createEroWithIpPrefixes(final List<String> ipPrefixes) {