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

[BUG] "Split lines failed: bufio.Scanner: token too long" for phase with a lot of output #2612

Closed
muffl0n opened this issue Jan 17, 2024 · 2 comments · Fixed by #2641
Closed
Labels
Projects

Comments

@muffl0n
Copy link
Contributor

muffl0n commented Jan 17, 2024

Describe the bug
One of our phases increased its output by a huge amount of lines from one day to another.
From 450 lines (40Kb) to 2530 lines (169Kb). This caused Kanister to fail the actionset with the error

Split lines failed: bufio.Scanner: token too long

https://github.com/kanisterio/kanister/blame/master/pkg/output/stream.go#L50

To Reproduce
Steps to reproduce the behavior:

  1. Create blueprint
  2. Add phase that produces a lot of output: more than 64Kb
  3. Run actionset
  4. See error

Expected behavior
Output is parsed.

Screenshots
n/a

Environment
Kubernetes Version/Provider: v1.27.7-gke.1121001
Storage Provider: n/a
Cluster Size (#nodes): n/a
Data Size: n/a
Kanister: 0.99.0

Additional context
The default buffer of bufio is 64Kb: https://pkg.go.dev/bufio#pkg-constants
That explains, why the error occurs when crossing this boundary.

const (
	// MaxScanTokenSize is the maximum size used to buffer a token
	// unless the user provides an explicit buffer with Scanner.Buffer.
	// The actual maximum token size may be smaller as the buffer
	// may need to include, for instance, a newline.
	MaxScanTokenSize = 64 * 1024
)

I'm not quite sure how to handle this. Just increasing the buffer blindly to some higher value does not seem like a good solution.

Thread in Slack: https://kanisterio.slack.com/archives/C85C8V22J/p1705490134135299

@muffl0n muffl0n added the bug label Jan 17, 2024
Copy link
Contributor

Thanks for opening this issue 👍. The team will review it shortly.

If this is a bug report, make sure to include clear instructions how on to reproduce the problem with minimal reproducible examples, where possible. If this is a security report, please review our security policy as outlined in SECURITY.md.

If you haven't already, please take a moment to review our project's Code of Conduct document.

@infraq infraq added this to To Be Triaged in Kanister Jan 17, 2024
@hairyhum
Copy link
Contributor

@muffl0n thanks for the contribution!

I think it should be addressable by replacing bufio.Scanner. with bufio.Readerand usingReadString`, so should be an easy fix.

@hairyhum hairyhum removed the triage label Jan 31, 2024
hairyhum added a commit that referenced this issue Jan 31, 2024
bufio.Scanner has buffer limit and can fail the function if output is too big
fixes #2612
hairyhum added a commit that referenced this issue Jan 31, 2024
bufio.Scanner has buffer limit and can fail the function if output is too big
fixes #2612
@mergify mergify bot closed this as completed in #2641 Feb 15, 2024
Kanister automation moved this from To Be Triaged to Done Feb 15, 2024
mergify bot added a commit that referenced this issue Feb 15, 2024
* fix: switch to bufio.Reader for KubeTask output parsing

bufio.Scanner has buffer limit and can fail the function if output is too big
fixes #2612

* WIP: scan kube task output in chunks

* Bugfix - empty line dumped to log redundantly

* Add unit tests draft

* fix: set read buffer size to 64kb to keep existing behaviour with shorter lines

Fix some out of bounds errors
Don't run the tests twice

* test: support filters in `make test` command

* test: test and lint issues

* Couple additional tests

* Remove commented code

* code style fixes

---------

Co-authored-by: Eugen Sumin <eugen.sumin@kasten.io>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging a pull request may close this issue.

2 participants