Add Invalidation for Lineage Cache during updates#27606
Conversation
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| private EntityReference resolveRefForCacheInvalidation(String entityType, String id) { | ||
| EntityReference ref = new EntityReference().withId(UUID.fromString(id)); | ||
| if (nullOrEmpty(entityType)) { | ||
| return ref; | ||
| } | ||
| try { | ||
| EntityReference resolved = | ||
| Entity.getEntityReferenceById(entityType, UUID.fromString(id), Include.ALL); | ||
| return ref.withType(entityType).withFullyQualifiedName(resolved.getFullyQualifiedName()); | ||
| } catch (Exception e) { | ||
| LOG.debug( | ||
| "Could not resolve FQN for {}:{} during lineage cache invalidation", entityType, id); | ||
| return ref.withType(entityType); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Performance: Repeated DB lookups + full cache scans per edge in bulk delete
In deleteLineageFromSearch(List<EntityRelationshipObject>) (line 1264), each relation triggers two resolveRefForCacheInvalidation calls (potential DB hit each) and then invalidateLineageCacheForEdge, which scans the entire cache twice (once per FQN). For a bulk delete of N edges this is O(N × CacheSize) cache scans plus up to 2N DB queries. With the default cache size of 100 this is acceptable for small batches, but for large lineage deletions (e.g., dropping a pipeline source with many edges) it could be noticeably slow. Consider collecting all distinct FQNs first, then doing a single cache scan pass, or simply calling invalidateAllLineageCache() when the relation count exceeds a threshold.
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
There was a problem hiding this comment.
Pull request overview
This PR adds explicit lineage graph cache invalidation when lineage edges are added or removed, aiming to prevent stale lineage results being served after updates (addressing AUT failures related to lineage).
Changes:
- Introduces a
SearchClient.invalidateLineageCache(fqn)hook and implements it for Elasticsearch/OpenSearch clients. - Extends lineage cache abstractions with targeted invalidation (
invalidateIfGraphContains) and implements selective eviction for the Guava lineage cache. - Triggers cache invalidation from
LineageRepositoryafter lineage updates/removals in search.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchClient.java | Implements invalidateLineageCache delegation to the lineage graph builder |
| openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchClient.java | Implements invalidateLineageCache delegation to the lineage graph builder |
| openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java | Adds a default invalidateLineageCache API for cache eviction |
| openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/LineageGraphCache.java | Adds invalidateIfGraphContains to support targeted eviction (default falls back to full invalidation) |
| openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/GuavaLineageGraphCache.java | Implements selective eviction by scanning cached graphs for FQN references |
| openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/AbstractLineageGraphBuilder.java | Replaces TTL-only invalidation with targeted eviction via the cache |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/LineageRepository.java | Calls cache invalidation after lineage add/delete operations; resolves FQNs for bulk deletions |
| public void invalidateIfGraphContains(String fqn) { | ||
| if (fqn == null || fqn.isEmpty() || cache.size() == 0) { | ||
| return; | ||
| } | ||
| java.util.List<LineageCacheKey> toEvict = new java.util.ArrayList<>(); | ||
| for (java.util.Map.Entry<LineageCacheKey, SearchLineageResult> entry : | ||
| cache.asMap().entrySet()) { | ||
| if (graphReferencesFqn(entry.getKey(), entry.getValue(), fqn)) { | ||
| toEvict.add(entry.getKey()); | ||
| } | ||
| } | ||
| if (!toEvict.isEmpty()) { | ||
| cache.invalidateAll(toEvict); | ||
| LOG.debug("Cache INVALIDATE_FQN fqn={} evicted {} entries", fqn, toEvict.size()); | ||
| } | ||
| } |
There was a problem hiding this comment.
invalidateIfGraphContains introduces new eviction behavior but there is no unit test coverage verifying that only cache entries whose root/nodes/edges reference the given FQN are evicted (and unrelated entries remain cached). Adding a focused test here would help prevent regressions, especially since eviction logic now inspects nodes and edge endpoints.
| java.util.List<LineageCacheKey> toEvict = new java.util.ArrayList<>(); | ||
| for (java.util.Map.Entry<LineageCacheKey, SearchLineageResult> entry : | ||
| cache.asMap().entrySet()) { | ||
| if (graphReferencesFqn(entry.getKey(), entry.getValue(), fqn)) { | ||
| toEvict.add(entry.getKey()); | ||
| } | ||
| } | ||
| if (!toEvict.isEmpty()) { | ||
| cache.invalidateAll(toEvict); | ||
| LOG.debug("Cache INVALIDATE_FQN fqn={} evicted {} entries", fqn, toEvict.size()); | ||
| } | ||
| } | ||
|
|
||
| private boolean graphReferencesFqn(LineageCacheKey key, SearchLineageResult result, String fqn) { | ||
| if (fqn.equals(key.getFqn())) { | ||
| return true; | ||
| } | ||
| if (result == null) { | ||
| return false; | ||
| } | ||
| if (result.getNodes() != null && result.getNodes().containsKey(fqn)) { | ||
| return true; | ||
| } | ||
| return edgeMapReferencesFqn(result.getUpstreamEdges(), fqn) | ||
| || edgeMapReferencesFqn(result.getDownstreamEdges(), fqn); | ||
| } | ||
|
|
||
| private boolean edgeMapReferencesFqn( | ||
| java.util.Map<String, org.openmetadata.schema.api.lineage.EsLineageData> edges, String fqn) { | ||
| if (edges == null || edges.isEmpty()) { | ||
| return false; | ||
| } | ||
| for (org.openmetadata.schema.api.lineage.EsLineageData edge : edges.values()) { | ||
| if (edge.getFromEntity() != null |
There was a problem hiding this comment.
New code uses fully-qualified java.util.* and org.openmetadata.* types inside method signatures and local declarations (e.g., java.util.Map, org.openmetadata.schema.api.lineage.EsLineageData). This is inconsistent with the rest of the file’s import style and makes the new logic harder to read/maintain; please add imports and use the simple class names instead.
| searchClient.invalidateLineageCache(from.getFullyQualifiedName()); | ||
| } | ||
| if (to != null) { | ||
| searchClient.invalidateLineageCache(to.getFullyQualifiedName()); |
There was a problem hiding this comment.
invalidateLineageCacheForEdge can end up calling invalidateLineageCache(null) when FQN resolution fails (e.g., resolveRefForCacheInvalidation returns a ref without fullyQualifiedName). It currently works for the ES/OS clients because the builder guards against null/empty, but it couples callers to that implementation detail; prefer skipping the call when fullyQualifiedName is null/empty to keep the API contract safer for other SearchClient implementations.
| searchClient.invalidateLineageCache(from.getFullyQualifiedName()); | |
| } | |
| if (to != null) { | |
| searchClient.invalidateLineageCache(to.getFullyQualifiedName()); | |
| String fromFqn = from.getFullyQualifiedName(); | |
| if (!nullOrEmpty(fromFqn)) { | |
| searchClient.invalidateLineageCache(fromFqn); | |
| } | |
| } | |
| if (to != null) { | |
| String toFqn = to.getFullyQualifiedName(); | |
| if (!nullOrEmpty(toFqn)) { | |
| searchClient.invalidateLineageCache(toFqn); | |
| } |
| private EntityReference resolveRefForCacheInvalidation(String entityType, String id) { | ||
| EntityReference ref = new EntityReference().withId(UUID.fromString(id)); | ||
| if (nullOrEmpty(entityType)) { | ||
| return ref; | ||
| } | ||
| try { | ||
| EntityReference resolved = | ||
| Entity.getEntityReferenceById(entityType, UUID.fromString(id), Include.ALL); | ||
| return ref.withType(entityType).withFullyQualifiedName(resolved.getFullyQualifiedName()); | ||
| } catch (Exception e) { | ||
| LOG.debug( | ||
| "Could not resolve FQN for {}:{} during lineage cache invalidation", entityType, id); | ||
| return ref.withType(entityType); | ||
| } |
There was a problem hiding this comment.
resolveRefForCacheInvalidation performs a per-edge Entity.getEntityReferenceById(...) lookup inside a loop over relations. For large lineage deletions (e.g., deleting by source/pipeline), this becomes an N+1 pattern with potentially many DB calls and can noticeably slow down the operation. Consider resolving references in batch (group IDs by type and use Entity.getEntityReferencesByIds(...)) or changing the DAO query to also return FQNs when available, so cache invalidation doesn’t require per-row fetches.
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsImplements lineage cache invalidation during updates to ensure data consistency. Consider optimizing bulk delete operations to avoid repeated database lookups and redundant full cache scans per edge. 💡 Performance: Repeated DB lookups + full cache scans per edge in bulk delete📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/LineageRepository.java:1264-1272 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/LineageRepository.java:1274-1288 In 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
🟡 Playwright Results — all passed (18 flaky)✅ 3693 passed · ❌ 0 failed · 🟡 18 flaky · ⏭️ 89 skipped
🟡 18 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
* Add Invalidation for Lineage Cache during updates * Add test (cherry picked from commit 557626f)
* Add Invalidation for Lineage Cache during updates * Add test
Describe your changes:
Fixes issue observer in AUTs failing with DataASsetLineageSpec.ts errors
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
GuavaLineageGraphCacheTestto verify selective cache eviction based on FQNs within graph nodes and edge endpoints.AbstractLineageGraphBuilderTestto validate thatinvalidateLineageCacheForFqncorrectly removes relevant cached results while preserving unrelated entries.This will update automatically on new commits.