fix(dao): use canonical DB-stored URN when emitting MAE events#628
Merged
mridul111998 merged 2 commits intoJul 3, 2026
Merged
Conversation
mridul111998
force-pushed
the
research/fix-canonical-urn-in-mae
branch
5 times, most recently
from
f67d963 to
0c75f43
Compare
When two MCEs arrive with different-case URNs for the same entity (e.g. lixTrackingArchive vs LixTrackingArchive), MySQL's CI collation stores only one row, but the MAE was emitted with the incoming MCE URN. This caused duplicate Elasticsearch documents (one per case variant) leading to duplicate browse results in Datahub. Fix: extract the canonical URN from ExtraInfo.getUrn() (the URN as stored in the DB row) before shouldUpdateAspect(), so both the no-op early-return path and the update path carry the canonical URN. Thread it through AddResult into unwrapAddResult() (dropping the now- redundant 'urn' param) where it is used for all producer/hook calls. For new entities with no existing DB row, falls back to incoming URN. Also fixes the same issue in the batch path (addManyBatchInternal). Test: testBothCaseVariantsProduceSameCanonicalUrnInMAE uses BurgerUrn (string-keyed) to simulate two genuinely different-case URN variants and verifies both MAE calls use the canonical DB-stored URN. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mridul111998
force-pushed
the
research/fix-canonical-urn-in-mae
branch
from
d60c573 to
d599c9d
Compare
jphui
approved these changes
Jul 3, 2026
| // newValue can in theory be NULL outside of deletion operations, we need to check for that here. | ||
| if (_aspectPostUpdateHooksMap.containsKey(aspectClass) && !isDeletion) { | ||
| _aspectPostUpdateHooksMap.get(aspectClass).forEach(hook -> hook.accept(urn, newValue)); | ||
| _aspectPostUpdateHooksMap.get(aspectClass).forEach(hook -> hook.accept(maeUrn, newValue)); |
Contributor
There was a problem hiding this comment.
IIUC this is the core change, rather than use the urn from the payload, use the urn from the DB
Contributor
Author
There was a problem hiding this comment.
yes correct, and the main this is how we are passing this urn from all the places without adding a new extra DB call
yangyangv2
reviewed
Jul 3, 2026
yangyangv2
left a comment
yangyangv2
left a comment
Contributor
There was a problem hiding this comment.
Look good to me, thanks for making the fix!
Please fix the build failure and update the comments, then ship-it
Expand one-liner method bodies to multi-line and remove redundant same-package imports (AspectWithExtraInfo, ListResult) to satisfy checkstyle rules. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use the canonical DB-stored URN (from
ExtraInfo.getUrn()) when emitting MAE events inBaseLocalDAO, instead of the incoming MCE URN. This fix applies to all entity types and all indexes since it is in the core DAO layer.👉 Why is this required?
Two HDFS clusters (WAR and Holdem) emit MCEs with different casing for the same dataset path (e.g.
LixTrackingArchivevslixTrackingArchive). Because the Hosted Search document ID is derived from the URN, two different document IDs were created → duplicate browse results in Datahub.The root cause:
addCommon()was emitting MAE with the incoming MCE URN instead of the canonical URN stored in the DB. The DB (MySQL) normalises casing via CI collation, so there is only ever one row — but two different HS documents were being created.✅ How was this tested?
testBothCaseVariantsProduceSameCanonicalUrnInMAEinBaseLocalDAOTestusing a string-keyedBurgerUrnto simulate two MCEs with different-case URNs arriving for the same entity. Verifies that both MAEs are emitted with the canonical (DB-stored) URN and the upper-case URN never appears.makeAspectEntry()test helper inBaseLocalDAOTestandBaseLocalDAOAspectVersionTestto require an explicit@Nullable Urnparameter, mirroring production behaviour whereExtraInfoalways carries the stored URN for existing entities. This eliminatedRequiredFieldNotPresentExceptionfailures in existing tests.makeAspectEntryhelper into a single static method inBaseLocalDAOTest, shared via static import.dao-apitest suite passes (244 tests).