Skip to content
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

PLT-1759 | Refactor to use AtlasTypeRegistry to check for Duplicate Business Metadata Names #3319

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AnarghyaJ-atlan
Copy link
Collaborator

There was a performance issue identified on one of the Customer calls that creation of Custom Metadata was slow and adding Attributes to the Metadata was overshooting the allocated 30 seconds timeout on UI.

This led to investigation on identifying performance bottleneck for

  1. Creating Custom Business Metadata (Ueer defined typedef)
  2. Adding Attributes to existing Business Metadata.

The postmortem of the above is available here:
https://www.notion.so/atlanhq/Postmortem-Atlas-slowness-for-Custom-metadata-2b80ea2464044d4a95b66e92a6eeb433?pvs=4

One of the alternatives to solve this was suggested by @mehtaanshul to use AtlasTypeRegistry rather than traversing un-indexed graph store.

refactor the check for uniqueness to use typeRegistry instead of querying un-indexed attribute from atlasGraph
businessMetadataDef.getDisplayName(), DataTypes.TypeCategory.BUSINESS_METADATA);
if (ret != null && (
businessMetadataDef.getGuid() == null || !businessMetadataDef.getGuid().equals(ret.getProperty(Constants.GUID_PROPERTY_KEY, String.class)))) {
AtlasBusinessMetadataType ret = typeRegistry.getBusinessMetadataTypeByDisplayName(businessMetadataDef.getDisplayName());

Choose a reason for hiding this comment

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

Possible to cover this with an unit test??
If we don't have any existing tests, we can add one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@anbarasantr Yes. Right now we don't cover this via Unit test. But should be able to do. Will add

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants