NETVIRT-1545: MDSAL transactions optimizations

MDSAL transactions optimizations for Aclservice.
1. Security-Group is created as part of first SG rule creation, which
might lead to tx.merge, could cause delayed transactions.
2. Aclservice uses syncWrite and syncUpdate in neutronvpn module, which
could cause delayed transactions.

fix:
1. Added SecurityGroup Listener, which would create SG/Access-lists,
hopefully before SG rule. There-by tx would not get delayed.
2. Replace sync calls with Async using JobCoordinator.

Change-Id: I01318ea3ae1edcd5e25ad7aaf2a80b532777a028
Signed-off-by: kiranvasudeva <kirankumar.v@altencalsoftlabs.com>

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
index 80be48ee179c899cd72a3db35c96c8be032256b1..3f0a85ed3749b7d6e7e7099ed97bc96a997a7690 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
@@ -107,6 +107,13 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
             this.aclServiceUtils.releaseAclTag(aclName);
         }
         updateRemoteAclCache(acl.getAccessListEntries().getAce(), aclName, AclServiceManager.Action.REMOVE);
+        if (aclClusterUtil.isEntityOwner()) {
+            // Handle Rule deletion If SG Remove event is received before SG Rule delete event
+            List<Ace> aceList = acl.getAccessListEntries().getAce();
+            Collection<AclInterface> aclInterfaces =
+                    ImmutableSet.copyOf(aclDataUtil.getInterfaceList(new Uuid(aclName)));
+            updateAceRules(aclInterfaces, aclName, aceList, AclServiceManager.Action.REMOVE);
+        }
     }
 
     @Override

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityGroupListener.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityGroupListener.java
new file mode 100644 (file)
index 0000000..e208ccd
--- /dev/null
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityGroupListener.java
@@ -0,0 +1,115 @@
+/*
+ * Copyright (c) 2019 Ericsson India Global Services Pvt Ltd. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.netvirt.neutronvpn;
+
+import static org.opendaylight.controller.md.sal.binding.api.WriteTransaction.CREATE_MISSING_PARENTS;
+import static org.opendaylight.genius.infra.Datastore.CONFIGURATION;
+
+import java.util.ArrayList;
+import java.util.Collections;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
+import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
+import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl;
+import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.AccessLists;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.Acl;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.AclBuilder;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.AclKey;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.acl.AccessListEntriesBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.groups.attributes.SecurityGroups;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.groups.attributes.security.groups.SecurityGroup;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class NeutronSecurityGroupListener
+        extends AsyncDataTreeChangeListenerBase<SecurityGroup, NeutronSecurityGroupListener> {
+    private static final Logger LOG = LoggerFactory.getLogger(NeutronSecurityGroupListener.class);
+    private final DataBroker dataBroker;
+    private final ManagedNewTransactionRunner txRunner;
+    private final JobCoordinator jobCoordinator;
+
+    @Inject
+    public NeutronSecurityGroupListener(DataBroker dataBroker, JobCoordinator jobCoordinator) {
+        super(SecurityGroup.class, NeutronSecurityGroupListener.class);
+        this.dataBroker = dataBroker;
+        this.jobCoordinator = jobCoordinator;
+        this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker);
+    }
+
+    @Override
+    @PostConstruct
+    public void init() {
+        LOG.info("{} init", getClass().getSimpleName());
+        registerListener(LogicalDatastoreType.CONFIGURATION, dataBroker);
+    }
+
+    @Override
+    protected InstanceIdentifier<SecurityGroup> getWildCardPath() {
+        return InstanceIdentifier.create(Neutron.class).child(SecurityGroups.class).child(SecurityGroup.class);
+    }
+
+    @Override
+    protected void remove(InstanceIdentifier<SecurityGroup> key, SecurityGroup securityGroup) {
+        LOG.trace("Removing securityGroup: {}", securityGroup);
+        InstanceIdentifier<Acl> identifier = getAclInstanceIdentifier(securityGroup);
+        String jobKey = securityGroup.key().getUuid().getValue();
+        jobCoordinator.enqueueJob(jobKey,
+            () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
+                tx -> tx.delete(identifier))),
+            NeutronSecurityRuleConstants.DJC_MAX_RETRIES);
+    }
+
+    @Override
+    protected void update(InstanceIdentifier<SecurityGroup> key, SecurityGroup dataObjectModificationBefore,
+        SecurityGroup dataObjectModificationAfter) {
+        LOG.debug("Do nothing");
+    }
+
+    @Override
+    protected void add(InstanceIdentifier<SecurityGroup> instanceIdentifier, SecurityGroup securityGroup) {
+        LOG.trace("Adding securityGroup: {}", securityGroup);
+        Acl acl = toAclBuilder(securityGroup).build();
+        InstanceIdentifier<Acl> identifier = getAclInstanceIdentifier(securityGroup);
+        String jobKey = securityGroup.key().getUuid().getValue();
+        jobCoordinator.enqueueJob(jobKey,
+            () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
+                tx -> tx.put(identifier, acl, CREATE_MISSING_PARENTS))),
+            NeutronSecurityRuleConstants.DJC_MAX_RETRIES);
+    }
+
+    @Override
+    protected NeutronSecurityGroupListener getDataTreeChangeListener() {
+        return this;
+    }
+
+    private InstanceIdentifier<Acl> getAclInstanceIdentifier(SecurityGroup securityGroup) {
+        return InstanceIdentifier
+            .builder(AccessLists.class).child(Acl.class,
+                new AclKey(securityGroup.key().getUuid().getValue(), NeutronSecurityRuleConstants.ACLTYPE))
+            .build();
+    }
+
+    private AclBuilder toAclBuilder(SecurityGroup securityGroup) {
+        AclBuilder aclBuilder = new AclBuilder();
+        aclBuilder.setAclName(securityGroup.key().getUuid().getValue());
+        aclBuilder.setAclType(NeutronSecurityRuleConstants.ACLTYPE);
+        aclBuilder.setAccessListEntries(new AccessListEntriesBuilder().setAce(new ArrayList<>()).build());
+
+        return aclBuilder;
+    }
+}

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleConstants.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleConstants.java
index 31f1b3cfe4ac8c11a1089577afb14d67a121a136..0fede9b710219a76feec52bc68c23034fb6922d9 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleConstants.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleConstants.java
@@ -30,6 +30,7 @@ public interface NeutronSecurityRuleConstants {
     Short PROTOCOL_TCP = 6;
     Short PROTOCOL_UDP = 17;
     Short PROTOCOL_ICMPV6 = 58;
+    int DJC_MAX_RETRIES = 3;
 
     Class<EthertypeV4> ETHERTYPE_IPV4 = EthertypeV4.class;
 

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleListener.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleListener.java
index edd3a0abebda42f87086458023f429b08f0a2027..960787330c910426d88ae6528d6b6463ca78f016 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleListener.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronSecurityRuleListener.java
@@ -7,14 +7,22 @@
  */
 package org.opendaylight.netvirt.neutronvpn;
 
+import static org.opendaylight.controller.md.sal.binding.api.WriteTransaction.CREATE_MISSING_PARENTS;
+import static org.opendaylight.genius.infra.Datastore.CONFIGURATION;
+
 import com.google.common.collect.ImmutableBiMap;
+
+import java.util.Collections;
+
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
-import org.opendaylight.genius.mdsalutil.MDSALUtil;
+import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
+import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl;
+import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.AccessLists;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.Acl;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.AclKey;
@@ -66,11 +74,15 @@ public class NeutronSecurityRuleListener
             ProtocolIcmpV6.class, NeutronSecurityRuleConstants.PROTOCOL_ICMPV6);
 
     private final DataBroker dataBroker;
+    private final ManagedNewTransactionRunner txRunner;
+    private final JobCoordinator jobCoordinator;
 
     @Inject
-    public NeutronSecurityRuleListener(final DataBroker dataBroker) {
+    public NeutronSecurityRuleListener(final DataBroker dataBroker, JobCoordinator jobCoordinator) {
         super(SecurityRule.class, NeutronSecurityRuleListener.class);
         this.dataBroker = dataBroker;
+        this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker);
+        this.jobCoordinator = jobCoordinator;
     }
 
     @Override
@@ -93,7 +105,11 @@ public class NeutronSecurityRuleListener
         try {
             Ace ace = toAceBuilder(securityRule, false).build();
             InstanceIdentifier<Ace> identifier = getAceInstanceIdentifier(securityRule);
-            MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, identifier, ace);
+            String jobKey = securityRule.getSecurityGroupId().getValue();
+            jobCoordinator.enqueueJob(jobKey,
+                () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
+                    tx -> tx.put(identifier, ace, CREATE_MISSING_PARENTS))),
+                    NeutronSecurityRuleConstants.DJC_MAX_RETRIES);
         } catch (Exception ex) {
             LOG.error("Exception occured while adding acl for security rule: {}. ", securityRule, ex);
         }
@@ -220,9 +236,18 @@ public class NeutronSecurityRuleListener
         InstanceIdentifier<Ace> identifier = getAceInstanceIdentifier(securityRule);
         try {
             Ace ace = toAceBuilder(securityRule, true).build();
-            MDSALUtil.syncUpdate(dataBroker, LogicalDatastoreType.CONFIGURATION, identifier, ace);
+            String jobKey = securityRule.getSecurityGroupId().getValue();
+            jobCoordinator.enqueueJob(jobKey,
+                () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
+                    tx -> tx.merge(identifier, ace, CREATE_MISSING_PARENTS))),
+                    NeutronSecurityRuleConstants.DJC_MAX_RETRIES);
         } catch (Exception ex) {
-            LOG.error("Exception occured while removing acl for security rule: {}. ", securityRule, ex);
+            /*
+            If there are out of sequence events where-in Sg-Rule delete could occur after SecurityGroup delete.
+            we would hit with exception here, trying to delete a child-node, after it's parent has got deleted.
+            Logging it as Warn, because if such event occur we are handling it in the AclEventListeners' Remove method.
+             */
+            LOG.warn("Exception occured while removing acl for security rule: {}. ", securityRule, ex);
         }
     }
 

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
index 2e7082b90a7732ae348ad766f4afedbb730beb83..47f62d0f876f6b9ca78fd696048e153ece230220 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
@@ -1092,12 +1092,6 @@ public class NeutronvpnUtils {
                 FloatingIpIdToPortMappingKey(floatingIpId)).build();
     }
 
-    /*static InstanceIdentifier<PortSubnet> buildPortSubnetIdentifier(String portId) {
-        InstanceIdentifier<PortSubnet> id = InstanceIdentifier.builder(PortSubnets.class)
-                .child(PortSubnet.class, new PortSubnetKey(portId)).build();
-        return id;
-    }*/
-
     // TODO Remove this method entirely
     @SuppressWarnings("checkstyle:IllegalCatch")
     private <T extends DataObject> Optional<T> read(LogicalDatastoreType datastoreType, InstanceIdentifier<T> path) {