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

x-pack/filebeat/input/cel: fix eval state return on error #33996

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Dec 8, 2022

What does this PR do?

Previously, when an error in evaluation or result deserialisation occurred, the entire input activation container was returned instead of a valid state. The result of this was that the state would nest on each iteration of the eval event loop. In cases where errors can happen frequently, this can result in unbounded memory use.

Also improve the robustness of the want_more flag; the old code would continue with the loop if state["want_more"] was unset since any(nil) does not equal false.

Why is it important?

Unbounded memory use.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works — tested manually: see [Filebeat] cel input - deeply nested 'state' after HTTP error #33992 for approach
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 added bug Filebeat Filebeat 8.7-candidate backport-v8.6.0 Automated backport with mergify labels Dec 8, 2022
@efd6 efd6 self-assigned this Dec 8, 2022
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 8, 2022
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 8, 2022
@efd6 efd6 force-pushed the 33992-cel branch 2 times, most recently from f84e705 to 01a1b54 Compare December 8, 2022 07:22
@efd6 efd6 marked this pull request as ready for review December 8, 2022 07:23
@efd6 efd6 requested a review from a team as a code owner December 8, 2022 07:23
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 8, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-12-08T09:06:17.870+0000

  • Duration: 75 min 54 sec

Test stats 🧪

Test Results
Failed 0
Passed 2514
Skipped 167
Total 2681

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Previously, when an error in evaluation or result deserialisation
occurred, the entire input activation container was returned instead of
a valid state. The result of this was that the state would nest on each
iteration of the eval event loop. In cases where errors can happen
frequently, this can result in unbounded memory use.

Also improve the robustness of the want_more flag; the old code would
continue with the loop if state["want_more"] was unset since any(nil)
does not equal false.
@sonarcloud
Copy link

sonarcloud bot commented Dec 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. I re-tested and the state looks good.

@andrewkroh andrewkroh merged commit f08eed8 into elastic:main Dec 8, 2022
mergify bot pushed a commit that referenced this pull request Dec 8, 2022
Previously, when an error in evaluation or result deserialisation
occurred, the entire input activation container was returned instead of
a valid state. The result of this was that the state would nest on each
iteration of the eval event loop. In cases where errors can happen
frequently, this can result in unbounded memory use.

Also improve the robustness of the want_more flag; the old code would
continue with the loop if state["want_more"] was unset since any(nil)
does not equal false.

Relates: #33992
(cherry picked from commit f08eed8)
efd6 added a commit that referenced this pull request Dec 8, 2022
…33997)

Previously, when an error in evaluation or result deserialisation
occurred, the entire input activation container was returned instead of
a valid state. The result of this was that the state would nest on each
iteration of the eval event loop. In cases where errors can happen
frequently, this can result in unbounded memory use.

Also improve the robustness of the want_more flag; the old code would
continue with the loop if state["want_more"] was unset since any(nil)
does not equal false.

Relates: #33992
(cherry picked from commit f08eed8)

Co-authored-by: Dan Kortschak <[email protected]>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
Previously, when an error in evaluation or result deserialisation
occurred, the entire input activation container was returned instead of
a valid state. The result of this was that the state would nest on each
iteration of the eval event loop. In cases where errors can happen
frequently, this can result in unbounded memory use.

Also improve the robustness of the want_more flag; the old code would
continue with the loop if state["want_more"] was unset since any(nil)
does not equal false.

Relates: #33992
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.7-candidate backport-v8.6.0 Automated backport with mergify bug Filebeat Filebeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat] cel input - deeply nested 'state' after HTTP error
3 participants