kevinrr888 commented on code in PR #4983:
URL: https://github.com/apache/accumulo/pull/4983#discussion_r1847202120


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,34 +458,37 @@ public List<ActiveScan> getActiveScans() {
         addActiveScan(activeScans, scanSession,
             isSingle ? ((SingleScanSession) scanSession).extent
                 : ((MultiScanSession) scanSession).threadPoolExtent,
-            ct, isSingle ? ScanType.SINGLE : ScanType.BATCH,
-            computeScanState(scanSession.getScanTask()), 
scanSession.scanParams, entry.getKey());
+            ct, isSingle ? ScanType.SINGLE : ScanType.BATCH, 
computeScanState(scanSession),
+            scanSession.scanParams, entry.getKey());
       }
     }));
 
     return activeScans;
   }
 
-  private ScanState computeScanState(ScanTask<?> scanTask) {
+  private ScanState computeScanState(ScanSession<?> session) {
     ScanState state = ScanState.RUNNING;
 
-    if (scanTask == null) {
-      state = ScanState.IDLE;
+    if (session.getState() == State.REMOVED) {
+      state = ScanState.CLEANING;
     } else {
-      switch (scanTask.getScanRunState()) {
+      switch (session.getScanTask().getScanRunState()) {
         case QUEUED:
           state = ScanState.QUEUED;
           break;
         case FINISHED:
           state = ScanState.IDLE;
           break;
         case RUNNING:
+          if (session.getState() == State.REMOVED) {
+            state = ScanState.ZOMBIE;
+          }
+          break;

Review Comment:
   The current logic is:
   ```
       if (session.getState() == State.REMOVED) {
         state = ScanState.CLEANING;
       } else {
         ...
         if (session.getState() == State.REMOVED) {
           state = ScanState.ZOMBIE;
         }
         ...
       }
   ```
   `state = ScanState.ZOMBIE` will never execute in this logic. It seems like 
either (or both) the logic for identifying ZOMBIE or CLEANING need to be 
changed (but I'm not sure which or what it should be changed to)



##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,34 +458,37 @@ public List<ActiveScan> getActiveScans() {
         addActiveScan(activeScans, scanSession,
             isSingle ? ((SingleScanSession) scanSession).extent
                 : ((MultiScanSession) scanSession).threadPoolExtent,
-            ct, isSingle ? ScanType.SINGLE : ScanType.BATCH,
-            computeScanState(scanSession.getScanTask()), 
scanSession.scanParams, entry.getKey());
+            ct, isSingle ? ScanType.SINGLE : ScanType.BATCH, 
computeScanState(scanSession),
+            scanSession.scanParams, entry.getKey());
       }
     }));
 
     return activeScans;
   }
 
-  private ScanState computeScanState(ScanTask<?> scanTask) {
+  private ScanState computeScanState(ScanSession<?> session) {
     ScanState state = ScanState.RUNNING;
 
-    if (scanTask == null) {
-      state = ScanState.IDLE;
+    if (session.getState() == State.REMOVED) {
+      state = ScanState.CLEANING;
     } else {
-      switch (scanTask.getScanRunState()) {
+      switch (session.getScanTask().getScanRunState()) {

Review Comment:
   This removes the null check for the scanTask and I think it would still be 
needed, however, I'm not sure where it would go especially considering my next 
comment



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