[GitHub] [accumulo] EdColeman commented on a diff in pull request #3239: Use CompletableFuture for the result in ScanTask

["EdColeman (via GitHub)" <gi...@apache.org>]

EdColeman commented on code in PR #3239:
URL: https://github.com/apache/accumulo/pull/3239#discussion_r1136337609


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java:
##########
@@ -33,7 +33,7 @@
 
   protected final TabletHostingServer server;
   protected AtomicBoolean interruptFlag;
-  protected ArrayBlockingQueue<Object> resultQueue;
+  protected CompletableFuture<Object> resultFuture;

Review Comment:
   Is this required to be accessed by sub-classes?  Looking at the usages, it seems internal to this class - but not sure we can change now (if it is changing the API?)



##########
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java:
##########
@@ -73,30 +73,24 @@ public boolean cancel(boolean mayInterruptIfRunning) {
 
     if (state.compareAndSet(INITIAL, CANCELED)) {
       interruptFlag.set(true);
-      resultQueue = null;
+      resultFuture = null;

Review Comment:
   Should this be either cancel() or complete() - setting it to null will block future access to the future - but I don't think it impacts the current task if it is running.  This happens two places in this file.



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org