HttpPostEmitter with timeout #68
Conversation
| FailedBuffer failedBuffer = failedBuffers.peekFirst(); | ||
| if (failedBuffer != null) { | ||
| if (sendWithRetries(failedBuffer.buffer, failedBuffer.length, failedBuffer.eventCount)) { | ||
| if (sendWithRetries(failedBuffer.buffer, failedBuffer.length, failedBuffer.eventCount, false)) { |
There was a problem hiding this comment.
I think we should enable timeout for at least the tryEmitOneFailedBuffer because if the emitting thread gets blocked on this, we may still have the situation of growing buffersToEmit.
There was a problem hiding this comment.
Each individual retry is still timing out, so it's limited to approximately 3 timouts (because we do 3 retires), so buffers to emit could get ~ 3 more batches at this time
| } | ||
| } | ||
|
|
||
| private long sendRequestTimeoutMillis() |
There was a problem hiding this comment.
How about we have these literals configuration, for example
com.metamx.emitter.http.httpTimeoutAllowanceFactor.level1 = 1.5
com.metamx.emitter.http.httpTimeoutAllowanceFactor.level2 = 0.9
com.metamx.emitter.http.httpTimeoutAllowanceFactor.level3 = 0.5
com.metamx.emitter.http.emitQueueThreshold.level1 = 5
com.metamx.emitter.http.emitQueueThreshold.level2 = 10
Or at least make them into constant values (e.g. private static final int EMIT_QUEUE_THRESHOLD1 = 5;, etc)
There was a problem hiding this comment.
will add constants, too much configs which nobody would know how to set right
|
LGTM 👍 |
To fix #65