From: Jozef Bacigal Date: Fri, 4 Mar 2016 15:11:06 +0000 (+0100) Subject: Quick fix RPCs and DevicCtx.close X-Git-Tag: release/boron~295^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=249057bedc21b9983f0b111fe83f53df0c098eaa;p=openflowplugin.git Quick fix RPCs and DevicCtx.close Change-Id: I1cd92d91e1025ae3aee00418b86912f2e86580b4 Signed-off-by: Vaclav Demcak Signed-off-by: Jozef Bacigal --- diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java index 3ea654fe22..61480de5f0 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java @@ -34,21 +34,21 @@ public class HandshakeListenerImpl implements HandshakeListener { private static final Logger LOG = LoggerFactory.getLogger(HandshakeListenerImpl.class); - private ConnectionContext connectionContext; - private DeviceConnectedHandler deviceConnectedHandler; + private final ConnectionContext connectionContext; + private final DeviceConnectedHandler deviceConnectedHandler; private HandshakeContext handshakeContext; /** * @param connectionContext * @param deviceConnectedHandler */ - public HandshakeListenerImpl(ConnectionContext connectionContext, DeviceConnectedHandler deviceConnectedHandler) { + public HandshakeListenerImpl(final ConnectionContext connectionContext, final DeviceConnectedHandler deviceConnectedHandler) { this.connectionContext = connectionContext; this.deviceConnectedHandler = deviceConnectedHandler; } @Override - public void onHandshakeSuccessfull(GetFeaturesOutput featureOutput, Short version) { + public void onHandshakeSuccessfull(final GetFeaturesOutput featureOutput, final Short version) { LOG.debug("handshake succeeded: {}", connectionContext.getConnectionAdapter().getRemoteAddress()); closeHandshakeContext(); connectionContext.changeStateToWorking(); @@ -65,11 +65,11 @@ public class HandshakeListenerImpl implements HandshakeListener { deviceConnectedHandler.deviceConnected(connectionContext); SessionStatistics.countEvent(connectionContext.getNodeId().toString(), SessionStatistics.ConnectionStatus.CONNECTION_CREATED); - } catch (Exception e) { + } catch (final Exception e) { LOG.info("ConnectionContext initial processing failed: {}", e.getMessage()); SessionStatistics.countEvent(connectionContext.getNodeId().toString(), SessionStatistics.ConnectionStatus.CONNECTION_DISCONNECTED_BY_OFP); - connectionContext.closeConnection(false); + connectionContext.closeConnection(true); } } @@ -100,14 +100,14 @@ public class HandshakeListenerImpl implements HandshakeListener { private void closeHandshakeContext() { try { handshakeContext.close(); - } catch (Exception e) { + } catch (final Exception e) { LOG.warn("Closing handshake context failed: {}", e.getMessage()); LOG.debug("Detail in hanshake context close:", e); } } @Override - public void setHandshakeContext(HandshakeContext handshakeContext) { + public void setHandshakeContext(final HandshakeContext handshakeContext) { this.handshakeContext = handshakeContext; } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java index 524e223f45..e5c62d7e65 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java @@ -14,6 +14,7 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.AsyncFunction; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.FutureFallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import io.netty.util.HashedWheelTimer; @@ -29,6 +30,8 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService; import org.opendaylight.controller.md.sal.binding.api.NotificationService; @@ -244,6 +247,8 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi return Futures.immediateFuture(null); } if (OfpRole.BECOMEMASTER.equals(role)) { + MdSalRegistratorUtils.registerMasterServices(getRpcContext(), DeviceContextImpl.this, role); + getRpcContext().registerStatCompatibilityServices(); if (!deviceState.deviceSynchronized()) { //TODO: no necessary code for yet - it needs for initialization phase only LOG.debug("Setup Empty TxManager {} for initialization phase", getDeviceState().getNodeId()); @@ -251,7 +256,23 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi return Futures.immediateCheckedFuture(null); } /* Relevant for no initial Slave-to-Master scenario in cluster */ - return asyncClusterRoleChange(role); + final ListenableFuture deviceInitialization = asyncClusterRoleChange(); + Futures.addCallback(deviceInitialization, new FutureCallback() { + + @Override + public void onSuccess(@Nullable Void aVoid) { + //No operation + } + + @Override + public void onFailure(Throwable throwable) { + LOG.debug("Device {} init unexpected fail. Unregister RPCs", getDeviceState().getNodeId()); + MdSalRegistratorUtils.unregisterServices(getRpcContext()); + } + + }); + + return deviceInitialization; } else if (OfpRole.BECOMESLAVE.equals(role)) { if (null != rpcContext) { @@ -272,14 +293,14 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi * check all NodeInformation for statistics otherwise statistics will not contains * all possible MultipartTypes for polling in StatTypeList */ - private ListenableFuture asyncClusterRoleChange(final OfpRole role) { + private ListenableFuture asyncClusterRoleChange() { if (statCtx == null) { - final String errMsg = String.format("DeviceCtx {} is up but we are missing StatisticsContext", deviceState.getNodeId()); + final String errMsg = String.format("DeviceCtx %s is up but we are missing StatisticsContext", deviceState.getNodeId()); LOG.warn(errMsg); return Futures.immediateFailedFuture(new IllegalStateException(errMsg)); } if (rpcContext == null) { - final String errMsg = String.format("DeviceCtx {} is up but we are missing RpcContext", deviceState.getNodeId()); + final String errMsg = String.format("DeviceCtx %s is up but we are missing RpcContext", deviceState.getNodeId()); LOG.warn(errMsg); return Futures.immediateFailedFuture(new IllegalStateException(errMsg)); } @@ -322,14 +343,14 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi @Override public Void apply(final Boolean input) { if (ConnectionContext.CONNECTION_STATE.RIP.equals(getPrimaryConnectionContext().getConnectionState())) { - final String errMsg = String.format("We lost connection for Device {}, context has to be closed.", + final String errMsg = String.format("We lost connection for Device %s, context has to be closed.", getDeviceState().getNodeId()); LOG.warn(errMsg); transactionChainManager.clearUnsubmittedTransaction(); throw new IllegalStateException(errMsg); } if (!input.booleanValue()) { - final String errMsg = String.format("Get Initial Device {} information fails", + final String errMsg = String.format("Get Initial Device %s information fails", getDeviceState().getNodeId()); LOG.warn(errMsg); transactionChainManager.clearUnsubmittedTransaction(); @@ -338,8 +359,6 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi LOG.debug("Get Initial Device {} information is successful", getDeviceState().getNodeId()); getDeviceState().setDeviceSynchronized(true); transactionChainManager.activateTransactionManager(); - MdSalRegistratorUtils.registerMasterServices(getRpcContext(), DeviceContextImpl.this, role); - getRpcContext().registerStatCompatibilityServices(); initialSubmitTransaction(); getDeviceState().setStatisticsPollingEnabledProp(true); return null; @@ -542,14 +561,15 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi } @Override - public void close() { + public synchronized void close() { LOG.debug("closing deviceContext: {}, nodeId:{}", getPrimaryConnectionContext().getConnectionAdapter().getRemoteAddress(), getDeviceState().getNodeId()); - tearDown(); - - primaryConnectionContext.closeConnection(false); + if (deviceState.isValid()) { + primaryConnectionContext.closeConnection(false); + tearDown(); + } } private synchronized void tearDown() { @@ -583,9 +603,8 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi } } - protected void tearDownClean() { + private void tearDownClean() { LOG.info("Closing transaction chain manager without cleaning inventory operational"); - Preconditions.checkState(!deviceState.isValid()); transactionChainManager.close(); final LinkedList reversedCloseHandlers = new LinkedList<>(closeHandlers); diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java index 6b7d45f7f3..1531023711 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java @@ -53,6 +53,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.table.service.rev131026.Sal public class MdSalRegistratorUtils { + //TODO: Make one register and one unregister method for all services + private static final TypeToken> COMPOSITE_SERVICE_TYPE_TOKEN = new TypeToken>() { //NOBODY diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java index 7be1d00e16..63cdfceb05 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java @@ -395,10 +395,9 @@ public class DeviceContextImplTest { deviceContext.addAuxiliaryConenctionContext(mockedAuxiliaryConnectionContext); DeviceContextClosedHandler mockedDeviceContextClosedHandler = mock(DeviceContextClosedHandler.class); deviceContext.addDeviceContextClosedHandler(mockedDeviceContextClosedHandler); + when(deviceState.isValid()).thenReturn(true); deviceContext.close(); - verify(connectionContext).closeConnection(eq(false)); -// verify(deviceState).setValid(eq(false)); -// verify(mockedAuxiliaryConnectionContext).closeConnection(eq(false)); + verify(connectionContext).closeConnection(false); } @Test