From 0e53c3ee3a135f86641e5ba6d4bf64502b1645ed Mon Sep 17 00:00:00 2001
From: Sarath Subramanian <sarath@apache.org>
Date: Sun, 3 May 2020 17:01:08 -0700
Subject: [PATCH] ATLAS-3778: Improve performance during classification delete

---
 graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java              | 10 ++++++++++
 graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java | 30 ++++++++++++++++++++++++++++++
 repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java     | 45 +++++++++++++++++++++------------------------
 repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java   | 37 +++++++++++++++++--------------------
 4 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java b/graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
index 4af39ed..ef8bd42 100644
--- a/graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
+++ b/graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
@@ -111,6 +111,16 @@ public interface AtlasElement {
     void removeProperty(String propertyName);
 
     /**
+     * Removes a property with given property value from the vertex.
+     */
+    void removePropertyValue(String propertyName, Object propertyValue);
+
+    /**
+     * Removes a property with given property value from the vertex.
+     */
+    void removeAllPropertyValue(String propertyName, Object propertyValue);
+
+    /**
      * Sets a single-valued property to the given value.  For
      * properties defined as multiplicty many in the graph schema, the value is added instead
      * (following set semantics)
diff --git a/graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java b/graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java
index a9cc5a6..66ee4e9 100644
--- a/graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java
+++ b/graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java
@@ -22,6 +22,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.atlas.repository.graphdb.AtlasEdge;
@@ -107,6 +108,35 @@ public class AtlasJanusElement<T extends Element> implements AtlasElement {
     }
 
     @Override
+    public void removePropertyValue(String propertyName, Object propertyValue) {
+        Iterator<? extends Property<Object>> it = getWrappedElement().properties(propertyName);
+
+        while (it.hasNext()) {
+            Property currentProperty      = it.next();
+            Object   currentPropertyValue = currentProperty.value();
+
+            if (Objects.equals(currentPropertyValue, propertyValue)) {
+                currentProperty.remove();
+                break;
+            }
+        }
+    }
+
+    @Override
+    public void removeAllPropertyValue(String propertyName, Object propertyValue) {
+        Iterator<? extends Property<Object>> it = getWrappedElement().properties(propertyName);
+
+        while (it.hasNext()) {
+            Property currentProperty      = it.next();
+            Object   currentPropertyValue = currentProperty.value();
+
+            if (Objects.equals(currentPropertyValue, propertyValue)) {
+                currentProperty.remove();
+            }
+        }
+    }
+
+    @Override
     public void setProperty(String propertyName, Object value) {
         try {
             if (value == null) {
diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
index 5643599..3f8503a 100644
--- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
+++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
@@ -993,42 +993,39 @@ public abstract class DeleteHandlerV1 {
         }
         entityVertex.addListProperty(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, classificationName);
 
-        String propClsNames = entityVertex.getProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, String.class);
-
-        propClsNames = StringUtils.isEmpty(propClsNames) ? CLASSIFICATION_NAME_DELIMITER + classificationName
-                                                         : (propClsNames + classificationName);
-
-        propClsNames = propClsNames + CLASSIFICATION_NAME_DELIMITER;
-        entityVertex.setProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, propClsNames);
+        entityVertex.setProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, getDelimitedPropagatedClassificationNames(entityVertex, classificationName));
     }
 
-    private void removeFromPropagatedClassificationNames(AtlasVertex entityVertex, String classificationName) {
+    public void removeFromPropagatedClassificationNames(AtlasVertex entityVertex, String classificationName) {
         if (entityVertex != null && StringUtils.isNotEmpty(classificationName)) {
-            List<String> propagatedTraitNames = getTraitNames(entityVertex, true);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Removing from property: {} value: {} in vertex: {}", PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, classificationName, string(entityVertex));
+            }
 
-            propagatedTraitNames.remove(classificationName);
+            entityVertex.removePropertyValue(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, classificationName);
 
-            setPropagatedClassificationNames(entityVertex, propagatedTraitNames);
-        }
-    }
+            List<String> propagatedTraitNames = getPropagatedTraitNames(entityVertex);
 
-    private void setPropagatedClassificationNames(AtlasVertex entityVertex, List<String> classificationNames) {
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("Adding property {} = \"{}\" to vertex {}", PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, classificationNames, string(entityVertex));
-        }
+            if (CollectionUtils.isNotEmpty(propagatedTraitNames)) {
+                propagatedTraitNames.remove(classificationName);
 
-        entityVertex.removeProperty(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY);
-        entityVertex.removeProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY);
+                String propClsName = CLASSIFICATION_NAME_DELIMITER + StringUtils.join(propagatedTraitNames, CLASSIFICATION_NAME_DELIMITER) + CLASSIFICATION_NAME_DELIMITER;
 
-        if (CollectionUtils.isNotEmpty(classificationNames)) {
-            for (String classificationName : classificationNames) {
-                entityVertex.addListProperty(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, classificationName);
+                entityVertex.setProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, propClsName);
             }
+        }
+    }
 
-            String propClsName = CLASSIFICATION_NAME_DELIMITER + StringUtils.join(classificationNames, CLASSIFICATION_NAME_DELIMITER) + CLASSIFICATION_NAME_DELIMITER;
+    private String getDelimitedPropagatedClassificationNames(AtlasVertex entityVertex, String classificationName) {
+        String ret = entityVertex.getProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, String.class);
 
-            entityVertex.setProperty(PROPAGATED_CLASSIFICATION_NAMES_KEY, propClsName);
+        if (StringUtils.isEmpty(ret)) {
+            ret = CLASSIFICATION_NAME_DELIMITER + classificationName + CLASSIFICATION_NAME_DELIMITER;
+        } else {
+            ret = ret + classificationName + CLASSIFICATION_NAME_DELIMITER;
         }
+
+        return ret;
     }
 
     /**
diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
index 5564bc5..2ed524f 100644
--- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
+++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
@@ -2086,7 +2086,13 @@ public class EntityGraphMapper {
 
         traitNames.remove(classificationName);
 
-        setClassificationNames(entityVertex, traitNames);
+        // update 'TRAIT_NAMES_PROPERTY_KEY' property
+        entityVertex.removePropertyValue(TRAIT_NAMES_PROPERTY_KEY, classificationName);
+
+        // update 'CLASSIFICATION_NAMES_KEY' property
+        entityVertex.removeProperty(CLASSIFICATION_NAMES_KEY);
+
+        entityVertex.setProperty(CLASSIFICATION_NAMES_KEY, getClassificationNamesString(traitNames));
 
         updateModificationMetadata(entityVertex);
 
@@ -2140,30 +2146,21 @@ public class EntityGraphMapper {
     private void addToClassificationNames(AtlasVertex entityVertex, String classificationName) {
         AtlasGraphUtilsV2.addEncodedProperty(entityVertex, TRAIT_NAMES_PROPERTY_KEY, classificationName);
 
-        String clsNames = entityVertex.getProperty(CLASSIFICATION_NAMES_KEY, String.class);
-
-        clsNames = StringUtils.isEmpty(clsNames) ? CLASSIFICATION_NAME_DELIMITER + classificationName : clsNames + classificationName;
+        String delimitedClassificationNames = entityVertex.getProperty(CLASSIFICATION_NAMES_KEY, String.class);
 
-        clsNames = clsNames + CLASSIFICATION_NAME_DELIMITER;
+        if (StringUtils.isEmpty(delimitedClassificationNames)) {
+            delimitedClassificationNames = CLASSIFICATION_NAME_DELIMITER + classificationName + CLASSIFICATION_NAME_DELIMITER;
+        } else {
+            delimitedClassificationNames = delimitedClassificationNames + classificationName + CLASSIFICATION_NAME_DELIMITER;
+        }
 
-        entityVertex.setProperty(CLASSIFICATION_NAMES_KEY, clsNames);
+        entityVertex.setProperty(CLASSIFICATION_NAMES_KEY, delimitedClassificationNames);
     }
 
-    private void setClassificationNames(AtlasVertex entityVertex, List<String> traitNames) {
-        if (entityVertex != null) {
-            entityVertex.removeProperty(TRAIT_NAMES_PROPERTY_KEY);
-            entityVertex.removeProperty(CLASSIFICATION_NAMES_KEY);
+    private String getClassificationNamesString(List<String> traitNames) {
+        String ret = StringUtils.join(traitNames, CLASSIFICATION_NAME_DELIMITER);
 
-            for (String traitName : traitNames) {
-                AtlasGraphUtilsV2.addEncodedProperty(entityVertex, TRAIT_NAMES_PROPERTY_KEY, traitName);
-            }
-
-            String clsNames = StringUtils.join(traitNames, CLASSIFICATION_NAME_DELIMITER);
-
-            clsNames = StringUtils.isEmpty(clsNames) ? clsNames : CLASSIFICATION_NAME_DELIMITER + clsNames + CLASSIFICATION_NAME_DELIMITER;
-
-            entityVertex.setProperty(CLASSIFICATION_NAMES_KEY, clsNames);
-        }
+        return StringUtils.isEmpty(ret) ? ret : CLASSIFICATION_NAME_DELIMITER + ret + CLASSIFICATION_NAME_DELIMITER;
     }
 
     public void updateClassifications(EntityMutationContext context, String guid, List<AtlasClassification> classifications) throws AtlasBaseException {
--
libgit2 0.27.1