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

Add pending control to batcher #4042

Merged
merged 4 commits into from
Sep 9, 2015
Merged

Add pending control to batcher #4042

merged 4 commits into from
Sep 9, 2015

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Sep 8, 2015

With this change, the generic batcher used by many inputs can now be buffered. Testing shows that this improves performance of the Graphite input by 10-100%, with the biggest improvements at lower numbers of connections. Internal testing showed that this configuration -- a batcher per service, as opposed to per connection -- gave the best throughput and stable behaviour.

This patch also sets defaults for UDP input batching, which were not being set before.

@otoolep otoolep force-pushed the buffered_batcher branch 2 times, most recently from 658eba1 to 6035acd Compare September 8, 2015 22:40
@otoolep
Copy link
Contributor Author

otoolep commented Sep 8, 2015

#3741

@jwilder

cc @pkittenis

// NewPointBatcher returns a new PointBatcher.
func NewPointBatcher(sz int, d time.Duration) *PointBatcher {
// NewPointBatcher returns a new PointBatcher. sz is the batching size,
// bf is the maximum number of batches that may be pending. d is the time
Copy link
Contributor

Choose a reason for hiding this comment

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

s/bf/bp/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, I used to call it bf in the code, but forgot to update the comment -- thanks.

@jwilder
Copy link
Contributor

jwilder commented Sep 9, 2015

Small comment issue but 👍

With this change, the generic batcher used by many inputs can now be
buffered. Testing shows that this performance of the Graphite input by
10-100%, with the biggest improvements at lower numbers of connections.
otoolep added a commit that referenced this pull request Sep 9, 2015
@otoolep otoolep merged commit 02a54d0 into master Sep 9, 2015
@otoolep otoolep deleted the buffered_batcher branch September 9, 2015 02:48
@pkittenis
Copy link

Hello,

Many thanks for this change and your continual improvements - every new version generally keeps getting better 👍

Have been running this version in our testing environment and it looks much, much better write performance to graphite plugin wise.

We have not seen a client write failure since the upgrade and CPU usage of InfluxDB has dropped a bit on average as well.

There is only one connection in to the influxdb graphite plugin.

Client logs:

[2015-09-10 10:46:46] failed to write() to 127.0.0.1:2013: Connection reset by peer
[2015-09-10 10:46:58] server 127.0.0.1:2013: OK
[2015-09-10 10:50:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 10:50:36] server 127.0.0.1:2013: OK
[2015-09-10 10:50:38] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 10:50:38] server 127.0.0.1:2013: OK
[2015-09-10 11:12:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:12:36] server 127.0.0.1:2013: OK
[2015-09-10 11:13:38] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:13:38] server 127.0.0.1:2013: OK
[2015-09-10 11:17:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:17:36] server 127.0.0.1:2013: OK
[2015-09-10 11:17:38] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:17:38] server 127.0.0.1:2013: OK
[2015-09-10 11:33:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:33:36] server 127.0.0.1:2013: OK
[2015-09-10 11:34:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:34:36] server 127.0.0.1:2013: OK
[2015-09-10 11:36:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:36:36] server 127.0.0.1:2013: OK
[2015-09-10 11:39:35] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:39:36] server 127.0.0.1:2013: OK
[2015-09-10 11:40:36] failed to write() to 127.0.0.1:2013: uncomplete write
[2015-09-10 11:40:36] server 127.0.0.1:2013: OK
[2015-09-10 12:16:20] failed to write() to 127.0.0.1:2013: Connection reset by peer
[2015-09-10 12:17:28] server 127.0.0.1:2013: OK

12:16:20 was when the DB was upgraded - no write errors since then.
Version now running is
2015/09/10 12:16:23 InfluxDB starting, version 0.9.4-nightly-5e3fa1e, branch master, commit 5e3fa1e80fc460b338f08471e0ce11fd8856aade

@otoolep
Copy link
Contributor Author

otoolep commented Sep 10, 2015

Great, thanks for the feedkback @pkittenis

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.

3 participants