Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap "Pattern too complex" exception into an IllegalArgumentException #109173

Merged
merged 3 commits into from
May 31, 2024

Conversation

afoucret
Copy link
Contributor

@afoucret afoucret commented May 29, 2024

The behavior of java.util.regex.Pattern.:compile has been updated in a way that make it possible too handle OutOfMemoryError errors cause by too complex pattern more gracefully and make them recoverable.

@afoucret afoucret added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team Team:SearchOrg Meta label for the Search Org (Enterprise Search) v8.15.0 labels May 29, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @afoucret, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine removed the Team:SearchOrg Meta label for the Search Org (Enterprise Search) label May 29, 2024
@afoucret afoucret requested a review from a team May 29, 2024 15:47
settings:
index:
analysis:
analyzer:
Copy link
Contributor

@saikatsarkar056 saikatsarkar056 May 29, 2024

Choose a reason for hiding this comment

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

What is the rectangle shaped char at the end of each line? Can we remove them?

Screenshot 2024-05-29 at 9 57 38 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

GH does not render it well - but this is an actual case where we know Pattern::compile throws an OutOfMemoryError so I think it is fine to keep it. Might be worth adding a comment here for folks that might read this code in the future? @afoucret

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this test to a more appropriate place and added a comment to it

Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me with one question about release notes.

docs/changelog/109173.yaml Show resolved Hide resolved
@afoucret afoucret added auto-backport Automatically create backport pull requests when merged v8.14.0 labels May 30, 2024
@afoucret afoucret merged commit c93862f into elastic:main May 31, 2024
15 checks passed
afoucret added a commit to afoucret/elasticsearch that referenced this pull request May 31, 2024
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants