fix(search): filter _source in column tag/glossary search to prevent serving-time OOM - #29502
fix(search): filter _source in column tag/glossary search to prevent serving-time OOM#29502mohityadav766 wants to merge 2 commits into
Conversation
…serving-time OOM The column tag/glossary search (OpenSearch/ElasticSearchColumnAggregator .fetchColumnsWithTagsFromSource) fetched up to 10,000 data-asset documents with their full _source to scan column tags in Java (flat ES mapping can't filter "column X has tag Y" at query time). Full _source pulls table-level descriptions, sample data, profiles and every column sub-field, so a single fetch can be tens of MB. Buffering + Jackson-parsing that produces multi-MB (G1 humongous) allocations; under concurrent load this drives Full-GC churn and stop-the-world stalls that can leave liveness probes unanswered and get the pod SIGKILLed. Restrict the fetch to the fields the extractor actually reads via a shared TAG_SOURCE_FETCH_INCLUDES list (no duplication across OS/ES). Measured on a real ~20k-entity dataset the fetch drops from 25.1 MB to 4.1 MB (6.2x), and far more on data with large descriptions, while keeping behavior unchanged. Adds a regression test asserting the include list covers every extractor-read field and never re-broadens to full _source. 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}
| @Test | ||
| void tagSourceFetchIncludes_coverFieldsTheExtractorReads() { | ||
| // The tag/glossary column search reads _source in Java to locate tagged columns. The fix | ||
| // restricts _source to TAG_SOURCE_FETCH_INCLUDES to avoid fetching multi-MB documents (full | ||
| // _source caused G1 humongous allocations and Full-GC churn under concurrent load). This list | ||
| // must cover every field the extractor and parseColumn read, or filtering would silently drop | ||
| // data the feature needs. | ||
| List<String> includes = ColumnAggregator.TAG_SOURCE_FETCH_INCLUDES; | ||
| assertTrue( | ||
| includes.containsAll( | ||
| List.of( | ||
| "fullyQualifiedName", | ||
| "entityType", | ||
| "displayName", | ||
| "service.name", |
There was a problem hiding this comment.
💡 Quality: Coverage test duplicates the constant rather than verifying extractor reads
tagSourceFetchIncludes_coverFieldsTheExtractorReads asserts that TAG_SOURCE_FETCH_INCLUDES contains a hardcoded list of field names that is an exact copy of the constant's own definition. This makes the assertion effectively tautological: it guards against the constant being shrunk, but it does NOT actually verify the list covers "every field the extractor and parseColumn read" as the comment claims. If a future change adds a new _source read in extractMatchingColumnsFromHit/parseColumn (e.g. a new column sub-field) and forgets to add it to the include list, this test will still pass and the field will be silently dropped from results — exactly the failure mode the test purports to prevent.
I confirmed via the source that the current include list does cover all fields read by both ES and OS extractors (fullyQualifiedName, entityType, displayName, service.name, database.name, databaseSchema.name, columns.name/displayName/description/dataType/fullyQualifiedName/tags/children), so the fix itself is correct today. The concern is purely the durability of the regression guard.
Consider strengthening the test so it derives the required set from actual reads (or at minimum add a comment that the list must be manually kept in sync, and keep a single source of truth). A lightweight improvement is to assert the include list is non-empty and that each entry is a known extractor field, but the strongest form would scan the extractor for .get("...") literals. At minimum, the test name/comment overstates the guarantee.
Was this helpful? React with 👍 / 👎
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| List<String> TAG_SOURCE_FETCH_INCLUDES = | ||
| List.of( | ||
| "fullyQualifiedName", | ||
| "entityType", | ||
| "displayName", | ||
| "service.name", | ||
| "database.name", | ||
| "databaseSchema.name", | ||
| "columns.name", | ||
| "columns.displayName", | ||
| "columns.description", | ||
| "columns.dataType", | ||
| "columns.fullyQualifiedName", | ||
| "columns.tags", | ||
| "columns.children"); |
There was a problem hiding this comment.
columns entity types in ES aggregator
TAG_SOURCE_FETCH_INCLUDES only covers the columns.* field path, but ElasticSearchColumnAggregator.INDEX_CONFIGS maps three additional entity types to different root paths: topic → messageSchema.schemaFields, searchIndex → fields, and container → dataModel.columns. With the source filter applied, getNestedJsonNode(sourceNode, columnFieldPath) returns null for those paths and silently produces zero tag-matched columns for those entity types. This is a regression versus the previous full-_source fetch, which returned the correct data for all entity types. The missing paths should be added: messageSchema.schemaFields, fields, and dataModel.columns (or their individual sub-fields).
Jest test CoverageUI tests summary
|
|
🔴 Playwright Results — 9 failure(s), 55 flaky✅ 4419 passed · ❌ 9 failed · 🟡 55 flaky · ⏭️ 38 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsRestricts _source fields in the OpenSearch column aggregator to reduce memory pressure and prevent OOM-related pod restarts. Consider refactoring the regression test to dynamically verify field coverage instead of duplicating the constant definition. 💡 Quality: Coverage test duplicates the constant rather than verifying extractor reads📄 openmetadata-service/src/test/java/org/openmetadata/service/search/ColumnAggregatorTest.java:114-128 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnAggregator.java:48-62
I confirmed via the source that the current include list does cover all fields read by both ES and OS extractors (fullyQualifiedName, entityType, displayName, service.name, database.name, databaseSchema.name, columns.name/displayName/description/dataType/fullyQualifiedName/tags/children), so the fix itself is correct today. The concern is purely the durability of the regression guard. Consider strengthening the test so it derives the required set from actual reads (or at minimum add a comment that the list must be manually kept in sync, and keep a single source of truth). A lightweight improvement is to assert the include list is non-empty and that each entry is a known extractor field, but the strongest form would scan the extractor for 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
1 participant
Fixes #29501
Problem
The column tag/glossary search (
OpenSearchColumnAggregator/ElasticSearchColumnAggregator→fetchColumnsWithTagsFromSource) fetches up to 10,000 data-asset documents with their full_sourceand scans column tags in Java (flat ES/OS mapping can't filter "column X has tag Y" at query time). Full_sourcecarries table descriptions, sample data, profiles and every column sub-field, so a single fetch can be tens of MB. Buffering + Jackson-parsing it produces multi-MB G1 humongous allocations; under concurrent load this causes Full-GC churn and stop-the-world stalls that can leave the liveness probe unanswered → pod SIGKILL (exit 137).Fix
Restrict the fetch to exactly the fields the extractor reads, via a shared
ColumnAggregator.TAG_SOURCE_FETCH_INCLUDES(_sourceinclude-filter; no OS/ES duplication). Behavior is unchanged — the extractor reads the same fields, just from a lean document.Evidence
size=10000fetch over a real ~20k-entity dataset: 25.1 MB → 4.1 MB (6.2× smaller); far larger reductions on data with big descriptions._sourcerequests pin the heap, ~900–1140 G1 humongous regions, repeated Full GCs.Type of change
Testing
ColumnAggregatorTestgreen (9/9), incl. a new regression test asserting the include list covers every extractor-read field and never re-broadens to full_source.openmetadata-servicecompiles clean (mvn -pl openmetadata-service -am clean install -DskipTests→ BUILD SUCCESS).Backport
Please backport to the active release branch(es).
🤖 Generated with Claude Code
Greptile Summary
This PR applies an
_sourceinclude-filter (TAG_SOURCE_FETCH_INCLUDES) to the tag/glossary column-search fetch in both the ElasticSearch and OpenSearch column aggregators, reducing response size from ~25 MB to ~4 MB per 10k-entity fetch and preventing G1 humongous allocations that caused Full-GC churn and OOM kills under concurrent load.TAG_SOURCE_FETCH_INCLUDES(a shared constant on theColumnAggregatorinterface) listing exactly the 13 fields that the tag-extractor reads, and wires it as_source.filter.includesin bothElasticSearchColumnAggregator.fetchColumnsWithTagsFromSourceandOpenSearchColumnAggregator.fetchColumnsWithTagsFromSource.tagSourceFetchIncludes_coverFieldsTheExtractorReadsto assert the constant covers all extractor-read fields and guards against future widening back to full_source.Confidence Score: 4/5
Safe to merge for the common
tableanddashboardDataModelentity types, but the ES aggregator's tag/glossary search will silently return zero results fortopic,searchIndex, andcontainerentities — a regression that was not present before the source filter was applied.The fix achieves its goal and is safe for the default entity type (
table). However,ElasticSearchColumnAggregator.INDEX_CONFIGSmapstopic,searchIndex, andcontainerto non-columnsfield paths (messageSchema.schemaFields,fields,dataModel.columns), none of which appear inTAG_SOURCE_FETCH_INCLUDES. After the filter is applied,getNestedJsonNodereturns null for those paths and the extractor silently produces an empty result — a behavioral regression for those entity types in the ES path.openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnAggregator.java — the TAG_SOURCE_FETCH_INCLUDES list needs the additional field paths for topic, searchIndex, and container entities to restore correct behavior in the ES aggregator.
Important Files Changed
columns.*paths and is missingmessageSchema.schemaFields.*,fields.*, anddataModel.columns.*needed by ES INDEX_CONFIGS for topic, searchIndex, and container entity types (already flagged in prior review thread).columnspaths not included in the filter (covered in prior thread).columnspath, so the include list is consistent with what is actually read.*/description/sampleData; guards against accidental widening of the filter.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant ColumnAggregator participant ES_OS as ES/OS Search Engine Client->>ColumnAggregator: aggregateColumns(request) [tags/glossaryTerms set] ColumnAggregator->>ColumnAggregator: buildTagFilterQuery() ColumnAggregator->>ES_OS: "SearchRequest (size=10000, _source.filter.includes=TAG_SOURCE_FETCH_INCLUDES)" Note over ColumnAggregator,ES_OS: Before fix: full _source (~25 MB per 10k docs)<br/>After fix: lean _source (~4 MB per 10k docs) ES_OS-->>ColumnAggregator: Hits with filtered _source (13 fields only) loop For each hit ColumnAggregator->>ColumnAggregator: "extractMatchingColumnsFromHit()<br/>reads: fullyQualifiedName, entityType, displayName,<br/>service.name, database.name, databaseSchema.name,<br/>columns.*, columns.tags, columns.children" end ColumnAggregator-->>Client: ColumnGridResponse (paginated tagged columns)%%{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"}}}%% sequenceDiagram participant Client participant ColumnAggregator participant ES_OS as ES/OS Search Engine Client->>ColumnAggregator: aggregateColumns(request) [tags/glossaryTerms set] ColumnAggregator->>ColumnAggregator: buildTagFilterQuery() ColumnAggregator->>ES_OS: "SearchRequest (size=10000, _source.filter.includes=TAG_SOURCE_FETCH_INCLUDES)" Note over ColumnAggregator,ES_OS: Before fix: full _source (~25 MB per 10k docs)<br/>After fix: lean _source (~4 MB per 10k docs) ES_OS-->>ColumnAggregator: Hits with filtered _source (13 fields only) loop For each hit ColumnAggregator->>ColumnAggregator: "extractMatchingColumnsFromHit()<br/>reads: fullyQualifiedName, entityType, displayName,<br/>service.name, database.name, databaseSchema.name,<br/>columns.*, columns.tags, columns.children" end ColumnAggregator-->>Client: ColumnGridResponse (paginated tagged columns)Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/column-aggr..." | Re-trigger Greptile