-
Notifications
You must be signed in to change notification settings - Fork 901
ATLAS_4988: BusinessMetadata with attribute of data type Array takes time to Delete. #490
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.
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
base: master
Are you sure you want to change the base?
Conversation
…ay-type attributes.
| String qualifiedName = AtlasStructType.AtlasAttribute.getQualifiedAttributeName(businessMetadataDef, attributeDef.getName()); | ||
| String vertexPropertyName = AtlasStructType.AtlasAttribute.generateVertexPropertyName(businessMetadataDef, attributeDef, qualifiedName); | ||
| Set<String> applicableTypes = AtlasJson.fromJson(attributeDef.getOption(AtlasBusinessMetadataDef.ATTR_OPTION_APPLICABLE_ENTITY_TYPES), Set.class); | ||
| for (AtlasStructDef.AtlasAttributeDef attributeDef : businessMetadataDef.getAttributeDefs()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract the attribute iteration logic in checkBusinessMetadataRef to a private method (e.g., validateAttributeReferences) for better readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| return atlasSearchResult != null && CollectionUtils.isNotEmpty(atlasSearchResult.getEntities()); | ||
| } | ||
|
|
||
| private boolean isBusinessAttributePresentInGraph(String vertexPropertyName, Set<String> applicableTypes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Replace manual iteration with Graph Query for better performance:
The method isBusinessAttributePresentInGraph manually iterates over all vertices of each applicable type using graph.getVertices(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName), then checks each vertex for the property and state. This can be slow for large graphs, as it loads and inspects every vertex in memory. This needs to be improved
2. Add error handling and logging:
No exception handling in the graph operations.
If the graph is unavailable or queries fail, it could lead to unhandled exceptions. Wrap the query in a try-catch and log warnings/errors.
Also, add optional performance logging to monitor query times in production.
3. Add JUnit tests for the new method covering:
-Non-indexable attributes with/without assignments.
-Inactive entities (should not count).
…handling and logging.
|
|
||
| return CollectionUtils.isNotEmpty(atlasSearchResult.getEntities()); | ||
| Iterable<AtlasVertex> vertices = graph.query() | ||
| .has(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to add ENTITY_TYPE_PROPERTY_KEY filter here? If yes, note that all subTypes of types inapplicableTypes should be checked as well; consider a business attribute associated with a type like DataSet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. The ENTITY_TYPE_PROPERTY_KEY filter is not necessary and will remove these.
| Iterable<AtlasVertex> vertices = graph.query() | ||
| .has(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName) | ||
| .has(vertexPropertyName, AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null) | ||
| .has(Constants.STATE_PROPERTY_KEY, AtlasEntity.Status.ACTIVE.name()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to check only for ACTIVE entities? The business metadata can't be deleted even when a deleted entity has reference to it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank's for pointing these out, To ensure full data integrity, we should block the deletion of Business Metadata if any entity—Active or Deleted—still holds a reference to it.
ATLAS-4988
What changes were proposed in this pull request?
Background: In Apache Atlas, deleting a Business Metadata (BM) definition requires a pre-check to ensure no existing entities are assigned to those attributes. For attributes with a data type of Array (multi-valued), the deletion process was taking an exceptionally long time (e.g., over 5 minutes for -38k entities), even when the BM was not assigned to any entity.
Root Cause:
Changes Proposed:
Impact:
Performance Gain: Deletion time for Business Metadata with Array attributes reduced from -341 seconds to -1.5 second.
Reliability: Maintains strict data integrity by ensuring no "in-use" Business Metadata can be deleted.
Scalability: The fix ensures that as the number of entities grows, the deletion of metadata remains performant.
How was this patch tested?
Maven Build:
Build Successful.
Manual Testing: