Re: [PR] HBASE-29231 Throttles should support limits based on handler thread usage time [hbase]

2025-12-12 Thread via GitHub


Apache9 commented on PR #7000:
URL: https://github.com/apache/hbase/pull/7000#issuecomment-3646873765

   TestThreadHandlerUsageQuota fails for me locally most times, and it is also 
on the flaky dashboard.
   
   Go deep into the logic, I found that I still can not understand the logic of 
calculateHandlerUsageTimeEstimate. In general, I think here we are trying to 
predicate how much time a request will be consumed, but why we just use 
`requestsPerSecond /1000`?  How could this work? The unit of `requestsPerSecond 
/ 1000` is `/ms`, but here what we want is `ms`? At least we should use 
reciprocal of this value?
   
   Can someone explain more on this?
   
   @rmdmattingly @ajkh88 
   
   Thanks.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] HBASE-29231 Throttles should support limits based on handler thread usage time [hbase]

2025-06-05 Thread via GitHub


ajkh88 commented on code in PR #7000:
URL: https://github.com/apache/hbase/pull/7000#discussion_r2128259789


##
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/DefaultOperationQuota.java:
##
@@ -288,4 +309,25 @@ private long calculateWriteCapacityUnitDiff(final long 
actualSize, final long es
   private long calculateReadCapacityUnitDiff(final long actualSize, final long 
estimateSize) {
 return calculateReadCapacityUnit(actualSize) - 
calculateReadCapacityUnit(estimateSize);
   }
+
+  private long calculateHandlerUsageTimeEstimate(final double 
requestsPerSecond,

Review Comment:
   Yes, that is pretty much it—we create the `OperationQuota` object and first 
'check' it, meaning we estimate how much of the quota the operation will 
consume. The quota represents different resources for different throttle types 
(it could be time, space, etc.). Each quota type calculates its estimate 
differently, but they are all similar in that they don’t try anything too 
complicated; we just want a general idea of what will be used.
   
   We compare our estimate to the current remaining quota, and if the estimate 
would consume it all, we throw an `RpcThrottlingException` (also calculating a 
wait interval here). This stops the request and backs off any retries.
   
   If the estimate will not consume all the quota, then we 'grab', the quota - 
meaning we subtract the estimate from it.
   
   Finally, when the `OperationQuota::close` method is called, we calculate the 
_actual_ time taken and update the quota accordingly, either adding to or 
subtracting from it.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] HBASE-29231 Throttles should support limits based on handler thread usage time [hbase]

2025-06-03 Thread via GitHub


Apache9 commented on code in PR #7000:
URL: https://github.com/apache/hbase/pull/7000#discussion_r2125257856


##
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/DefaultOperationQuota.java:
##
@@ -288,4 +309,25 @@ private long calculateWriteCapacityUnitDiff(final long 
actualSize, final long es
   private long calculateReadCapacityUnitDiff(final long actualSize, final long 
estimateSize) {
 return calculateReadCapacityUnit(actualSize) - 
calculateReadCapacityUnit(estimateSize);
   }
+
+  private long calculateHandlerUsageTimeEstimate(final double 
requestsPerSecond,

Review Comment:
   I'm not very familiar with the quota implementation, so for every operation, 
we will create a OperationQuota object and calculate the estimate time, and 
then compare it with the actual time usage, and see whether we should stop it?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] HBASE-29231 Throttles should support limits based on handler thread usage time [hbase]

2025-06-03 Thread via GitHub


rmdmattingly merged PR #7000:
URL: https://github.com/apache/hbase/pull/7000


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]