Skip to content

MLH-1151 | optimise lineage calculation #5386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

MLH-1151 | optimise lineage calculation #5386

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41531df
optimise lineage calculation
aarshi0301 Sep 17, 2025
53faac0
add branch for canary
aarshi0301 Sep 25, 2025
8c5945a
optimise lineage calculation
aarshi0301 Oct 1, 2025
865b2c7
Merge branch 'master' into mindbodylineage
aarshi0301 Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 16 additions & 18 deletions repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@
import static org.apache.atlas.repository.store.graph.v2.AtlasGraphUtilsV2.getState;
import static org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.__.id;
import static org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.__.outV;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.hasId;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.not;

public abstract class DeleteHandlerV1 {
public static final Logger LOG = LoggerFactory.getLogger(DeleteHandlerV1.class);
Expand Down Expand Up @@ -1895,9 +1897,10 @@ private void updateAssetHasLineageStatusV2(AtlasVertex assetVertex, AtlasEdge cu

/**
* Helper method to check for active lineage in a specific direction
*
* @param assetVertex The vertex to check
* @param currentEdge The current edge to exclude
* @param direction The edge direction to explore
* @param direction The edge direction to explore
* @return True if active lineage exists in the specified direction
*/
private boolean hasActiveLineageDirection(AtlasVertex assetVertex, AtlasEdge currentEdge, Direction direction) {
Expand All @@ -1915,24 +1918,19 @@ private boolean hasActiveLineageDirection(AtlasVertex assetVertex, AtlasEdge cur
.has(STATE_PROPERTY_KEY, ACTIVE_STATE_VALUE);
}

// Complete the traversal with common operations

// Filter out current edge using Gremlin
traversal = traversal.where(not(hasId(currentEdge.getIdForDisplay())));
Copy link

@sriram-atlan sriram-atlan Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont see this change behind FF @aarshi0301


for (String deletedEdgeId : RequestContext.get().getDeletedEdgesIdsForResetHasLineage()) {
traversal = traversal.where(not(hasId(deletedEdgeId)));
}
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop modifies the traversal object repeatedly, which could create performance issues if there are many deleted edge IDs. Consider building a single filter condition or using a more efficient batch filtering approach to avoid multiple traversal modifications.

Suggested change
// Filter out current edge using Gremlin
traversal = traversal.where(not(hasId(currentEdge.getIdForDisplay())));
for (String deletedEdgeId : RequestContext.get().getDeletedEdgesIdsForResetHasLineage()) {
traversal = traversal.where(not(hasId(deletedEdgeId)));
}
// Filter out current edge and all deleted edges using a single Gremlin filter
Set<String> excludedEdgeIds = new HashSet<>();
excludedEdgeIds.add(currentEdge.getIdForDisplay());
excludedEdgeIds.addAll(RequestContext.get().getDeletedEdgesIdsForResetHasLineage());
traversal = traversal.where(not(hasId(excludedEdgeIds.toArray(new String[0]))));

Copilot uses AI. Check for mistakes.


return traversal
.project("id", HAS_LINEAGE)
.by(id())
.by(outV().values(HAS_LINEAGE))
.toStream()
.anyMatch(edge -> {
Object edgeId = edge.get("id");
String edgeIdStr = (edgeId != null) ? edgeId.toString() : "";

// Skip if in deleted list or matches current edge
if (RequestContext.get().getDeletedEdgesIdsForResetHasLineage().contains(edgeIdStr) ||
currentEdge.getIdForDisplay().equals(edgeIdStr)) {
return false;
}
.outV() // Get the connected vertex
.has(HAS_LINEAGE, true) // Filter vertices with lineage=true
.hasNext(); // Just check existence, don't materialize using stream()
Copy link

@cursor cursor bot Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect Lineage Calculation and Edge Traversal

The hasActiveLineageDirection method may calculate lineage incorrectly. It passes string representations of edge IDs to hasId(), which expects actual ID objects, causing filtering to fail. Additionally, the traversal uses .outV() universally, which is incorrect for outgoing edges as it returns the source vertex instead of the connected target.

Fix in Cursor Fix in Web


// Check if this edge has lineage
return Boolean.TRUE.equals(edge.get(HAS_LINEAGE));
});
}
}
Loading