[jira] [Work logged] (GOBBLIN-724) Throttling server delays responses for throttling causing too many connections

[ASF GitHub Bot (JIRA)]

     [ 
https://issues.apache.org/jira/browse/GOBBLIN-724?focusedWorklogId=225161&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-225161
 ]

ASF GitHub Bot logged work on GOBBLIN-724:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 09/Apr/19 17:18
            Start Date: 09/Apr/19 17:18
    Worklog Time Spent: 10m 
      Work Description: ibuenros commented on pull request #2591: [GOBBLIN-724] 
Upgrade throttling server so waiting until tokens can be used is done…
URL: https://github.com/apache/incubator-gobblin/pull/2591#discussion_r273611552
 
 

 ##########
 File path: 
gobblin-restli/gobblin-throttling-service/gobblin-throttling-service-server/src/main/java/org/apache/gobblin/restli/throttling/DynamicTokenBucket.java
 ##########
 @@ -63,39 +74,56 @@
    * @param minPermits the minimum number of tokens useful for the calling 
process. If this many tokens cannot be acquired,
    *                   the method will return 0 instead,
    * @param timeoutMillis the maximum wait the calling process is willing to 
wait for tokens.
-   * @return the number of allocated tokens.
+   * @return a {@link PermitsAndDelay} for the allocated permits.
    */
-  public long getPermits(long requestedPermits, long minPermits, long 
timeoutMillis) {
-
+  public PermitsAndDelay getPermitsAndDelay(long requestedPermits, long 
minPermits, long timeoutMillis) {
     try {
       long storedTokens = this.tokenBucket.getStoredTokens();
 
       long eagerTokens = storedTokens / 2;
       if (eagerTokens > requestedPermits && 
this.tokenBucket.getTokens(eagerTokens, 0, TimeUnit.MILLISECONDS)) {
-        return eagerTokens;
+        return new PermitsAndDelay(eagerTokens, 0, true);
       }
 
       long millisToSatisfyMinPermits = (long) (minPermits / 
this.tokenBucket.getTokensPerMilli());
       if (millisToSatisfyMinPermits > timeoutMillis) {
-        return 0;
+        return new PermitsAndDelay(0, 0, false);
       }
       long allowedTimeout = Math.min(millisToSatisfyMinPermits + 
this.baseTimeout, timeoutMillis);
 
       while (requestedPermits > minPermits) {
-        if (this.tokenBucket.getTokens(requestedPermits, allowedTimeout, 
TimeUnit.MILLISECONDS)) {
-          return requestedPermits;
+        long wait = this.tokenBucket.tryReserveTokens(requestedPermits, 
allowedTimeout);
+        if (wait >= 0) {
+          return new PermitsAndDelay(requestedPermits, wait, true);
         }
         requestedPermits /= 2;
       }
 
-      if (this.tokenBucket.getTokens(minPermits, allowedTimeout, 
TimeUnit.MILLISECONDS)) {
-        return minPermits;
+      long wait = this.tokenBucket.tryReserveTokens(minPermits, 
allowedTimeout);
+      if (wait >= 0) {
+        return new PermitsAndDelay(requestedPermits, wait, true);
       }
+
     } catch (InterruptedException ie) {
       // Fallback to returning 0
     }
 
-    return 0;
+    return new PermitsAndDelay(0, 0, true);
 
 Review comment:
   Impossible to satisfy means the policy cannot ever grant that many permits. 
In this case, it might have been that there is just too much contention at the 
moment, so the permits cannot be granted just now but may be later.
 
----------------------------------------------------------------
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:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 225161)
    Time Spent: 1h  (was: 50m)

> Throttling server delays responses for throttling causing too many connections
> ------------------------------------------------------------------------------
>
>                 Key: GOBBLIN-724
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-724
>             Project: Apache Gobblin
>          Issue Type: Bug
>            Reporter: Issac Buenrostro
>            Priority: Major
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> Currently, the throttling server implements throttling in part by delaying 
> the response with the permit allocation. However, when waiting to respond, 
> the request remains in flight utilizing system resources and severely 
> limiting how many clients can use the throttling server.
> As a fix, the server should respond immediately and ask the client to wait 
> before distributing the permits.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)