Skip to content

Comments

Fix an OOM error when creating to many chained synonym graph token filter.#140026

Merged
afoucret merged 3 commits intoelastic:patch/serverless-fixfrom
afoucret:fix-chained-synonyms-oom-reload
Dec 30, 2025
Merged

Fix an OOM error when creating to many chained synonym graph token filter.#140026
afoucret merged 3 commits intoelastic:patch/serverless-fixfrom
afoucret:fix-chained-synonyms-oom-reload

Conversation

@afoucret
Copy link
Contributor

@afoucret afoucret commented Dec 29, 2025

  1. b36152d: Add YAML REST tests that describe the current behavior when chaining synonym graph token filters (check no functional regression in the patch)
  2. 589c712: Add unit tests for chained analyzer creation at scale (100 synonyms set each containing 10_000 rules).
  3. 32d60e5 Fix that prevent to apply preceding synonym filter when building the synonym token filter

@afoucret afoucret changed the base branch from main to patch/serverless-fix December 29, 2025 16:13
@afoucret afoucret force-pushed the fix-chained-synonyms-oom-reload branch from 2a4b17f to b36152d Compare December 29, 2025 16:21
@afoucret afoucret force-pushed the fix-chained-synonyms-oom-reload branch from cd3ee68 to 32d60e5 Compare December 29, 2025 22:19
ts.reset();
assertTrue("Should produce at least one token", ts.incrementToken());
ts.close();
}
Copy link
Contributor

@markjhoy markjhoy Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to help the test GC along, you may want to place:

indexAnalyzers = null;

at the end of the test. (context: playing with the test, I had it loop through the entire test 20 times, and it OOM'd for me... adding this inside the loop, it passed). Not saying an OOM will happen during a CI test run with just the one time through in the test, but it can't hurt just in case.

Copy link
Contributor

@markjhoy markjhoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good, and the logic for the overridden method makes sense.

Left one comment in the unit tests, but overall, I think this will work.

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@afoucret afoucret marked this pull request as ready for review December 30, 2025 09:45
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Dec 30, 2025
@afoucret afoucret added >bug :Search Relevance/Search Catch all for Search Relevance and removed needs:triage Requires assignment of a team area label labels Dec 30, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Dec 30, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@afoucret afoucret merged commit e6bd70d into elastic:patch/serverless-fix Dec 30, 2025
35 checks passed
@afoucret
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
main

Questions ?

Please refer to the Backport tool documentation

afoucret added a commit to afoucret/elasticsearch that referenced this pull request Dec 31, 2025
…lter. (elastic#140026)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)
@mayya-sharipova mayya-sharipova mentioned this pull request Jan 7, 2026
10 tasks
szybia added a commit to szybia/elasticsearch that referenced this pull request Jan 7, 2026
* upstream/main:
  Add hook for blocking termination (elastic#133555)
  Delegate to ES93ScalarQuantizedVectorsFormat rather than copying behaviour (elastic#139834)
  Refactor compound block types (elastic#140219)
  Flush the rate buffer when the slice index changes (elastic#138856)
  Log linked project connection errors at debug during shutdown (elastic#140239)
  Periodic FIPS 140-3 buildkite pipelines (elastic#139909)
  ES|QL - Remove TERM function (elastic#139953)
  Fix name of started time field in shutdown status (elastic#139910)
  Drop `project_routing` from query params (elastic#140272)
  Fix flaky test: AllocationDecidersTests (elastic#140271)
  Add List Reindex API (elastic#140184)
  Expose _tier metadata attribute in ESQL (elastic#139894)
  Tweak TSDBRestEsqlIT#testTimeSeriesQuerying(...) (elastic#140210)
  Fix an OOM error when creating to many chained synonym graph token filter. (elastic#140026)
  Suppress Azure SDK error logs (elastic#139730)
  Rewritten integer sorts need to use SortedNumericSortField (elastic#139538) (elastic#139700)
  Adjust index versions for skippers for time series (elastic#139670)
  Fix host.name skippers index version range (elastic#139636)
  Remove BWC shim for a broken commit
  Fix index.mapping.use_doc_values_skipper defaults in serverless (elastic#139532)
Mikep86 pushed a commit to Mikep86/elasticsearch that referenced this pull request Feb 5, 2026
…lter. (elastic#140026)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)
@Mikep86
Copy link
Contributor

Mikep86 commented Feb 5, 2026

💚 All backports created successfully

Status Branch Result
9.3
9.2
8.19

Questions ?

Please refer to the Backport tool documentation

Mikep86 pushed a commit to Mikep86/elasticsearch that referenced this pull request Feb 5, 2026
…lter. (elastic#140026)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)
Mikep86 pushed a commit to Mikep86/elasticsearch that referenced this pull request Feb 5, 2026
…lter. (elastic#140026)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)
elasticsearchmachine pushed a commit that referenced this pull request Feb 5, 2026
…lter. (#140026) (#141966)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)

Co-authored-by: Aurélien FOUCRET <aurelien.foucret@gmail.com>
elasticsearchmachine pushed a commit that referenced this pull request Feb 5, 2026
…lter. (#140026) (#141965)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)

Co-authored-by: Aurélien FOUCRET <aurelien.foucret@gmail.com>
elasticsearchmachine pushed a commit that referenced this pull request Feb 5, 2026
…lter. (#140026) (#141964)

* Create YAML REST test for chained synonym filters.

* Adding some unit tests for chained synonyms graph filters creation.

* Ensure previous synonyms set from the chain aren't applied again.

(cherry picked from commit e6bd70d)

Co-authored-by: Aurélien FOUCRET <aurelien.foucret@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants