-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-26551 Add FastPath feature to HBase RWQueueRpcExecutor #3929
Conversation
I have two considerations.
|
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Reconstructed the code and extracted the RpcHandler to avoid the redundant coding of fastpath handler. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
The failed UT is not related with the changes. |
LGTM. |
Left some comments to explain in the corresponding ticket. @Reidddddd |
public boolean dispatch(final CallRunner callTask) throws InterruptedException { | ||
RpcCall call = callTask.getRpcCall(); | ||
boolean isWriteRequest = isWriteRequest(call.getHeader(), call.getParam()); | ||
boolean shouldDispatchToScanQueue = shouldDispatchToScanQueue(callTask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here it uses isWriteRequest
, can you rename it isScanRequest
as well? Naming alias.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about change the isWriteRequest to shouldDispatchToWriteQueue. While scan requests can also be handled as read requests when the scan ratio is 0.0.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Refined the variable name in the latest commit. I think this is more readable then. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@Reidddddd Any new comments here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping (pardon me, I forgot it)
OK to go, +1
https://issues.apache.org/jira/browse/HBASE-26551