From c262d7912fba8fed542860f527dc173977d022dd Mon Sep 17 00:00:00 2001 From: Sangwook Ha Date: Mon, 2 Oct 2023 13:24:48 -0700 Subject: [PATCH] Close device's source resolution on teardown Checking the current connection status is not enough to guarantee that schema setup was completed for the initial session-up event. To resolve this close ListenableFuture for device schema resolution when the 'onRemoteSessionDown' method is called. This prevents unnecessary duplicate notification, creating duplicate SchemaContext and avoid confusion in the connection state. JIRA: NETCONF-1173 Change-Id: I7f84da4a9197db52a0c33d2640e0c2543e587d7b (cherry picked from commit e9518fdc4410b5407318d4b3bcc7fa333e296c55) Signed-off-by: Sangwook Ha Signed-off-by: Peter Suna Signed-off-by: Ivan Hrasko --- .../sal/connect/netconf/NetconfDevice.java | 22 ++++++++--- .../connect/netconf/NetconfDeviceTest.java | 39 +++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java index 4ffafe55b2..ca821798b1 100644 --- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java +++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -116,6 +117,8 @@ public class NetconfDevice private final EventExecutor eventExecutor; private final NetconfNodeAugmentedOptional nodeOptional; + @GuardedBy("this") + private ListenableFuture> schemaFuturesList; @GuardedBy("this") private boolean connected = false; @@ -151,8 +154,8 @@ public class NetconfDevice } @Override - public void onRemoteSessionUp(final NetconfSessionPreferences remoteSessionCapabilities, - final NetconfDeviceCommunicator listener) { + public synchronized void onRemoteSessionUp(final NetconfSessionPreferences remoteSessionCapabilities, + final NetconfDeviceCommunicator listener) { // SchemaContext setup has to be performed in a dedicated thread since // we are in a netty thread in this method // Yang models are being downloaded in this method and it would cause a @@ -178,6 +181,7 @@ public class NetconfDevice // Potentially acquire mount point list and interpret it final ListenableFuture futureContext = Futures.transformAsync(futureSchema, schemaContext -> createMountPointContext(schemaContext, baseSchema, listener), processingExecutor); + schemaFuturesList = Futures.allAsList(sourceResolverFuture, futureSchema, futureContext); Futures.addCallback(futureContext, new FutureCallback() { @Override @@ -188,8 +192,12 @@ public class NetconfDevice @Override public void onFailure(final Throwable cause) { - LOG.warn("{}: Unexpected error resolving device sources", id, cause); + if (cause instanceof CancellationException) { + LOG.warn("{}: Device communicator was tear down since the schema setup started", id); + return; + } + LOG.warn("{}: Unexpected error resolving device sources", id, cause); // No more sources, fail or try to reconnect if (cause instanceof EmptySchemaContextException) { if (nodeOptional != null && nodeOptional.getIgnoreMissingSchemaSources().getAllowed()) { @@ -342,10 +350,14 @@ public class NetconfDevice } @Override - public void onRemoteSessionDown() { + public synchronized void onRemoteSessionDown() { setConnected(false); + if (schemaFuturesList != null && !schemaFuturesList.isDone()) { + if (!schemaFuturesList.cancel(true)) { + LOG.warn("The cleanup of Schema Futures for device {} was unsuccessful.", id); + } + } notificationHandler.onRemoteSchemaDown(); - salFacade.onDeviceDisconnected(); sourceRegistrations.forEach(SchemaSourceRegistration::close); sourceRegistrations.clear(); diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java index 656b669787..12dcf90b21 100644 --- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java +++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java @@ -372,6 +372,45 @@ public class NetconfDeviceTest extends AbstractTestModelTest { verify(facade, after(1000).never()).onDeviceConnected(any(), any(), any(), any(DOMActionService.class)); } + @Test + public void testNetconfDeviceReconnectBeforeSchemaSetup() throws Exception { + final RemoteDeviceHandler facade = getFacade(); + + final EffectiveModelContextFactory schemaContextProviderFactory = mock(EffectiveModelContextFactory.class); + final SettableFuture schemaFuture = SettableFuture.create(); + doReturn(schemaFuture).when(schemaContextProviderFactory).createEffectiveModelContext(anyCollection()); + + final NetconfDevice.SchemaResourcesDTO schemaResourcesDTO = new NetconfDevice.SchemaResourcesDTO( + getSchemaRegistry(), getSchemaRepository(), schemaContextProviderFactory, STATE_SCHEMAS_RESOLVER); + final NetconfDevice device = new NetconfDeviceBuilder() + .setReconnectOnSchemasChange(true) + .setSchemaResourcesDTO(schemaResourcesDTO) + .setGlobalProcessingExecutor(getExecutor()) + .setId(getId()) + .setSalFacade(facade) + .setBaseSchemas(BASE_SCHEMAS) + .build(); + final NetconfSessionPreferences sessionCaps = getSessionCaps(true, + List.of(TEST_NAMESPACE + "?module=" + TEST_MODULE + "&revision=" + TEST_REVISION)); + + final NetconfDeviceCommunicator listener = getListener(); + // session up, start schema resolution + device.onRemoteSessionUp(sessionCaps, listener); + // session down + device.onRemoteSessionDown(); + verify(facade, timeout(5000)).onDeviceDisconnected(); + // session back up, start another schema resolution + device.onRemoteSessionUp(sessionCaps, listener); + // complete schema setup + schemaFuture.set(SCHEMA_CONTEXT); + // schema setup performed twice + verify(schemaContextProviderFactory, timeout(5000)).createEffectiveModelContext(anyCollection()); + // onDeviceConnected called once + verify(facade, timeout(5000)).onDeviceConnected( + any(MountPointContext.class), any(NetconfSessionPreferences.class), any(DOMRpcService.class), + isNull()); + } + @Test public void testNetconfDeviceAvailableCapabilitiesBuilding() throws Exception { final RemoteDeviceHandler facade = getFacade(); -- 2.36.6