d-c-manning commented on code in PR #7490:
URL: https://github.com/apache/hbase/pull/7490#discussion_r2686681302
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcExecutor.java:
##########
@@ -222,6 +230,11 @@ public Map<String, Long> getCallQueueSizeSummary() {
.collect(Collectors.groupingBy(Pair::getFirst,
Collectors.summingLong(Pair::getSecond)));
}
+ // IMPORTANT: Call this method only ONCE per executor instance.
Review Comment:
I wonder if we could just enforce this in the code? Only allow it to be
called once, and if it is called a second time then throw an "already
initialized" error?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcExecutor.java:
##########
@@ -296,7 +309,7 @@ protected void startHandlers(final String nameSuffix, final
int numHandlers,
*/
private static final QueueBalancer ONE_QUEUE = val -> 0;
- public static QueueBalancer getBalancer(final String executorName, final
Configuration conf,
+ protected static QueueBalancer getBalancer(final String executorName, final
Configuration conf,
Review Comment:
this does seem like a useful visibility change, but must it be included
here? Changing a public method of a "public" class (LimitedPrivate for COPROC,
PHOENIX) is a breaking change. That being said, it's hard to imagine how anyone
is using it outside of hbase... and the return class is not LimitedPrivate for
COPROC... and I don't see it used in Phoenix... so 🤷♂️
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleRpcScheduler.java:
##########
@@ -67,15 +67,13 @@ public SimpleRpcScheduler(Configuration conf, int
handlerCount, int priorityHand
Abortable server, int highPriorityLevel) {
int bulkLoadHandlerCount =
conf.getInt(HConstants.REGION_SERVER_BULKLOAD_HANDLER_COUNT,
HConstants.DEFAULT_REGION_SERVER_BULKLOAD_HANDLER_COUNT);
- int maxQueueLength =
conf.getInt(RpcScheduler.IPC_SERVER_MAX_CALLQUEUE_LENGTH,
- handlerCount * RpcServer.DEFAULT_MAX_CALLQUEUE_LENGTH_PER_HANDLER);
- int maxPriorityQueueLength =
conf.getInt(RpcScheduler.IPC_SERVER_PRIORITY_MAX_CALLQUEUE_LENGTH,
- priorityHandlerCount *
RpcServer.DEFAULT_MAX_CALLQUEUE_LENGTH_PER_HANDLER);
+ int maxQueueLength =
conf.getInt(RpcScheduler.IPC_SERVER_MAX_CALLQUEUE_LENGTH, -1);
Review Comment:
can we make the `-1` a constant too? something like
`MAX_CALLQUEUE_LENGTH_UNDEFINED`?
```suggestion
int maxQueueLength =
conf.getInt(RpcScheduler.IPC_SERVER_MAX_CALLQUEUE_LENGTH,
MAX_CALLQUEUE_LENGTH_UNDEFINED);
```
--
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]
