Improved abstractions + better object construction #96
Conversation
| import java.nio.charset.Charset; | ||
| import java.util.concurrent.Callable; | ||
|
|
||
| import java.util.Queue; |
There was a problem hiding this comment.
Note: Several imports are unused.
There was a problem hiding this comment.
We have to add linting to the CI - thank you for checking. I don't use an IDE, so..... 😊
| } | ||
| } | ||
|
|
||
| boolean isShutdown() { |
There was a problem hiding this comment.
Note: As shutdown is defined in StatsDProcessor, isShutdown() and shutdown() could be moved to StatsDProcessor. Same comment could also be applied for StatsDNonBlockingProcessor.
There was a problem hiding this comment.
Yup, code was already defined in StatsdProcessor just forgot doing this cleanup. Thank you! Great catch again.
| qSize.incrementAndGet(); | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Would it make sense to catch InterruptedException as in https://github.com/DataDog/java-dogstatsd-client/pull/96/files#diff-d87f261783303f309bda2086a2e14756R35?
There was a problem hiding this comment.
I wouldn't expect an InterruptedException to happen there as we are dealing with a non-blocking queue in this case and the offer() method would just return as opposed to stay there waiting for the write.
| } | ||
| } catch (final InterruptedException e) { | ||
| if (shutdown) { | ||
| return; |
There was a problem hiding this comment.
Do we need endSignal.countDown(); as for https://github.com/DataDog/java-dogstatsd-client/pull/96/files#diff-d87f261783303f309bda2086a2e14756R86
| { 30, 17255, 2048, 2, 2 }, // 30 seconds, 17255 port, 2048 qSize, 2 sender workers, 2 processor workers | ||
| { 30, 17255, 4096, 2, 2 } // 30 seconds, 17255 port, 4096 qSize, 2 sender workers, 2 processor workers | ||
| // // { 30, 17255, Integer.MAX_VALUE, 2 } // 30 seconds, 17255 port, MAX_VALUE qSize, 2 sender workers | ||
| { 30, false, 256, 1, 1 }, // 30 seconds, non-blocking, 256 qSize, 1 worker |
There was a problem hiding this comment.
Note: This collection could be created with 3 "for loop" (non-blocking, Qsize, worker count)
truthbk
force-pushed
the
jaime/abstractions
branch
from
b527293 to
98b374c
Compare
truthbk
force-pushed
the
jaime/abstractions
branch
from
68f1749 to
774f11e
Compare
truthbk
force-pushed
the
jaime/abstractions
branch
from
774f11e to
4e6b027
Compare
|
Addressed changes + fixed CI to skip Merging onto #95 |
Introduces breaking changes.
Better description pending...