[GitHub] [bookkeeper] dlg99 commented on pull request #3214: Issue 3070: Fix bug where checkAllLedgers gets stuck when read throttling is enabled

[GitBox <gi...@apache.org>]

dlg99 commented on PR #3214:
URL: https://github.com/apache/bookkeeper/pull/3214#issuecomment-1161990054

   @massakam I think that Throttle is not a good approach here.
   
   Requests-in-progress (RIPs) approach is very similar when everything works but easier to tune and self-adjusts to remote system's dynamically changing capabilities while throttle is impossible to tuen for that.
   Imagine you configured 20 RIPs and one request takes 100ms normally. Now you have 200 RPS (requests per second).
   Remote system (bookie) slowed down and now take 500ms to process a request, now you are down to 40RPS without overloading the bookie, changing config etc.
   Bookie speeds up and can process request in 10ms - the rate dynamically goes up to 2000rps.
   
   Let's use throttle now: set it to 200 request per second.
   In the normal case everything looks ok and the same as in normal case above.
   Now the bookie slows down.
   Throttle keeps on letting through 200 RPS even though the bookie can only handle 40, with that requests will pile up and eventually make result in OOM on bookie or client side. 
   Bookie speeds up and can process request in 10ms - the rate stays at 200rps.
   
   I'd prefer to track down where the semaphore is not released and fix that.
   
   Throttle is simple, does not require release etc. but it has its drawback and was not used there for a reason.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org