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

[processors.execd] Make the buffer size configurable #8142

Closed
M0rdecay opened this issue Sep 17, 2020 · 3 comments · Fixed by #8145
Closed

[processors.execd] Make the buffer size configurable #8142

M0rdecay opened this issue Sep 17, 2020 · 3 comments · Fixed by #8145
Labels
feature request Requests for new plugin and for new features to existing plugins

Comments

@M0rdecay
Copy link
Contributor

Feature Request

Opening a feature request kicks off a discussion.

Proposal:

Add the size of the buffer used in the cmdReadOut function to the processor parameters

Current behavior:

The buffer size is currently not configurable. For this reason, an attempt to get a large metric from an external processor fails with an error:

E! [processors.execd] Error reading stdout: bufio.Scanner: token too long

Desired behavior:

The buffer size can be adjusted so that the processor can handle large metrics.

Use case:

To work with XML, I have made a wrapper for the parser and work with it as with an external processor. The processor is written following the example from the README.
When processing large documents (~7 thousand lines), receiving data back from the processor fails. The reason is as follows - as a result of parsing, the processor can return a metric of 70-80 KiB.

Also, if this error occurs, the processor stops working and does not process the following metrics, which could fit in the default buffer.

@M0rdecay M0rdecay added the feature request Requests for new plugin and for new features to existing plugins label Sep 17, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Sep 17, 2020

Wow. That's a large metric. It looks like line protocol limits a single field to 64 KB anyway. You might run into other problems even if we expand it, but I'm not against expanding the buffer size available for the metric. the change should be pretty straightforward, and doesn't necessarily have to be configurable.

something like:

	scanner := bufio.NewScanner(out)
	scanBuf := make([]byte, 4_000) // default size
	scanner.Buffer(scanBuf, 300_000) // max size

@M0rdecay
Copy link
Contributor Author

M0rdecay commented Sep 17, 2020

I will try to do it, test it, and, if it works, prepare the PR.
What do you think?

@M0rdecay
Copy link
Contributor Author

Yeah, this works as expected.

This means that the error was not related to a protocol limitation - otherwise, I think, I would have received an error in the external processor even at the serialization stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants