HBASE-27078 Allow configuring a separate timeout for meta scans (branch-2 backport) #4585
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bbeaudreault
added
the
backport
|
🎊 +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. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@apurtell any chance you can take a look at this backport when you get a chance? I had some decisions to make (documented in description), would like them gut checked if possible. |
|
@apurtell one more ping, if you have time. Since this is just a backport I'll go ahead merging in the next day or two if I don't hear any concerns. Here's the relevant part I'd like another opinion on:
|
I would claim it is directly fixable in branch-2.5 and branch-2. This is a change we can make on a minor, if there is some attention paid to compatibility with the earlier setting. Consider something functionally like: When resolving the JIRA add a release note so docmaker will pick up a note about the change. For branch-2.4, agree, I don't think we should touch it. The strategy you use in c52e2bf is fine there. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
bbeaudreault
force-pushed
the
HBASE-27078-2
branch
from
30544aa
to
713a6b0
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
bbeaudreault
added a commit
that referenced
this pull request
Jul 7, 2022
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>
bbeaudreault
added a commit
to HubSpot/hbase
that referenced
this pull request
Jul 12, 2022
…r meta scans (apache#4585) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First commit is a cherry-pick from master branch, with some minor conflict resolution.
Second commit re-implements for the blocking client. For this, part of the complexity is that the rpc timeout needs to be pushed way down into ResultBoundedCompletionService's RetryingRpcCaller. What I have now seems like the cleanest way to achieve that, doing the
isSystemTable()check in TableBuilder, then passing down into ClientScanner constructors, ScannerCallble constructors, etc. We already had this pattern for replica timeout, but it now adds rpcTimeout and scannerTimeout.Another piece of complexity, covered by my 3rd commit, is that unfortunately in branch-2 scans use
hbase.rpc.timeoutinstead ofhbase.read.rpc.timeout. If anyone thinks its allowed, I'd love to fix that. But I assume it's not allowed, so the 3rd commit handles the case that we want to usehbase.rpc.timeoutfor normal scans andhbase.client.meta.read.rpc.timeoutfor meta scans. This will be unified in 3.0.0 where scans will usehbase.read.rpc.timeoutandhbase.client.meta.read.rpc.timeoutfor both AsyncTable and Table.The tests needed minor changes, mostly to support the different semantics of scanner next() timeouts in blocking client. More details in a comment of
testNormalScanTimeoutOnNext