From 466e1630afd87e306906b1cf4d69d6ef97b6c035 Mon Sep 17 00:00:00 2001 From: "K.V Suneelu Verma" Date: Mon, 18 Dec 2017 16:46:17 +0530 Subject: [PATCH] bug 6578 added mdsal read retry Change-Id: I5dba7362bf41ea84baa35dc29a6a4a6a69ba78fa Signed-off-by: K.V Suneelu Verma --- .../HwvtepSouthboundUtil.java | 7 +-- .../transact/AbstractTransactCommand.java | 5 ++ .../transact/HwvtepOperationalState.java | 21 ++++--- .../transact/PhysicalSwitchUpdateCommand.java | 6 +- .../transact/TransactUtils.java | 11 ---- .../transact/UcastMacsLocalUpdateCommand.java | 6 +- .../md/AbstractTransactionCommand.java | 3 +- .../ovsdb/utils/mdsal/utils/MdsalUtils.java | 62 +++++++++++++------ 8 files changed, 73 insertions(+), 48 deletions(-) diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java index aa6804f07..a2941e134 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java @@ -14,6 +14,7 @@ import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException; +import org.opendaylight.ovsdb.utils.mdsal.utils.MdsalUtils; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalAugmentation; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalRef; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalSwitchAttributes; @@ -100,16 +101,12 @@ public class HwvtepSouthboundUtil { public static Optional getManagingNode(DataBroker db, HwvtepGlobalRef ref) { try { - ReadOnlyTransaction transaction = db.newReadOnlyTransaction(); @SuppressWarnings("unchecked") // Note: erasure makes this safe in combination with the typecheck // below InstanceIdentifier path = (InstanceIdentifier) ref.getValue(); - CheckedFuture, ReadFailedException> nf = - transaction.read(LogicalDatastoreType.OPERATIONAL, path); - transaction.close(); - Optional optional = nf.get(); + Optional optional = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, path); if (optional != null && optional.isPresent()) { HwvtepGlobalAugmentation hwvtepNode = null; Node node = optional.get(); diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java index 34e1165f2..813c54d62 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java @@ -21,6 +21,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.DataObjectModification; import org.opendaylight.controller.md.sal.binding.api.DataTreeModification; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; @@ -61,6 +62,10 @@ public abstract class AbstractTransactCommand> getChanges() { return changes; } diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/HwvtepOperationalState.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/HwvtepOperationalState.java index ea7dee8a1..829777c93 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/HwvtepOperationalState.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/HwvtepOperationalState.java @@ -14,10 +14,12 @@ import org.apache.commons.lang3.tuple.Pair; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.DataTreeModification; import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; +import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepConnectionInstance; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepDeviceInfo; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundUtil; import org.opendaylight.ovsdb.lib.notation.UUID; +import org.opendaylight.ovsdb.utils.mdsal.utils.MdsalUtils; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.EncapsulationTypeBase; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalAugmentation; @@ -98,7 +100,7 @@ public class HwvtepOperationalState { this.db = connectionInstance.getDataBroker(); this.changes = null; transaction = connectionInstance.getDataBroker().newReadWriteTransaction(); - Optional readNode = HwvtepSouthboundUtil.readNode(transaction, + Optional readNode = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, connectionInstance.getInstanceIdentifier()); if (readNode.isPresent()) { operationalNodes.put(connectionInstance.getInstanceIdentifier(), readNode.get()); @@ -135,7 +137,8 @@ public class HwvtepOperationalState { if (nodeCreateOrUpdate != null) { transaction = db.newReadWriteTransaction(); for (Entry, Node> entry: nodeCreateOrUpdate.entrySet()) { - Optional readNode = HwvtepSouthboundUtil.readNode(transaction, entry.getKey()); + Optional readNode = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, + entry.getKey()); //add related globalNode or physicalSwitchNode to operationalNodes map //for example, when creating physical port, logical switch is needed //but logical switch is in HwvtepGlobalAugmentation rather than PhysicalSwitchAugmentation @@ -147,7 +150,8 @@ public class HwvtepOperationalState { for (Switches pswitch : hgAugmentation.getSwitches()) { @SuppressWarnings("unchecked") InstanceIdentifier psNodeIid = (InstanceIdentifier) pswitch.getSwitchRef().getValue(); - Optional psNode = HwvtepSouthboundUtil.readNode(transaction, psNodeIid); + Optional psNode = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, + psNodeIid); if (psNode.isPresent()) { operationalNodes.put(psNodeIid, psNode.get()); } @@ -156,7 +160,8 @@ public class HwvtepOperationalState { if (psAugmentation != null) { @SuppressWarnings("unchecked") InstanceIdentifier hgNodeIid = (InstanceIdentifier) psAugmentation.getManagedBy().getValue(); - Optional hgNode = HwvtepSouthboundUtil.readNode(transaction, hgNodeIid); + Optional hgNode = new MdsalUtils(db).readOptional( + LogicalDatastoreType.OPERATIONAL, hgNodeIid); if (hgNode.isPresent()) { operationalNodes.put(hgNodeIid, hgNode.get()); } @@ -369,7 +374,7 @@ public class HwvtepOperationalState { } public Optional getPhysicalLocatorAugmentation(InstanceIdentifier iid) { - Optional tp = HwvtepSouthboundUtil.readNode(transaction, iid); + Optional tp = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, iid); if (tp.isPresent()) { return Optional.fromNullable(tp.get().getAugmentation(HwvtepPhysicalLocatorAugmentation.class)); } @@ -377,17 +382,17 @@ public class HwvtepOperationalState { } public Optional getLogicalSwitches(InstanceIdentifier iid) { - Optional lswitch = HwvtepSouthboundUtil.readNode(transaction, iid); + Optional lswitch = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, iid); return lswitch; } public Optional getTunnels(InstanceIdentifier iid) { - Optional tunnels = HwvtepSouthboundUtil.readNode(transaction, iid); + Optional tunnels = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, iid); return tunnels; } public Optional getAcls(InstanceIdentifier iid) { - Optional acl = HwvtepSouthboundUtil.readNode(transaction, iid); + Optional acl = new MdsalUtils(db).readOptional(LogicalDatastoreType.OPERATIONAL, iid); return acl; } diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/PhysicalSwitchUpdateCommand.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/PhysicalSwitchUpdateCommand.java index 89849d7f8..d2ece230d 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/PhysicalSwitchUpdateCommand.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/PhysicalSwitchUpdateCommand.java @@ -22,6 +22,7 @@ import java.util.Set; import org.opendaylight.controller.md.sal.binding.api.DataObjectModification; import org.opendaylight.controller.md.sal.binding.api.DataTreeModification; +import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundMapper; import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException; import org.opendaylight.ovsdb.lib.notation.Mutator; @@ -31,6 +32,7 @@ import org.opendaylight.ovsdb.lib.schema.typed.TyperUtils; import org.opendaylight.ovsdb.schema.hardwarevtep.Global; import org.opendaylight.ovsdb.schema.hardwarevtep.PhysicalSwitch; import org.opendaylight.ovsdb.schema.hardwarevtep.Tunnel; +import org.opendaylight.ovsdb.utils.mdsal.utils.MdsalUtils; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalLocatorAugmentation; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.PhysicalSwitchAugmentation; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.physical._switch.attributes.ManagementIps; @@ -284,8 +286,8 @@ public class PhysicalSwitchUpdateCommand extends AbstractTransactCommand { } else { // TODO/FIXME: Not in operational, do we create a new one? LOG.warn("Trying to create tunnel without creating physical locators first"); - Optional confLocOptional = - TransactUtils.readNodeFromConfig(getOperationalState().getReadWriteTransaction(), iid); + Optional confLocOptional = new MdsalUtils(getOperationalState().getDataBroker()) + .readOptional(LogicalDatastoreType.CONFIGURATION, iid); if (confLocOptional.isPresent()) { HwvtepPhysicalLocatorAugmentation locatorAugmentation = confLocOptional.get().getAugmentation(HwvtepPhysicalLocatorAugmentation.class); diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactUtils.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactUtils.java index 5b6c855be..41c15053c 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactUtils.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactUtils.java @@ -131,17 +131,6 @@ public class TransactUtils { return result; } - public static Optional readNodeFromConfig( - ReadWriteTransaction transaction, final InstanceIdentifier connectionIid) { - Optional node = Optional.absent(); - try { - node = transaction.read(LogicalDatastoreType.CONFIGURATION, connectionIid).checkedGet(); - } catch (final ReadFailedException e) { - LOG.warn("Read Configration/DS for Node failed! {}", connectionIid, e); - } - return node; - } - public static UUID createPhysicalLocatorSet(HwvtepOperationalState hwvtepOperationalState, TransactionBuilder transaction, List locatorList) { Set locators = new HashSet(); for (LocatorSet locator: locatorList) { diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UcastMacsLocalUpdateCommand.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UcastMacsLocalUpdateCommand.java index 1f5b5cc06..82c2478e6 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UcastMacsLocalUpdateCommand.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UcastMacsLocalUpdateCommand.java @@ -16,10 +16,12 @@ import java.util.Map; import java.util.Map.Entry; import org.opendaylight.controller.md.sal.binding.api.DataTreeModification; +import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.ovsdb.lib.notation.UUID; import org.opendaylight.ovsdb.lib.operations.TransactionBuilder; import org.opendaylight.ovsdb.lib.schema.typed.TyperUtils; import org.opendaylight.ovsdb.schema.hardwarevtep.UcastMacsLocal; +import org.opendaylight.ovsdb.utils.mdsal.utils.MdsalUtils; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalAugmentation; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalLocatorAugmentation; @@ -117,8 +119,8 @@ public class UcastMacsLocalUpdateCommand extends AbstractTransactCommand configLocatorOptional = - TransactUtils.readNodeFromConfig(getOperationalState().getReadWriteTransaction(), iid); + Optional configLocatorOptional = new MdsalUtils( + getOperationalState().getDataBroker()).readOptional(LogicalDatastoreType.CONFIGURATION, iid); if (configLocatorOptional.isPresent()) { HwvtepPhysicalLocatorAugmentation locatorAugmentation = configLocatorOptional.get().getAugmentation(HwvtepPhysicalLocatorAugmentation.class); diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/AbstractTransactionCommand.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/AbstractTransactionCommand.java index ec19f3b5b..d7dca6b95 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/AbstractTransactionCommand.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transactions/md/AbstractTransactionCommand.java @@ -8,6 +8,7 @@ package org.opendaylight.ovsdb.hwvtepsouthbound.transactions.md; +import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepConnectionInstance; import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepDeviceInfo; import org.opendaylight.ovsdb.lib.message.TableUpdates; @@ -37,7 +38,7 @@ public abstract class AbstractTransactionCommand implement return key; } - public AbstractTransactionCommand(HwvtepConnectionInstance key, TableUpdates updates, DatabaseSchema dbSchema) { + public AbstractTransactionCommand(HwvtepConnectionInstance key,TableUpdates updates, DatabaseSchema dbSchema) { this.updates = updates; this.dbSchema = dbSchema; this.key = key; diff --git a/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/MdsalUtils.java b/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/MdsalUtils.java index 642ba080a..f5354abf1 100644 --- a/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/MdsalUtils.java +++ b/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/MdsalUtils.java @@ -15,6 +15,7 @@ import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,7 +23,8 @@ import org.slf4j.LoggerFactory; public class MdsalUtils { private static final Logger LOG = LoggerFactory.getLogger(MdsalUtils.class); private DataBroker databroker = null; - + private static int MDSAL_MAX_READ_TRIALS = Integer.getInteger("mdsalutil.max.tries", 30); + private static int MDSAL_READ_SLEEP_INTERVAL_MS = Integer.getInteger("mdsalutil.sleep.between.mdsal.reads", 1000); /** * Class constructor setting the data broker. * @@ -109,24 +111,46 @@ public class MdsalUtils { * @param the data object type * @return the result as the data object requested */ - public D read( - final LogicalDatastoreType store, final InstanceIdentifier path) { - D result = null; - final ReadOnlyTransaction transaction = databroker.newReadOnlyTransaction(); - Optional optionalDataObject; - CheckedFuture, ReadFailedException> future = transaction.read(store, path); - try { - optionalDataObject = future.checkedGet(); - if (optionalDataObject.isPresent()) { - result = optionalDataObject.get(); - } else { - LOG.debug("{}: Failed to read {}", - Thread.currentThread().getStackTrace()[1], path); - } - } catch (ReadFailedException e) { - LOG.warn("Failed to read {} ", path, e); + public D read( + final LogicalDatastoreType store, final InstanceIdentifier path) { + Optional optionalDataObject = readOptional(store, path); + if (optionalDataObject.isPresent()) { + return optionalDataObject.get(); } - transaction.close(); - return result; + LOG.debug("{}: Failed to read {}", + Thread.currentThread().getStackTrace()[1], path); + return null; + } + + public Optional readOptional( + final LogicalDatastoreType store, final InstanceIdentifier path) { + int trialNo = 0; + ReadOnlyTransaction transaction = databroker.newReadOnlyTransaction(); + do { + try { + Optional result = transaction.read(store, (InstanceIdentifier)path).checkedGet(); + transaction.close(); + return result; + } catch (ReadFailedException e) { + if (trialNo == 0) { + logReadFailureError(path, " mdsal Read failed exception retrying the read after sleep"); + } + try { + transaction.close(); + Thread.sleep(MDSAL_READ_SLEEP_INTERVAL_MS); + transaction = databroker.newReadOnlyTransaction(); + } catch (InterruptedException e1) { + logReadFailureError(path, " Sleep interrupted"); + } + } + } while (trialNo++ < MDSAL_MAX_READ_TRIALS); + logReadFailureError(path, " All read trials exceeded"); + return Optional.absent(); + } + + private void logReadFailureError( + InstanceIdentifier path, String cause) { + LOG.error("{}: Failed to read {} Cause : {}", Thread.currentThread().getStackTrace()[2], path, cause); + } } -- 2.36.6