virajjasani commented on code in PR #6789:
URL: https://github.com/apache/hbase/pull/6789#discussion_r2032125220
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -8682,15 +8706,15 @@ protected void decrementFlushesQueuedCount() {
* {{@link #enableInterrupts()}.
*/
void disableInterrupts() {
- regionLockHolders.computeIfPresent(Thread.currentThread(), (t, b) ->
false);
+ intrRegionLockHolders.computeIfPresent(Thread.currentThread(), (t, b) ->
false);
Review Comment:
I thought earlier version of having single map was simpler. It seems bit
confusing now that we have two maps `intrRegionLockHolders` for interruptible
threads and `nonintrRegionLockHolders` for non-interruptible threads.
However, here in disableInterrupts(), we update interruptible as
non-interruptible so even though we have `intrRegionLockHolders` for
interruptible threads, we can make them non-interruptible and still have them
stay in `intrRegionLockHolders` map 🤔
In a way, the interruptible info is only meaningfully exercised by
`interruptRegionOperations()` for which the previous version of the PR was good
enough I think.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -8408,42 +8423,21 @@ public void startRegionOperation() throws IOException {
@Override
public void startRegionOperation(Operation op) throws IOException {
- boolean isInterruptableOp = false;
- switch (op) {
- case GET: // interruptible read operations
- case SCAN:
- isInterruptableOp = true;
- checkReadsEnabled();
- break;
- case INCREMENT: // interruptible write operations
- case APPEND:
- case PUT:
- case DELETE:
- case BATCH_MUTATE:
- case CHECK_AND_MUTATE:
- isInterruptableOp = true;
- break;
- default: // all others
- break;
- }
- if (
- op == Operation.MERGE_REGION || op == Operation.SPLIT_REGION || op ==
Operation.COMPACT_REGION
- || op == Operation.COMPACT_SWITCH
- ) {
- // split, merge or compact region doesn't need to check the
closing/closed state or lock the
- // region
- return;
- }
+ boolean isInterruptibleOp = isOperationInterruptible(op);
+ if (!shouldLock(op)) return;
Review Comment:
Yes, this too was simpler in the earlier version of the PR.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
