ctubbsii commented on a change in pull request #2347:
URL: https://github.com/apache/accumulo/pull/2347#discussion_r744984895



##########
File path: 
core/src/main/java/org/apache/accumulo/core/iterators/IterationInterruptedException.java
##########
@@ -18,6 +18,9 @@
  */
 package org.apache.accumulo.core.iterators;
 
+/**
+ * Exception thrown if an interrupt flag is detected.
+ */
 public class IterationInterruptedException extends RuntimeException {

Review comment:
       Should this RTE exist in public API, or should it be in iteratorsImpl as 
well?

##########
File path: 
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/SortedMapIterator.java
##########
@@ -121,4 +143,9 @@ public void init(SortedKeyValueIterator<Key,Value> source, 
Map<String,String> op
       IteratorEnvironment env) throws IOException {
     throw new UnsupportedOperationException();
   }
+
+  @Override
+  public void setInterruptFlag(AtomicBoolean flag) {
+    this.interruptFlag = flag;
+  }

Review comment:
       Because changing the reference breaks atomicity guarantees, this method 
looks dubious. It looks like the way this is intended to be used is like a 
BooleanSupplier (or even more strictly, a one-way latch that can move from 
false to true, but never back to false). I'm not sure the right solution, but 
if the interruptible stuff is only on the server-side, it could be cleaned up 
to avoid using AtomicBoolean and use something more appropriate instead.




-- 
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]


Reply via email to