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 #16482 to 7.17: Bugfix for BufferedTokenizer to completely consume lines in case of lines bigger then sizeLimit #16577

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Oct 17, 2024

Non clean backport of #16482

The differences are:

  • usage of data.convertToString().split(context, delimiter, MINUS_ONE); instead of data.convertToString().split(delimiter, -1);
  • avoid to extend BuffererdTokenir test cases from org.logstash.RubyTestBase which was introduced in Refactor: drop redundant (jruby-complete.jar) dependency #13159
  • JDK 8 compatibilities:
    • Arrays.asList vs List.of
    • assertThrows method from JUnit5 not available in JUnit4 so reimplemented in the test

Fixes the behaviour of the tokenizer to be able to work properly when buffer full conditions are met.

Updates BufferedTokenizerExt so that can accumulate token fragments coming from different data segments. When a "buffer full" condition is matched, it record this state in a local field so that on next data segment it can consume all the token fragments till the next token delimiter. Updated the accumulation variable from RubyArray containing strings to a StringBuilder which contains the head token, plus the remaining token fragments are stored in the input array. Furthermore it translates the buftok_spec tests into JUnit tests.

Release notes

What does this PR do?

Why is it important/What is the impact to the user?

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 (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

…ines bigger then sizeLimit (elastic#16482)

Fixes the behaviour of the tokenizer to be able to work properly when buffer full conditions are met.

Updates BufferedTokenizerExt so that can accumulate token fragments coming from different data segments. When a "buffer full" condition is matched, it record this state in a local field so that on next data segment it can consume all the token fragments till the next token delimiter.
Updated the accumulation variable from RubyArray containing strings to a StringBuilder which contains the head token, plus the remaining token fragments are stored in the input array.
Furthermore it translates the `buftok_spec` tests into JUnit tests.
- usage of `data.convertToString().split(context, delimiter, MINUS_ONE);` instead of `data.convertToString().split(delimiter, -1);`
- avoid to extend BuffererdTokenir test cases from `org.logstash.RubyTestBase` which was introduced in elastic#13159
- JDK 8 compatibilities:
  - `Arrays.asList` vs `List.of`
  - `assertThrows` method from JUnit5 not available in JUnit4 so reimplemented in the test
@andsel andsel changed the title Backport #16482 Bugfix for BufferedTokenizer to completely consume lines in case of lines bigger then sizeLimit Backport #16482 to 7.17: Bugfix for BufferedTokenizer to completely consume lines in case of lines bigger then sizeLimit Oct 17, 2024
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @andsel

@andsel andsel requested a review from jsvd October 18, 2024 15:08
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

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

LGTM

@edmocosta edmocosta merged commit b4ca550 into elastic:7.17 Oct 22, 2024
3 checks passed
donoghuc added a commit to donoghuc/logstash that referenced this pull request Nov 21, 2024
…to completely consume lines in case of lines bigger then sizeLimit (elastic#16577)"

This reverts commit b4ca550.
donoghuc added a commit that referenced this pull request Nov 21, 2024
…letely consume lines in case of lines bigger then sizeLimit (#16577)" (#16713)

This reverts commit b4ca550.
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.

4 participants