kevinrr888 commented on code in PR #4983:
URL: https://github.com/apache/accumulo/pull/4983#discussion_r1804962918
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,28 +458,36 @@ 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) {
+ if (session.getScanTask() == null) {
state = ScanState.IDLE;
} else {
- switch (scanTask.getScanRunState()) {
+ switch (session.getScanTask().getScanRunState()) {
case QUEUED:
state = ScanState.QUEUED;
break;
case FINISHED:
state = ScanState.IDLE;
break;
case RUNNING:
+ if (zombieCountConsumer != null) {
+ state = ScanState.ZOMBIE;
+ handleZombieScan(session);
+ } else if (session.getScanTask() == null) {
+ state = ScanState.CLEANING;
+ handleCleaningScan(session);
Review Comment:
I don't think any "handling" should be done... I believe the goal is to just
have a state identified as ZOMBIE or CLEANING and no further handling.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,28 +458,36 @@ 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) {
+ if (session.getScanTask() == null) {
state = ScanState.IDLE;
} else {
- switch (scanTask.getScanRunState()) {
+ switch (session.getScanTask().getScanRunState()) {
Review Comment:
could store the scan task in a var
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,28 +458,36 @@ 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) {
+ if (session.getScanTask() == null) {
state = ScanState.IDLE;
} else {
- switch (scanTask.getScanRunState()) {
+ switch (session.getScanTask().getScanRunState()) {
case QUEUED:
state = ScanState.QUEUED;
break;
case FINISHED:
state = ScanState.IDLE;
break;
case RUNNING:
+ if (zombieCountConsumer != null) {
+ state = ScanState.ZOMBIE;
+ handleZombieScan(session);
+ } else if (session.getScanTask() == null) {
Review Comment:
```suggestion
} else if (deferredCleanupQueue.contains(session)) {
```
I believe the CLEANING state is meant for sessions that haven't been cleaned
up yet (aka they have been deferred for cleanup)
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -458,28 +458,36 @@ 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) {
+ if (session.getScanTask() == null) {
state = ScanState.IDLE;
} else {
- switch (scanTask.getScanRunState()) {
+ switch (session.getScanTask().getScanRunState()) {
case QUEUED:
state = ScanState.QUEUED;
break;
case FINISHED:
state = ScanState.IDLE;
break;
case RUNNING:
+ if (zombieCountConsumer != null) {
Review Comment:
`zombieCountConsumer != null` is not the correct indicator for a ZOMBIE
scan. Look at `countZombieScans()`. You will probably have to write a method
similar to that to determine a zombie scan
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java:
##########
@@ -489,6 +497,43 @@ private ScanState computeScanState(ScanTask<?> scanTask) {
return state;
}
+ private void handleZombieScan(ScanSession<?> session) {
+ // Log that we have encountered a zombie scan
+ session.logZombieStackTrace();
+
+ // Perform any cleanup or resource deallocation
+ try {
+ // Stop any active tasks or threads associated with the session
+ if (session.getState() != null) {
+ session.clearScanTask();
+ }
+
+ log.info("Successfully cleaned up zombie session: "
+ + session.getScanTask().getScanThread().getId());
+ } catch (Exception e) {
+ log.error(
+ "Failed to cleanup zombie session: " +
session.getScanTask().getScanThread().getId(), e);
+ }
+
+ // Mark session as being cleaned up
+ cleanup(session);
+ }
+
+ private void handleCleaningScan(ScanSession<?> session) {
Review Comment:
I don't think this is working as intended
this is only called when
`session.getScanTask() == null`
but the method calls things like
`session.getScanTask().getScanThread().getId()`
which will always result in NPE...
--
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]
