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

Backport PR #16760 to 8.16: ensure inputSize state value is reset during buftok.flush #16771

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 9, 2024

Backport PR #16760 to 8.16 branch, original message:


The BufferedTokenizer.flush must reset the inputSize value back to zero on flush, otherwise it will grow until a buffer full exception is thrown if the buffer has a size limit.

We can see the test fails if the inputSize = 0 is not present:

❯ bin/rspec logstash-core/spec/logstash/util/buftok_spec.rb -fd -e flush
[...]
FileWatch::BufferedTokenizer
  flush
    emits the contents of the buffer
    resets the state of the buffer
    with decode_size_limit_bytes
      resets the state of the buffer (FAILED - 1)
      emits the contents of the buffer

Failures:

  1) FileWatch::BufferedTokenizer flush with decode_size_limit_bytes resets the state of the buffer
     Failure/Error: expect(subject).to be_empty
       expected `#<FileWatch::BufferedTokenizer:0x35977ba7>.empty?` to be truthy, got false
     # ./logstash-core/spec/logstash/util/buftok_spec.rb:65:in `block in <main>'
     # ./spec/spec_helper.rb:84:in `block in <main>'

But with it tests pass:

❯ bin/rspec logstash-core/spec/logstash/util/buftok_spec.rb -fd -e flush

FileWatch::BufferedTokenizer
  flush
    resets the state of the buffer
    emits the contents of the buffer
    with decode_size_limit_bytes
      emits the contents of the buffer
      resets the state of the buffer

Finished in 0.0246 seconds (files took 1.34 seconds to load)
4 examples, 0 failures

@donoghuc
Copy link
Member

donoghuc commented Dec 9, 2024

CI does not seem to be triggering for this. This was raised over 30mins ago and there are no linked buildkite etc jobs. @mashhur do you know how to troubleshoot that?

Copy link
Member

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

Backport looks clean. Manual trigger of PR tests is green https://buildkite.com/elastic/logstash-pull-request-pipeline/builds/1937 Still debugging why the workflow was not triggered by PR open event.

@yaauie yaauie merged commit 1f93dc5 into 8.16 Dec 9, 2024
1 check passed
@yaauie yaauie deleted the backport_16760_8.16 branch December 9, 2024 19:23
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.

3 participants