performance improvements: squash messages, and NIO #7
Conversation
| try { | ||
| String message = queue.poll(1, TimeUnit.SECONDS); | ||
| if(null != message) { | ||
| byte[] data = message.getBytes(); |
There was a problem hiding this comment.
You should always specify the encoding. I think that it would be best to assume that this encoding is UTF-8
There was a problem hiding this comment.
I can do that. Although it's strictly not part of this pull request (the original code also uses getBytes()), and i'd rather keep it as a separate commit and pull request (for the sake of bisecting).
Verdict?
There was a problem hiding this comment.
I'm fine with it not being part of this.
|
DONE: i've got a test coming… |
| executor.shutdown(); | ||
| executor.awaitTermination(30, TimeUnit.SECONDS); | ||
| executor.awaitTermination(1, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Is this change actually needed. I think I would prefer that it stays at 30 seconds.
There was a problem hiding this comment.
Not really (the change isn't needed).
But the test seems to wait the full period. (maybe because the threads are set to daemon?)
In one second the test (on my machine) pushes through 10k statsd messages.
Given that, and the point that udp packets are not crucial, are we worried about consistency of the messages here?
There was a problem hiding this comment.
i'd forgotten to call executor.shutdown() in the test.
fixing that fixed this. it's returned to 30 seconds now.
|
I am not sure that I understand enough about the failure cases of Disruptor to add it as a dependency. I like the NIO and squashing though. Can we maybe make the disruptor stuff a different branch, and change this PULL into only squashing and NIO? |
|
sure thing. i've taken the lmax-disruptor implementation out of this branch. and will put it into a new pull request. |
- fit as many messages (that are in the queue) into the MTU as possible
- convert to NIO
In addition to any NIO improvements,
this will offer a performance improvement when the incoming message rate per the time it takes to call blockingSend() goes above one.
Add NonBlockingStatsDClientPerfTest to ensure no messages are lost in a concurrent environment.
|
okay this LGTM I will work on getting it pulled in. |
| blockingSend(); | ||
| } | ||
| if(sendBuffer.position() > 0) { | ||
| sendBuffer.put( (byte) '\n'); |
There was a problem hiding this comment.
Just for future reference https://github.com/DataDog/dd-agent/blob/de14dac5f1af5a444cec46cff047ca70fa6e294f/aggregator.py#L398 shows that dogstatsd supports '\n' separated metrics as well.
performance improvements: squash messages, and NIO
In addition to any NIO improvements,
this will offer a performance improvement when the incoming message rate per the time it takes to call blockingSend() goes above one.