From 1f4b3131e1318ddd6a2a8637cdbe09578bb48f0a Mon Sep 17 00:00:00 2001 From: Chandra Shekar S Date: Mon, 23 Mar 2020 15:41:17 +0530 Subject: [PATCH] Processing Hwvtep/Ovsdb client only once There are chances of same client is getting processed multiple times and resulting in stale eos entry in one controller. Due to this stale eos entry in controller1 if the device is getting connected to controller2 it could not be processed as controller1 still remains as eos leader for this device. Ideally controller1 should have released its ownership when the device is disconnected. Signed-off-by: Chandra Shekar S Change-Id: I3b8705d7c18358a1f5be78e5ec3b50055464c23f --- .../HwvtepConnectionManager.java | 12 +++++++++++ .../HwvtepSouthboundConstants.java | 1 + .../HwvtepSouthboundProvider.java | 20 ++++++++++++++----- .../lib/impl/OvsdbConnectionService.java | 8 ++++++-- .../southbound/OvsdbConnectionManager.java | 14 +++++++++++++ .../OvsdbConnectionManagerTest.java | 1 + 6 files changed, 49 insertions(+), 7 deletions(-) diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java index 864dcaf20..d8fa25eb4 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java @@ -90,6 +90,7 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo private final Map, TransactionHistory> controllerTxHistory = new ConcurrentHashMap<>(); private final Map, TransactionHistory> deviceUpdateHistory = new ConcurrentHashMap<>(); private final OvsdbConnection ovsdbConnectionService; + private final Map alreadyProcessedClients = new ConcurrentHashMap<>(); public HwvtepConnectionManager(final DataBroker db, final TransactionInvoker txInvoker, final EntityOwnershipService entityOwnershipService, final OvsdbConnection ovsdbConnectionService) { @@ -116,6 +117,16 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo @Override public void connected(final OvsdbClient externalClient) { + if (alreadyProcessedClients.containsKey(externalClient)) { + LOG.info("Hwvtep Library already connected {} from {}:{} to {}:{} to this, hence skipping the processing", + externalClient.getConnectionInfo().getType(), + externalClient.getConnectionInfo().getRemoteAddress(), + externalClient.getConnectionInfo().getRemotePort(), + externalClient.getConnectionInfo().getLocalAddress(), + externalClient.getConnectionInfo().getLocalPort()); + return; + } + alreadyProcessedClients.put(externalClient, externalClient); HwvtepConnectionInstance hwClient = null; try { List databases = externalClient.getDatabases().get(DB_FETCH_TIMEOUT, TimeUnit.MILLISECONDS); @@ -143,6 +154,7 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo @Override @SuppressWarnings("checkstyle:IllegalCatch") public void disconnected(final OvsdbClient client) { + alreadyProcessedClients.remove(client); HwvtepConnectionInstance hwvtepConnectionInstance = null; try { LOG.info("Library disconnected {} from {}:{} to {}:{}. Cleaning up the operational data store", diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundConstants.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundConstants.java index 328945cf2..1bf1c0c75 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundConstants.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundConstants.java @@ -21,6 +21,7 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology. public interface HwvtepSouthboundConstants { TopologyId HWVTEP_TOPOLOGY_ID = new TopologyId(new Uri("hwvtep:1")); Integer DEFAULT_OVSDB_PORT = 6640; + long PORT_OPEN_MAX_DELAY_IN_MINS = 5; String IID_OTHER_CONFIG_KEY = "opendaylight-iid"; String UUID = "uuid"; ImmutableBiMap,String> ENCAPS_TYPE_MAP diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundProvider.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundProvider.java index 3330c8e99..15446d83f 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundProvider.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundProvider.java @@ -140,6 +140,12 @@ public class HwvtepSouthboundProvider implements ClusteredDataTreeChangeListener LOG.trace("Registering listener for path {}", treeId); operTopologyRegistration = dataBroker.registerDataTreeChangeListener(treeId, this); + Scheduler.getScheduledExecutorService().schedule(() -> { + if (!registered.get()) { + openOvsdbPort(); + LOG.error("Timed out to get eos notification opening the port now"); + } + }, HwvtepSouthboundConstants.PORT_OPEN_MAX_DELAY_IN_MINS, TimeUnit.MINUTES); } private void registerConfigListenerPostUpgrade() { @@ -223,11 +229,7 @@ public class HwvtepSouthboundProvider implements ClusteredDataTreeChangeListener @Override public void onDataTreeChanged(final Collection> collection) { - if (!registered.getAndSet(true)) { - LOG.info("Starting the ovsdb port"); - ovsdbConnection.registerConnectionListener(cm); - ovsdbConnection.startOvsdbManager(); - } + openOvsdbPort(); if (operTopologyRegistration != null) { operTopologyRegistration.close(); @@ -235,6 +237,14 @@ public class HwvtepSouthboundProvider implements ClusteredDataTreeChangeListener } } + private void openOvsdbPort() { + if (!registered.getAndSet(true)) { + LOG.info("Starting the ovsdb port"); + ovsdbConnection.registerConnectionListener(cm); + ovsdbConnection.startOvsdbManager(); + } + } + public void setShardStatusCheckRetryCount(int retryCount) { this.shardStatusCheckRetryCount = retryCount; } diff --git a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java index 675dee291..cd76a00fb 100644 --- a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java +++ b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java @@ -284,8 +284,12 @@ public class OvsdbConnectionService implements AutoCloseable, OvsdbConnection { @Override public void registerConnectionListener(final OvsdbConnectionListener listener) { LOG.info("registerConnectionListener: registering {}", listener.getClass().getSimpleName()); - CONNECTION_LISTENERS.add(listener); - notifyAlreadyExistingConnectionsToListener(listener); + if (CONNECTION_LISTENERS.add(listener)) { + LOG.info("registerConnectionListener: registered {} notifying exisitng connections", + listener.getClass().getSimpleName()); + //notify only the first time if called multiple times + notifyAlreadyExistingConnectionsToListener(listener); + } } private void notifyAlreadyExistingConnectionsToListener(final OvsdbConnectionListener listener) { diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java index 296216ec9..b020a8bf3 100644 --- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java +++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java @@ -76,6 +76,7 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos private final DataBroker db; private final TransactionInvoker txInvoker; + private final Map alreadyProcessedClients = new ConcurrentHashMap<>(); private final Map> instanceIdentifiers = new ConcurrentHashMap<>(); private final Map, OvsdbConnectionInstance> nodeIdVsConnectionInstance = @@ -106,6 +107,18 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos @Override public void connected(final OvsdbClient externalClient) { + if (alreadyProcessedClients.containsKey(externalClient)) { + LOG.info("OvsdbConnectionManager Library already connected {} from {}:{} to {}:{} " + + "to this, hence skipping the processing", + externalClient.getConnectionInfo().getType(), + externalClient.getConnectionInfo().getRemoteAddress(), + externalClient.getConnectionInfo().getRemotePort(), + externalClient.getConnectionInfo().getLocalAddress(), + externalClient.getConnectionInfo().getLocalPort()); + return; + } + alreadyProcessedClients.put(externalClient, externalClient); + LOG.info("Library connected {} from {}:{} to {}:{}", externalClient.getConnectionInfo().getType(), externalClient.getConnectionInfo().getRemoteAddress(), @@ -161,6 +174,7 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos @Override public void disconnected(final OvsdbClient client) { + alreadyProcessedClients.remove(client); LOG.info("Library disconnected {} from {}:{} to {}:{}. Cleaning up the operational data store", client.getConnectionInfo().getType(), client.getConnectionInfo().getRemoteAddress(), diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java index 53754cce5..2b6b9d421 100644 --- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java +++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java @@ -88,6 +88,7 @@ public class OvsdbConnectionManagerTest { field(OvsdbConnectionManager.class, "entityOwnershipService").set(ovsdbConnManager, entityOwnershipService); field(OvsdbConnectionManager.class, "reconciliationManager").set(ovsdbConnManager, reconciliationManager); field(OvsdbConnectionManager.class, "ovsdbConnection").set(ovsdbConnManager, ovsdbConnection); + field(OvsdbConnectionManager.class, "alreadyProcessedClients").set(ovsdbConnManager, new HashMap()); entityConnectionMap = new ConcurrentHashMap<>(); OvsdbConnectionInfo info = mock(OvsdbConnectionInfo.class); -- 2.36.6