zipkin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-zipkin] Logic-32 commented on issue #2502: Adding storage-throttle module to address "over capacity" issues
Date Tue, 30 Apr 2019 22:21:21 GMT
Logic-32 commented on issue #2502: Adding storage-throttle module to address "over capacity"
issues
URL: https://github.com/apache/incubator-zipkin/pull/2502#issuecomment-488136796
 
 
   > There is a fair amount of code with experiential notes in. There aren't enough tests
for some of this, especially things like pool resizing we should have tests or notes on how
people are going to resize...
   >
   > Functionality wise, this looks ok.. we need to up the coverage a bit and also do a
multithreaded case .like pool of 10 consumers to prove throttling works under contension..
this could help smoke out any thread safety bugs.
   
   A majority of the tests currently reside in ThrottledCall.  Testing ThrottledStorageComponent
is proving to be quite difficult due to how Netflix's concurrency-limit works.  Reviewing
some of their tests, it looks like a certain amount of "latency" is required for the limiter
to function as expected.  So I can't simply submit tasks that either pass/fail and expect
the limit to go up/down.
   
   If the additional tests in ThrottledCall are not sufficient then I'd have to request we
do some brainstorming of some kind.  The only route I see to testing ThrottledStorageComponent
more would involve some significant refactoring to inject mocks/etc. which I'm not sure I
can dedicate the time to :(

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message