perf(search): default-exclude heavy fields (embeddings, upstreamLineage, ...) from search responses - #29517
perf(search): default-exclude heavy fields (embeddings, upstreamLineage, ...) from search responses#29517mohityadav766 wants to merge 4 commits into
Conversation
The main /search/query path applied no default _source filter — it only honored caller-provided include/exclude fields — so normal explore/search shipped full _source including embeddings (vector search only), upstreamLineage (lineage pages only, and already stripped post-fetch), the *_suggest fields, schemaDefinition and customMetrics. These bloat every search response and drive memory/GC pressure under concurrent load. Apply SearchUtils.DEFAULT_SEARCH_SOURCE_EXCLUDES (the existing SOURCE_FIELDS_TO_EXCLUDE set + upstreamLineage) as the default _source exclude on the OS and ES search paths, merged with any caller-provided excludes. Callers that need a field opt in via includeSourceFields, so lineage and vector-search paths are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
🔴 Playwright Results — 14 failure(s), 218 flaky✅ 4251 passed · ❌ 14 failed · 🟡 218 flaky · ⏭️ 38 skipped
Genuine Failures (failed on all attempts)❌
|
|
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
|
1 participant
Fixes #29516
Problem
The main
/search/querypath (OpenSearchSearchManager/ElasticSearchSearchManager) applies no default_sourcefilter — only caller-provided include/exclude fields. So normal explore/search ships the full_source, including fields the UI doesn't need:embedding— large vectors, used only by the vector-search endpointupstreamLineage— lineage pages only (and alreadynode.remove(...)'d post-fetch in some paths — fetched, parsed, then discarded)*_suggest,schemaDefinition,customMetrics,lifeCycle,fqnPartsThese are already defined as excludable (
SOURCE_FIELDS_TO_EXCLUDE) and stripped on lineage / entity-relationship paths — just not on the main search path. They bloat every search response and add memory/GC pressure under load (same class as the column-aggregator OOM, #29501/#29502).Fix
SearchUtils.DEFAULT_SEARCH_SOURCE_EXCLUDES=SOURCE_FIELDS_TO_EXCLUDE+upstreamLineage, andwithDefaultSearchSourceExcludes(...)to merge with caller excludes.includeSourceFields(lineage, vector search, etc.) and explicitfetchSource=falsebehave as before; explicitexcludeSourceFieldsare merged with the defaults.Safety
Lineage and vector-search endpoints request their own source fields, so they're unaffected. The excluded fields were already the established "exclude from search source" set; this just extends them to the high-traffic explore/search path.
Type of change
Testing
SearchUtilsTestgreen (142/142), incl. a new test asserting the default excludes dropembedding/upstreamLineage/schemaDefinition/customMetricsand that caller excludes are merged (not replaced).openmetadata-servicecompiles clean.🤖 Generated with Claude Code
Greptile Summary
This PR applies default
_sourceexclusions (embedding,upstreamLineage, suggest fields,schemaDefinition,customMetrics) to the main/search/querypath in bothElasticSearchSearchManagerandOpenSearchSearchManager, reducing response payload size and GC pressure without touching lineage or vector-search paths that opt in viaincludeSourceFields.SearchUtilsgainsDEFAULT_SEARCH_SOURCE_EXCLUDESandwithDefaultSearchSourceExcludesto merge caller excludes with the defaults; the underlying constant is still sourced fromElasticSearchClient.SOURCE_FIELDS_TO_EXCLUDE(pre-existing cross-engine coupling).fetchSource=falsestill skips the source entirely, explicitincludeSourceFieldsstill bypass the defaults, and only the "no include, no false" path is changed to apply the default excludes.Set.of(String[])which Java resolves to the fixed-aritySet.of(E e1)overload — producing a singletonSet<String[]>rather than aSet<String>— and would fail at runtime.Confidence Score: 4/5
The production filtering logic is correct and all existing callers are unaffected; the new test has assertion bugs that are likely failing in CI.
The two managers and SearchUtils are correctly implemented and all existing callers that use includeSourceFields or fetchSource=false continue to behave as before. The only concern is in the new test: Set.of(withDefaultSearchSourceExcludes(null)) uses Java's fixed-arity Set.of(E e1) overload (not the varargs overload), creating a singleton Set<String[]> that would not equal the Set of defaults, causing the assertEquals and both assertTrue calls to fail at runtime. The first assertion in the test (checking specific field names via containsAll) is correct. The production change itself is safe to ship once the test assertions are fixed.
openmetadata-service/src/test/java/org/openmetadata/service/search/SearchUtilsTest.java — the two assertions that wrap withDefaultSearchSourceExcludes results in Set.of(...) need to be replaced with new HashSet<>(Arrays.asList(...)).
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Search Request] --> B{fetchSource == false?} B -- Yes --> C[fetchSource false\nno _source returned] B -- No --> D{includeSourceFields\nprovided?} D -- Yes --> E[fetchSource includeFields, excludeFields\nonly requested fields returned] D -- No --> F[fetchSource null,\nwithDefaultSearchSourceExcludes excludeFields\nall fields EXCEPT heavy defaults] F --> G[DEFAULT_SEARCH_SOURCE_EXCLUDES\nembedding, upstreamLineage,\nschemaDefinition, customMetrics,\n*_suggest, lifeCycle, fqnParts]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Search Request] --> B{fetchSource == false?} B -- Yes --> C[fetchSource false\nno _source returned] B -- No --> D{includeSourceFields\nprovided?} D -- Yes --> E[fetchSource includeFields, excludeFields\nonly requested fields returned] D -- No --> F[fetchSource null,\nwithDefaultSearchSourceExcludes excludeFields\nall fields EXCEPT heavy defaults] F --> G[DEFAULT_SEARCH_SOURCE_EXCLUDES\nembedding, upstreamLineage,\nschemaDefinition, customMetrics,\n*_suggest, lifeCycle, fqnParts]Comments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/search/SearchUtils.java, line 7 (link)DEFAULT_SEARCH_SOURCE_EXCLUDESandwithDefaultSearchSourceExcludesare shared utilities consumed by bothElasticSearchSearchManagerandOpenSearchSearchManager, but they are built fromElasticSearchClient.SOURCE_FIELDS_TO_EXCLUDE— an engine-specific constant. If OpenSearch ever needs a different exclusion set (e.g. a field renamed or split in one engine's mapping), this cross-engine import will silently apply the wrong list to OpenSearch. MovingSOURCE_FIELDS_TO_EXCLUDE(andFIELDS_TO_REMOVE) toSearchClientorSearchUtilsitself would remove the coupling.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (4): Last reviewed commit: "Merge branch 'main' into perf/lean-searc..." | Re-trigger Greptile