HBASE-25212 [branch-2] Optionally abort requests in progress after deciding a region should close #2575
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
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
apurtell
force-pushed
the
HBASE-25212-branch-2
branch
from
f436d24
to
b769ce3
Compare
|
🎊 +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 one looks ready to go. Going to flip the CLOSE_ABORT default to 'true' on the master patch and see how precommit does there. branch-1 version has some checkstyle nits and otherwise looks good, fixing those on next push. |
apurtell
force-pushed
the
HBASE-25212-branch-2
branch
from
467549a
to
b4b10ef
Compare
|
Rebase |
|
🎊 +1 overall
This message was automatically generated. |
|
Fix remaining reported linelength checkstyle nits |
|
🎊 +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
force-pushed
the
HBASE-25212-branch-2
branch
from
20e642e
to
2ea1807
Compare
|
Rebase. Sync with updates to master branch patch. |
|
🎊 +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. |
|
Rebased and removed some checkInterrupt cases to address review feedback. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
apurtell
force-pushed
the
HBASE-25212-branch-2
branch
from
472efe9
to
5255c41
Compare
|
Rework disable/enableInterrupt to use existing try-finally blocks to reduce whitespace divergence |
|
🎊 +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. |
added 9 commits
November 3, 2020 11:30
…gion should close If hbase.regionserver.close.wait.abort is set to true, interrupt RPC handler threads holding the region close lock. Until requests in progress can be aborted, wait on the region close lock for a configurable interval (specified by hbase.regionserver.close.wait.time, in ms). If we have failed to acquire the close lock after this interval elapses, if allowed (also specified by hbase.regionserver.close.wait.abort), abort the regionserver. Define a subset of region operations as interruptable. Track threads holding the close lock transiting those operations. Set the thread interrupt status of tracked threads when trying to close the region. Use the thread interrupt status where safe to break out of request processing.
- Address review feedback - Fix spotbugs warnings - Fix checkstyle nit - Fix new unit tests, missed a needed HTU change for master branch - Fix more unit tests after HTU change - Reduce chance of race conditions in new TestHRegion units by joining on lock holder
… acquire the region close lock. Wait for one lock acquisition attempt before interrupting region opeations. Repeat interrupts at this interval until the lock is acquired or the maximum close wait time has been reached. If we interrupt too soon we are too aggressive. Allowing some time for operations in flight to complete is reasonable. This has added benefit of retrying the interrupt attempt in the event that one or more handlers were not interrupted because third party code swallowed the interrupt. - Use a less conservative default close wait limit of 1 minute (60000 ms). - Address more review feedback. - Fix a race problem with TestHRegion#testCloseAbort where we may need to return a valid ServerName from the mock RegionServerServices.
apurtell
force-pushed
the
HBASE-25212-branch-2
branch
from
0334acf
to
3a25188
Compare
|
Last commit on PR branch addresses remaining feedback, going to commit shortly. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
clarax
pushed a commit
to clarax/hbase
that referenced
this pull request
Nov 15, 2020
…ciding a region should close (apache#2575) If hbase.regionserver.close.wait.abort is set to true, interrupt RPC handler threads holding the region close lock. Until requests in progress can be aborted, wait on the region close lock for a configurable interval (specified by hbase.regionserver.close.wait.time.ms, default 60000 (1 minute)). If we have failed to acquire the close lock after this interval elapses, if allowed (also specified by hbase.regionserver.close.wait.abort), abort the regionserver. We will attempt to interrupt any running handlers every hbase.regionserver.close.wait.interval.ms (default 10000 (10 seconds)) until either the close lock is acquired or we reach the maximum wait time. Define a subset of region operations as interruptible. Track threads holding the close lock transiting those operations. Set the thread interrupt status of tracked threads when trying to close the region. Use the thread interrupt status where safe to break out of request processing. Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org> Signed-off-by: Viraj Jasani <vjasani@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.
If hbase.regionserver.close.wait.abort is set to true, interrupt RPC
handler threads holding the region close lock.
Until requests in progress can be aborted, wait on the region close lock
for a configurable interval (specified by hbase.regionserver.close.wait.time,
in ms). If we have failed to acquire the close lock after this interval
elapses, if allowed (also specified by hbase.regionserver.close.wait.abort),
abort the regionserver.
Define a subset of region operations as interruptable. Track threads holding
the close lock transiting those operations. Set the thread interrupt status
of tracked threads when trying to close the region. Use the thread interrupt
status where safe to break out of request processing.