keith-turner commented on code in PR #4983:
URL: https://github.com/apache/accumulo/pull/4983#discussion_r1815465958
##########
core/src/main/java/org/apache/accumulo/core/client/admin/ScanState.java:
##########
@@ -34,5 +34,15 @@ public enum ScanState {
* Indicates a task is queued in a server side thread pool to fetch the next
bach of key/values
* for a scan.
*/
- QUEUED
+ QUEUED,
+
+ /**
+ * Indicate the session is no longer accessible and is in the process of
cleaning up resources
+ */
+ CLEANING,
+ /**
+ * Indicate the client session is no longer accessible and still has a
thread running on the
+ * server side
Review Comment:
```suggestion
* server side
*
* @since 3.1.0
```
##########
core/src/main/java/org/apache/accumulo/core/client/admin/ScanState.java:
##########
@@ -34,5 +34,15 @@ public enum ScanState {
* Indicates a task is queued in a server side thread pool to fetch the next
bach of key/values
* for a scan.
*/
- QUEUED
+ QUEUED,
+
+ /**
+ * Indicate the session is no longer accessible and is in the process of
cleaning up resources
+ */
Review Comment:
```suggestion
* @since 3.1.0
*/
```
##########
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);
+ }
+ break;
Review Comment:
If a session is in the removed state where its no longer accessible to
clients and it still has a thread running, then it could be considered a
zombie. The `zombieCountConsumer` instance variable is related to metrics and
it being set or not does mean anything for a specific session. The two handle
methods seem to have a lot of side effects that should not happened while
computing the current state of the session and could probably removed.
```suggestion
if(session.getState() == State.REMOVED){
state = ScanState.ZOMBIE;
}
break;
```
##########
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;
Review Comment:
When a scan sessions state is removed its no longer accessible by clients.
If it has no associated task when removed then it can be presumed to be hanging
around so it can be cleaned up.
```suggestion
if(session.getState() == State.REMOVED) {
state = ScanState.CLEANING;
} else {
state = ScanState.IDLE;
}
```
--
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]
