keith-turner commented on code in PR #5986:
URL: https://github.com/apache/accumulo/pull/5986#discussion_r2547410523
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -725,6 +725,7 @@ public interface MetricsProducer {
String METRICS_SCAN_SCANNED_ENTRIES = METRICS_SCAN_PREFIX +
"query.scanned.entries";
String METRICS_SCAN_ZOMBIE_THREADS = METRICS_SCAN_PREFIX + "zombie.threads";
String METRICS_SCAN_TABLET_METADATA_CACHE = METRICS_SCAN_PREFIX +
"tablet.metadata.cache";
+ String METRICS_SCAN_EXECUTOR_EXCEPTIONS = METRICS_SCAN_PREFIX +
"executor.exceptions";
Review Comment:
Could remove executor from the name as there is a tag named executor. Then
would have a metric named `accumulo.scan.exceptions` w/ a tag `executor=...`.
```suggestion
String METRICS_SCAN_EXECUTOR_EXCEPTIONS = METRICS_SCAN_PREFIX +
"exceptions";
```
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java:
##########
@@ -93,10 +106,12 @@ public void run() {
addResult(iie);
}
} catch (TooManyFilesException | SampleNotPresentException e) {
+ recordException(scanSession);
Review Comment:
NextBatchTask run executes normal scans on the server side. There is
another class named LookupTask that executes batch scans on there server side.
Will need to add similar code to it.
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -725,6 +725,7 @@ public interface MetricsProducer {
String METRICS_SCAN_SCANNED_ENTRIES = METRICS_SCAN_PREFIX +
"query.scanned.entries";
Review Comment:
Further up in this file there is javadoc that needs to be updated when
adding a new metric.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java:
##########
@@ -48,6 +49,18 @@ public NextBatchTask(TabletHostingServer server, long
scanID, AtomicBoolean inte
}
}
+ private void recordException(SingleScanSession scanSession) {
+ if (scanSession != null && server.getScanMetrics() != null) {
+ String executorName = getExecutorName(scanSession);
+ server.getScanMetrics().incrementExecutorExceptions(executorName);
+ }
+ }
+
+ private String getExecutorName(SingleScanSession scanSession) {
+ String executorName = scanSession.getExecutionHints().get("scan_type");
Review Comment:
The scan hint can influence which executor is used, but it does not have to
be present and table config can also influence what scan executor is used.
Should probably do the following to get the executor name.
```suggestion
return scanSession.scanParams.getScanDispatch().getExecutorName();
```
--
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]
