Apache9 commented on a change in pull request #2574:
URL: https://github.com/apache/hbase/pull/2574#discussion_r515991892
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -7623,6 +7740,9 @@ protected boolean nextRow(ScannerContext scannerContext,
Cell curRowCell) throws
Cell next;
while ((next = this.storeHeap.peek()) != null &&
CellUtil.matchingRows(next, curRowCell)) {
+ // Check for thread interrupt status in case we have been signaled from
+ // #interruptRegionOperation.
+ checkInterrupt();
Review comment:
I wonder whether it is possible to return what we have to client instead
of throwing an exception to client. Anyway, Can be a follow on issue, not a
blocker here.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4565,17 +4663,23 @@ private void doMiniBatchMutate(BatchOperation<?>
batchOp) throws IOException {
lock(this.updatesLock.readLock(), miniBatchOp.getReadyToWriteCount());
locked = true;
+ // From this point until memstore update this operation should not be
interrupted.
+ disableInterrupts();
Review comment:
I think here we could have a checkInterrupt? And for me, I think before
STEP 4, we could always interrupt the handler but I wonder whether it worth.
The prepare mini batch and build WAL are all in memory operations so they
should be fast. So I suggest that, we call checkInterrupt here, if we pass,
then remove the handler from the Map.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -8303,9 +8428,14 @@ public void processRowsWithLocks(RowProcessor<?,?>
processor, long timeout,
prevRowLock = rowLock;
}
}
+
// STEP 3. Region lock
lock(this.updatesLock.readLock(), acquiredRowLocks.isEmpty() ? 1 :
acquiredRowLocks.size());
locked = true;
+
+ // From this point until memstore update this operation should not be
interrupted.
+ disableInterrupts();
Review comment:
Same with the doMiniBatchMutation above.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4565,17 +4663,23 @@ private void doMiniBatchMutate(BatchOperation<?>
batchOp) throws IOException {
lock(this.updatesLock.readLock(), miniBatchOp.getReadyToWriteCount());
locked = true;
+ // From this point until memstore update this operation should not be
interrupted.
+ disableInterrupts();
Review comment:
Oh, you are right, I recall that we have a loop and will only acquire
row locks as many as possible in each loop. Then this should be fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
