feat(ui) Add upgrade step to enable CLL impact analysis for existing data #6427

chriscollins3456 · 2022-11-14T18:00:41Z

Adds a new upgrade step for restoring indices for the upstreamLineage aspects as well as the inputFields aspect. The upstreamLineage aspect has fineGrainedLineage that needs to be reindexed with our new logic for impact analysis. Same this with inputFields. After this, users will have their graph index set up properly to do impact analysis with all columns.

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

github-actions · 2022-11-14T18:36:14Z

Unit Test Results (build & test)

616 tests +3 612 ✔️ +3 15m 43s ⏱️ + 3m 55s
152 suites +1     4 💤 ±0
152 files +1     0 ❌ ±0

Results for commit 21434f1. ± Comparison against base commit 7ea9797.

♻️ This comment has been updated with latest results.

jjoyce0510 · 2022-11-14T20:49:00Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndices.java

+
+  private int getAndRestoreUpstreamLineageIndices(int start, AuditStamp auditStamp)
+      throws Exception {
+    final AspectSpec upstreamLineageAspectSpec = _entityRegistry.getEntitySpec(Constants.DATASET_ENTITY_NAME)


Do we need this AspectSpec? Is there any other method on entityService to produce MCL that doesn't require the AspectSpec?

hmm all the methods I'm seeing from entityService that produce an MCL use an aspect spec. is there something specific you had in mind/know about?

jjoyce0510 · 2022-11-14T20:49:40Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndices.java

+    final AspectSpec upstreamLineageAspectSpec = _entityRegistry.getEntitySpec(Constants.DATASET_ENTITY_NAME)
+        .getAspectSpec(Constants.UPSTREAM_LINEAGE_ASPECT_NAME);
+    SearchResult datasetResult =
+        _entitySearchService.search(Constants.DATASET_ENTITY_NAME, "", null, null, start, BATCH_SIZE);


For a single entity type, this will only work up to 10k entities. After 10k, elastic doesn't allow us to paginate by default

It's something to note. If you want fuller ability to paginate you can try to use the "scroll" method.

I'm also wondering if there's a way to do this that doesn't require that we go to elasticsearch. There is a listLatestAspects method on the EntityService class that would allow you get the latest of a specific aspect for every URN. That may help here...

Also, is there any Field-Level Lineage in DataJob -> Dataset that we need to worry about?

(listLatestAspects will hit MySQL in a slightly more performant query)

(It's used in the browse path upgrade step as well)

sounds good! moved to using listLatestAspects - gonna be faster and more efficient overall

jjoyce0510 · 2022-11-14T21:05:23Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndices.java

+
+    return entityResult.getNumEntities();
+  }
+


Minor: Consider adding @nullable

jjoyce0510 · 2022-11-14T21:06:11Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndices.java

+  public RestoreColumnLineageIndices(EntityService entityService, EntitySearchService entitySearchService,
+                                EntityRegistry entityRegistry) {
+    super(entityService, VERSION, UPGRADE_ID);
+    _entitySearchService = entitySearchService;


Minor -> consider adding

_entitySearchService = Objects.requireNonNull(entitySearchService, 'entitySearchService must not be null'

Basically this allows us to fail faster (in the constructor at init time) as opposed to waiting for an NPE to arise

Also add @nonnull and final annotations on these parameters!

jjoyce0510 · 2022-11-14T21:06:51Z

...actories/src/test/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndicesTest.java

+    final Urn upgradeEntityUrn = UrnUtils.getUrn(COLUMN_LINEAGE_UPGRADE_URN);
+    com.linkedin.upgrade.DataHubUpgradeRequest upgradeRequest = new com.linkedin.upgrade.DataHubUpgradeRequest().setVersion(version);
+    Map<String, EnvelopedAspect> upgradeRequestAspects = new HashMap<>();
+    upgradeRequestAspects.put(Constants.DATA_HUB_UPGRADE_REQUEST_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(upgradeRequest.data())));


Very thorough! Nice!!

aditya-radhakrishnan · 2022-11-15T01:28:42Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndices.java

+  public RestoreColumnLineageIndices(EntityService entityService, EntitySearchService entitySearchService,
+                                EntityRegistry entityRegistry) {
+    super(entityService, VERSION, UPGRADE_ID);
+    _entitySearchService = entitySearchService;


Also add @nonnull and final annotations on these parameters!

aditya-radhakrishnan · 2022-11-15T02:10:03Z

...actories/src/test/java/com/linkedin/metadata/boot/steps/RestoreColumnLineageIndicesTest.java

+import java.util.HashSet;
+import java.util.Map;
+
+public class RestoreColumnLineageIndicesTest {


Thanks for adding these tests!

aditya-radhakrishnan

LGTM!

…data (datahub-project#6427)

add upgrade step for upstreamLineage and inputFields

e82ea65

chriscollins3456 requested a review from aditya-radhakrishnan November 14, 2022 18:22

jjoyce0510 reviewed Nov 14, 2022

View reviewed changes

aditya-radhakrishnan reviewed Nov 15, 2022

View reviewed changes

PR edits

21434f1

aditya-radhakrishnan approved these changes Nov 15, 2022

View reviewed changes

aditya-radhakrishnan added the product PR or Issue related to the DataHub UI/UX label Nov 15, 2022

chriscollins3456 merged commit e8aaf2a into datahub-project:master Nov 15, 2022

cccs-Dustin pushed a commit to CybercentreCanada/datahub that referenced this pull request Feb 1, 2023

feat(ui) Add upgrade step to enable CLL impact analysis for existing …

8330a45

…data (datahub-project#6427)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui) Add upgrade step to enable CLL impact analysis for existing data #6427

feat(ui) Add upgrade step to enable CLL impact analysis for existing data #6427

chriscollins3456 commented Nov 14, 2022

github-actions bot commented Nov 14, 2022 •

edited

Loading

jjoyce0510 Nov 14, 2022

chriscollins3456 Nov 15, 2022

jjoyce0510 Nov 14, 2022

jjoyce0510 Nov 14, 2022

jjoyce0510 Nov 14, 2022

jjoyce0510 Nov 14, 2022

chriscollins3456 Nov 15, 2022

jjoyce0510 Nov 14, 2022

jjoyce0510 Nov 14, 2022

aditya-radhakrishnan Nov 15, 2022

chriscollins3456 Nov 15, 2022

jjoyce0510 Nov 14, 2022

aditya-radhakrishnan Nov 15, 2022

aditya-radhakrishnan Nov 15, 2022

aditya-radhakrishnan left a comment

feat(ui) Add upgrade step to enable CLL impact analysis for existing data #6427

feat(ui) Add upgrade step to enable CLL impact analysis for existing data #6427

Conversation

chriscollins3456 commented Nov 14, 2022

Checklist

github-actions bot commented Nov 14, 2022 • edited Loading

Unit Test Results (build & test)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aditya-radhakrishnan left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2022 •

edited

Loading