keith-turner commented on code in PR #5102:
URL: https://github.com/apache/accumulo/pull/5102#discussion_r1855526562


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ActiveScanImpl.java:
##########
@@ -44,20 +46,21 @@ public class ActiveScanImpl extends ActiveScan {
 
   private final long scanId;
   private final String client;
-  private String tableName;
+  private final String tableName;
   private final long age;
   private final long idle;
-  private ScanType type;
-  private ScanState state;
-  private KeyExtent extent;
-  private List<Column> columns;
-  private List<String> ssiList;
-  private Map<String,Map<String,String>> ssio;
+  private final ScanType type;

Review Comment:
   Nice to change these to final.



##########
core/src/main/java/org/apache/accumulo/core/client/admin/ActiveScan.java:
##########
@@ -96,4 +97,11 @@ public abstract class ActiveScan {
    * @since 1.5.0
    */
   public abstract long getIdleTime();
+
+  /**
+   * Return the host where the compaction is running.
+   *
+   * @since 4.0.0
+   */
+  public abstract ServerId getHost();

Review Comment:
   There sevrver id contains more info than just the hostname, so this getter 
may need a different name.
   
   ```suggestion
     public abstract ServerId getServeId();
   ```



##########
core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java:
##########
@@ -265,33 +269,17 @@ public List<String> getTabletServers() {
   @Deprecated(since = "4.0.0")
   public List<ActiveScan> getActiveScans(String tserver)
       throws AccumuloException, AccumuloSecurityException {
-    final var parsedTserver = HostAndPort.fromString(tserver);
-    TabletScanClientService.Client client = null;
-    try {
-      client = getClient(ThriftClientTypes.TABLET_SCAN, parsedTserver, 
context);
-
-      List<ActiveScan> as = new ArrayList<>();
-      for (var activeScan : client.getActiveScans(TraceUtil.traceInfo(), 
context.rpcCreds())) {
-        try {
-          as.add(new ActiveScanImpl(context, activeScan));
-        } catch (TableNotFoundException e) {
-          throw new AccumuloException(e);
-        }
-      }
-      return as;
-    } catch (ThriftSecurityException e) {
-      throw new AccumuloSecurityException(e.user, e.code, e);
-    } catch (TException e) {
-      throw new AccumuloException(e);
-    } finally {
-      if (client != null) {
-        returnClient(client, context);
-      }
-    }
+    ServerId si = getServerId(tserver);

Review Comment:
   `getServerId` may only look at COMPACTOR and TSERVER.  In this case may need 
to look at scan server and tablet server.



##########
shell/src/main/java/org/apache/accumulo/shell/commands/ListScansCommand.java:
##########
@@ -66,11 +72,34 @@ public int execute(final String fullCommand, final 
CommandLine cl, final Shell s
       servers.addAll(instanceOps.getServers(ServerId.Type.TABLET_SERVER));
     }
 
-    shellState.printLines(new ActiveScanIterator(servers, instanceOps), 
paginate);
+    Stream<String> activeScans = getActiveScans(instanceOps, servers);
+    activeScans = appendHeader(activeScans);
+    shellState.printLines(activeScans.iterator(), paginate);
 
     return 0;
   }
 
+  private Stream<String> getActiveScans(InstanceOperations instanceOps, 
List<ServerId> servers) {
+    List<List<ServerId>> partServerIds = Lists.partition(servers, 100);
+    return partServerIds.stream().flatMap(ids -> {
+      try {
+        return instanceOps.getActiveScans(ids).stream().map(as -> {
+          var dur = new DurationFormat(as.getAge(), "");
+          var dur2 = new DurationFormat(as.getLastContactTime(), "");
+          var server = as.getHost();
+          return (String.format(
+              "%21s |%21s |%21s |%9s |%9s |%7s |%6s |%8s |%8s |%10s |%20s 
|%10s |%20s |%10s | %s",
+              server.getResourceGroup(), server.toHostPortString(), 
as.getClient(), dur, dur2,
+              as.getState(), as.getType(), as.getUser(), as.getTable(), 
as.getColumns(),
+              as.getAuthorizations(), (as.getType() == ScanType.SINGLE ? 
as.getTablet() : "N/A"),
+              as.getScanid(), as.getSsiList(), as.getSsio()));
+        });
+      } catch (AccumuloException | AccumuloSecurityException e) {
+        return Stream.of("ERROR " + e.getMessage());

Review Comment:
   That is an interesting way to propagate the error message.



-- 
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-unsubscr...@accumulo.apache.org

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

Reply via email to