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

Increasing the metric buffer #8145

Merged
merged 4 commits into from
Sep 18, 2020
Merged

Conversation

M0rdecay
Copy link
Contributor

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Closes #8142
Also related with #8121

Comment on lines 27 to 29
## Buffer size in bytes for storing metrics received from the process
## The minimum allowed value is 4096
buffer_size = 65536
Copy link
Contributor

Choose a reason for hiding this comment

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

this is actually the buffer size used for storing a single metric, not multiple metrics.

Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

I'm still not sure I like that it's configurable. we should just do the right thing. Too many knobs and dials to configure is just confusing.

This could literally be a two line change that works for 100% of people. No configuration options.

@M0rdecay
Copy link
Contributor Author

M0rdecay commented Sep 17, 2020

I'm still not sure I like that it's configurable. we should just do the right thing. Too many knobs and dials to configure is just confusing.

This could literally be a two line change that works for 100% of people. No configuration options.

What about these values?
Hope this will be enough (unfortunately i don't really know how large metrics i have yet to see)...

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

@M0rdecay M0rdecay changed the title Configurable execd processor buffer size Increasing the metric buffer Sep 18, 2020
Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

Looks good.

@ssoroka ssoroka merged commit 39f4c36 into influxdata:master Sep 18, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Sep 18, 2020

You may run into size issues in other places, but at least this won't be one of them.

@M0rdecay
Copy link
Contributor Author

You may run into size issues in other places, but at least this won't be one of them.

Yes, I understand, but for now we prefer to use an external processor for working with XML - until a built-in parser appears in the telegraf.

I tested processing larger documents (10-12 thousand lines), everything is fine with the new buffer.
Also, there are no errors when processing such metrics when using an assembly with the parser inside the agent.

Many thanks for the help!

idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020

## Buffer size in bytes for storing metrics received from the process
## The minimum allowed value is 4096
# buffer_size = 65536
Copy link

Choose a reason for hiding this comment

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

its the same when execd used as inputs plugin?

arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processors.execd] Make the buffer size configurable
3 participants