-
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-28352 HTable batch does not honor RpcThrottlingException waitInterval #5671
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -842,6 +849,18 @@ private void resubmit(ServerName oldServer, List<Action> toReplay, int numAttemp | |||
backOffTime, true, null, -1, -1)); | |||
} | |||
|
|||
long remainingTime = getRemainingTime(); | |||
// 1 is a special value meaning exceeded and 0 means no timeout | |||
if (remainingTime > 1 && backOffTime > remainingTime) { |
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.
I added this handling here as well. We have similar logic in RpcRetryingCallerImpl. I think it's important to add when adding throttling wait interval, because depending on the level of quota saturation the wait time could easily be higher than operation timeout.
🎊 +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. |
🎊 +1 overall
This message was automatically generated. |
This reverts commit 028dfeb.
67b0e53
to
0fa7f47
Compare
AsyncProcess is a nightmare... Hope we could get 3.0.0 out soon and start to use the new client implementation... |
💔 -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. |
Agreed! Thanks for the review. I can try to help on 3.0.0 hopefully in a couple months if it's still in progress. I need to get some internal day job things squared away first and start testing it internally. |
I also sort of want to backport TableOverAsyncTable as an optional feature in 2.x, if we can't get 3.x out soonish |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
I think we just need more testing, and also a final round of API cleanup, we should remove all the deprecated APIs which are marked deprecated since 2.0.0. There is a known issue that the memstore refernce counting is broken, but it is likely that 2.5.x also has the problem, so if we still can not figure out the root cause finally, it should not be a blocker anyway... |
…terval (#5671) Signed-off-by: Duo Zhang <zhangduo@apache.org>
…ngException waitInterval (apache#5671) Signed-off-by: Duo Zhang <zhangduo@apache.org>
edit: Tests have been added, ready for review