apurtell commented on pull request #4180: URL: https://github.com/apache/hbase/pull/4180#issuecomment-1079217460
I have an objection as to the naming choice. Let's discuss that first. The bulk of the work seems great and I suspect we can incorporate it. > Creates a ServerOverloadedException as a base class for both exceptions. Creates a new configuration hbase.client.pause.server.overloaded and deprecates hbase.client.pause.cqtbe. Updates all references to "cqtbe" in code to "serverOverloaded", and changes instanceof checks to check for new base class ServerOverloadedException. I confess I do not really like **ServerOverloaded**Exception. This hard codes the opinion, and expectation, that a dropped call is due to server overloading. Is that always going to be true? Maybe not. Maybe we want to drop calls per user someday, hand waving here, the server is fine in that case and it would be user behavior that is the issue. (Ok, admittedly we have ThrottlingException for this, although it is triggered by other code paths.) We might in the future adopt another queuing discipline that performs admission control considering more than simply server load / queue depth. Can we simply name the cause as expressed in configuration keys and exception names as **CallDropped**? Can we do `CallQueueTooBigException extends CallDroppedException`? or is that a binary compatibility issue? This is not an overly strong opinion. Others may have a different take. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
