-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,10 +27,13 @@ | |
| import org.apache.atlas.model.TypeCategory; | ||
| import org.apache.atlas.model.discovery.AtlasSearchResult; | ||
| import org.apache.atlas.model.discovery.SearchParameters; | ||
| import org.apache.atlas.model.instance.AtlasEntity; | ||
| import org.apache.atlas.model.typedef.AtlasBaseTypeDef; | ||
| import org.apache.atlas.model.typedef.AtlasBusinessMetadataDef; | ||
| import org.apache.atlas.model.typedef.AtlasStructDef; | ||
| import org.apache.atlas.repository.Constants; | ||
| import org.apache.atlas.repository.graphdb.AtlasGraph; | ||
| import org.apache.atlas.repository.graphdb.AtlasGraphQuery; | ||
| import org.apache.atlas.repository.graphdb.AtlasVertex; | ||
| import org.apache.atlas.type.AtlasBusinessMetadataType; | ||
| import org.apache.atlas.type.AtlasStructType; | ||
|
|
@@ -58,6 +61,9 @@ public class AtlasBusinessMetadataDefStoreV2 extends AtlasAbstractDefStoreV2<Atl | |
|
|
||
| private final EntityDiscoveryService entityDiscoveryService; | ||
|
|
||
| @Inject | ||
| private AtlasGraph graph; | ||
|
|
||
| @Inject | ||
| public AtlasBusinessMetadataDefStoreV2(AtlasTypeDefGraphStoreV2 typeDefStore, AtlasTypeRegistry typeRegistry, EntityDiscoveryService entityDiscoveryService) { | ||
| super(typeDefStore, typeRegistry); | ||
|
|
@@ -363,44 +369,91 @@ private AtlasBusinessMetadataDef toBusinessMetadataDef(AtlasVertex vertex) throw | |
| private void checkBusinessMetadataRef(String typeName) throws AtlasBaseException { | ||
| AtlasBusinessMetadataDef businessMetadataDef = typeRegistry.getBusinessMetadataDefByName(typeName); | ||
|
|
||
| if (businessMetadataDef != null) { | ||
| List<AtlasStructDef.AtlasAttributeDef> attributeDefs = businessMetadataDef.getAttributeDefs(); | ||
| if (businessMetadataDef == null || CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) { | ||
| return; | ||
| } | ||
|
|
||
| for (AtlasStructDef.AtlasAttributeDef attributeDef : attributeDefs) { | ||
| 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()) { | ||
| validateAttributeReferences(businessMetadataDef, attributeDef); | ||
| } | ||
| } | ||
|
|
||
| if (CollectionUtils.isNotEmpty(applicableTypes) && isBusinessAttributePresent(vertexPropertyName, applicableTypes)) { | ||
| throw new AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName); | ||
| } | ||
| } | ||
| private void validateAttributeReferences(AtlasBusinessMetadataDef bmDef, AtlasStructDef.AtlasAttributeDef attributeDef) throws AtlasBaseException { | ||
| String applicableTypesStr = attributeDef.getOption(ATTR_OPTION_APPLICABLE_ENTITY_TYPES); | ||
| Set<String> applicableTypes = StringUtils.isBlank(applicableTypesStr) ? null : AtlasJson.fromJson(applicableTypesStr, Set.class); | ||
|
|
||
| if (CollectionUtils.isEmpty(applicableTypes)) { | ||
| return; | ||
| } | ||
|
|
||
| String qualifiedName = AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef, attributeDef.getName()); | ||
| String vertexPropertyName = AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef, qualifiedName); | ||
|
|
||
| boolean isPresent; | ||
| long startTime = System.currentTimeMillis(); | ||
|
|
||
| if (Boolean.TRUE.equals(attributeDef.getIsIndexable())) { | ||
| isPresent = isBusinessAttributePresent(vertexPropertyName, applicableTypes); | ||
| } else { | ||
| isPresent = isBusinessAttributePresentInGraph(vertexPropertyName, applicableTypes); | ||
| } | ||
|
|
||
| if (LOG.isDebugEnabled()) { | ||
| LOG.info("Reference check for attribute {} took {} ms. Found: {}", | ||
| attributeDef.getName(), (System.currentTimeMillis() - startTime), isPresent); | ||
| } | ||
|
|
||
| if (isPresent) { | ||
| throw new AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, bmDef.getName()); | ||
| } | ||
| } | ||
|
|
||
| private boolean isBusinessAttributePresent(String attrName, Set<String> applicableTypes) throws AtlasBaseException { | ||
| SearchParameters.FilterCriteria criteria = new SearchParameters.FilterCriteria(); | ||
|
|
||
| criteria.setAttributeName(attrName); | ||
| criteria.setOperator(SearchParameters.Operator.NOT_EMPTY); | ||
| criteria.setOperator(SearchParameters.Operator.NOT_NULL); | ||
|
|
||
| SearchParameters.FilterCriteria entityFilters = new SearchParameters.FilterCriteria(); | ||
|
|
||
| entityFilters.setCondition(SearchParameters.FilterCriteria.Condition.OR); | ||
| entityFilters.setCriterion(Collections.singletonList(criteria)); | ||
|
|
||
| SearchParameters searchParameters = new SearchParameters(); | ||
|
|
||
| searchParameters.setTypeName(String.join(SearchContext.TYPENAME_DELIMITER, applicableTypes)); | ||
| searchParameters.setExcludeDeletedEntities(true); | ||
| searchParameters.setIncludeSubClassifications(false); | ||
| searchParameters.setEntityFilters(entityFilters); | ||
| searchParameters.setAttributes(Collections.singleton(attrName)); | ||
| searchParameters.setOffset(0); | ||
| searchParameters.setAttributes(Collections.emptySet()); | ||
| searchParameters.setLimit(1); | ||
|
|
||
| AtlasSearchResult atlasSearchResult = entityDiscoveryService.searchWithParameters(searchParameters); | ||
| return atlasSearchResult != null && CollectionUtils.isNotEmpty(atlasSearchResult.getEntities()); | ||
| } | ||
|
|
||
| private boolean isBusinessAttributePresentInGraph(String vertexPropertyName, Set<String> applicableTypes) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Replace manual iteration with Graph Query for better performance: 2. Add error handling and logging: 3. Add JUnit tests for the new method covering: |
||
| if (graph == null || CollectionUtils.isEmpty(applicableTypes)) { | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| for (String typeName : applicableTypes) { | ||
|
|
||
| return CollectionUtils.isNotEmpty(atlasSearchResult.getEntities()); | ||
| Iterable<AtlasVertex> vertices = graph.query() | ||
| .has(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it necessary to add
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| .has(vertexPropertyName, AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null) | ||
| .has(Constants.STATE_PROPERTY_KEY, AtlasEntity.Status.ACTIVE.name()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| .vertices(); | ||
|
|
||
| if (vertices != null && vertices.iterator().hasNext()) { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.info("Found at least one reference for {} on type {}", vertexPropertyName, typeName); | ||
| } | ||
| return true; | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| LOG.error("Error occurred while querying graph for references of property: {}", vertexPropertyName, e); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
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
checkBusinessMetadataRefto 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