Cleanup internal threading #144
Conversation
…structor call will spawn several threads. Instead we: * copy the current builder (to avoid side-effects) * resolve implicit elements * use the resolved builder to create the NonBlockingStatsDClient
(since each StatsD task is a busy loop that takes up exactly one thread)
| protected final CountDownLatch endSignal; | ||
|
|
||
| protected final int workers; | ||
| protected final ThreadFactory threadFactory; | ||
| protected final Thread[] workers; |
There was a problem hiding this comment.
This makes sense to me given the way the workers are used.
| @@ -49,7 +48,15 @@ | |||
| .onMalformedInput(CodingErrorAction.REPLACE) | |||
| .onUnmappableCharacter(CodingErrorAction.REPLACE); | |||
|
|
|||
| public abstract void run(); | |||
| public final void run() { | |||
There was a problem hiding this comment.
Totally unrelated to this PR, but maybe we should add an exclusion for this type if we haven't done so already, just in case.
| } | ||
| } | ||
|
|
||
| protected abstract void processLoop(); |
There was a problem hiding this comment.
Given that the worker is being interrupted to halt the worker, should we not invert control so the loop handles the interrupts? What if the implementation of this method catches the interrupt and resets the interrupted status?
There was a problem hiding this comment.
Yes, I'd like to refactor the worker loop further once this change is in (I'd prefer to keep that a separate PR though)
There was a problem hiding this comment.
@mcculls thank you so much for doing this. I think I speak not just for myself but for the community when I say really appreciate the cleanup and saner threading model. There was quite a bit of tech-debt here, this is awesome! 👏 🙇
These changes cleanup and simplify the internal threading model so that the Java tracer can upgrade to the latest level of
java-dogstatsd-client. It also adds support for using a custom thread factory which allows the Java tracer to configure StatsD threads so they are marked as agent related threads.List of the changes:
Avoid creating intermediate
NonBlockingStatsDClientsvia copy-constructorsbecause each call to
NonBlockingStatsDClientwill end up spawning threads - instead we:NonBlockingStatsDClientonceRemove unused copy-constructors
Remove intermediate thread pool, clean-up shutdown logic
Support using a custom thread factory to spawn StatsD worker threads
Replace fixed-size thread pools with a much simpler arrays of threads
(since each StatsD worker is a busy loop that takes up exactly one thread)