Skip to content

Backport PR #16323 to 8.15: make pipeline resilient to stack overflow during compilation#16336

Merged
jsvd merged 1 commit into8.15from
backport_16323_8.15
Jul 18, 2024
Merged

Backport PR #16323 to 8.15: make pipeline resilient to stack overflow during compilation#16336
jsvd merged 1 commit into8.15from
backport_16323_8.15

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Backport PR #16323 to 8.15 branch, original message:


A couple of thoughts on the way this is implemented:

  1. There should be a first barrier to handle pipelines that are too large based on the PipelineIR compilation. The barrier would use the detection of Xss to determine how big a pipeline could be. This however doesn't reduce the need to still handle a StackOverflow if it happens.
  2. The catching of StackOverflowError could also be done on the WorkerLoop. However I'd suggest that this is unrelated to the Worker initialization itself, it just so happens that compiledPipeline.buildExecution is computed inside the WorkerLoop class for performance reasons. So I'd prefer logging to not come from the existing catch, but from a dedicated catch clause.

Solves #16320

…16323)

This commit improves error handling when pipelines that are too big hit the Xss limit and throw a StackOverflowError. Currently the exception is printed outside of the logger, and doesn’t even show if log.format is json, leaving the user to wonder what happened.

A couple of thoughts on the way this is implemented:

* There should be a first barrier to handle pipelines that are too large based on the PipelineIR compilation. The barrier would use the detection of Xss to determine how big a pipeline could be. This however doesn't reduce the need to still handle a StackOverflow if it happens.
* The catching of StackOverflowError could also be done on the WorkerLoop. However I'd suggest that this is unrelated to the Worker initialization itself, it just so happens that compiledPipeline.buildExecution is computed inside the WorkerLoop class for performance reasons. So I'd prefer logging to not come from the existing catch, but from a dedicated catch clause.

Solves #16320

(cherry picked from commit 8f2dae6)
@jsvd jsvd merged commit 4f34601 into 8.15 Jul 18, 2024
@jsvd jsvd deleted the backport_16323_8.15 branch July 18, 2024 09:09
@elastic-sonarqube
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants