jojochuang commented on code in PR #7748:
URL: https://github.com/apache/ozone/pull/7748#discussion_r1929205742


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java:
##########
@@ -246,4 +204,91 @@ private File createDirectory(String volumeName, String 
bucketName,
     }
     return dir;
   }
+
+  private void findCandidateKeys(OzoneClient ozoneClient, OzoneAddress 
address) throws IOException {
+    ObjectStore objectStore = ozoneClient.getObjectStore();
+    String volumeName = address.getVolumeName();
+    String bucketName = address.getBucketName();
+    String keyName = address.getKeyName();
+    if (!keyName.isEmpty()) {
+      processKey(ozoneClient, volumeName, bucketName, keyName);
+    } else if (!bucketName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      OzoneBucket bucket = volume.getBucket(bucketName);
+      checkBucket(bucket, ozoneClient);
+    } else if (!volumeName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      checkVolume(volume, ozoneClient);
+    } else {
+      for (Iterator<? extends OzoneVolume> it = objectStore.listVolumes(null); 
it.hasNext();) {
+        checkVolume(it.next(), ozoneClient);
+      }
+    }
+  }
+
+  private void checkVolume(OzoneVolume volume, OzoneClient ozoneClient) throws 
IOException {
+    for (Iterator<? extends OzoneBucket> it = volume.listBuckets(null); 
it.hasNext();) {
+      OzoneBucket bucket = it.next();
+      checkBucket(bucket, ozoneClient);
+    }
+  }
+
+  private void checkBucket(OzoneBucket bucket, OzoneClient ozoneClient) throws 
IOException {
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+    for (Iterator<? extends OzoneKey> it = bucket.listKeys(null); 
it.hasNext();) {
+      OzoneKey key = it.next();
+//    TODO: Remove this check once HDDS-12094 is fixed
+      if (key.getName().endsWith("/")) {
+        continue;
+      }
+      processKey(ozoneClient, volumeName, bucketName, key.getName());
+    }
+  }
+
+  private void processKey(OzoneClient client, String volumeName, String 
bucketName, String keyName) throws IOException {
+    System.out.println("Processing key : " + volumeName + "/" + bucketName + 
"/" + keyName);
+    boolean isChecksumVerifyEnabled
+        = getConf().getBoolean("ozone.client.verify.checksum", true);
+    OzoneConfiguration configuration = new OzoneConfiguration(getConf());
+    configuration.setBoolean("ozone.client.verify.checksum",
+        !isChecksumVerifyEnabled);
+    RpcClient newClient = new RpcClient(configuration, null);

Review Comment:
   This method processKey() is going to be called multiple times for all keys 
in a bucket.
   The RpcClient object can be reused for multiple keys. I'd also suggest move 
the instantiation of OzoneConfiguration to be only one-time overall.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java:
##########
@@ -57,11 +64,12 @@
  * given key. It also generates a manifest file with information about the
  * downloaded replicas.
  */
[email protected](name = "read-replicas",
[email protected](name = "checksums",
     description = "Reads every replica for all the blocks associated with a " +
-        "given key.")
+        "given key.",
+    aliases = {"read-replicas"})
 @MetaInfServices(DebugSubcommand.class)
-public class ReadReplicas extends KeyHandler implements DebugSubcommand {
+public class ReadReplicas extends Handler implements DebugSubcommand {

Review Comment:
   Suggest to rename this class/file name as well to match the command name.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java:
##########
@@ -246,4 +204,91 @@ private File createDirectory(String volumeName, String 
bucketName,
     }
     return dir;
   }
+
+  private void findCandidateKeys(OzoneClient ozoneClient, OzoneAddress 
address) throws IOException {
+    ObjectStore objectStore = ozoneClient.getObjectStore();
+    String volumeName = address.getVolumeName();
+    String bucketName = address.getBucketName();
+    String keyName = address.getKeyName();
+    if (!keyName.isEmpty()) {
+      processKey(ozoneClient, volumeName, bucketName, keyName);
+    } else if (!bucketName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      OzoneBucket bucket = volume.getBucket(bucketName);
+      checkBucket(bucket, ozoneClient);
+    } else if (!volumeName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      checkVolume(volume, ozoneClient);
+    } else {
+      for (Iterator<? extends OzoneVolume> it = objectStore.listVolumes(null); 
it.hasNext();) {
+        checkVolume(it.next(), ozoneClient);
+      }
+    }
+  }
+
+  private void checkVolume(OzoneVolume volume, OzoneClient ozoneClient) throws 
IOException {
+    for (Iterator<? extends OzoneBucket> it = volume.listBuckets(null); 
it.hasNext();) {
+      OzoneBucket bucket = it.next();
+      checkBucket(bucket, ozoneClient);
+    }
+  }
+
+  private void checkBucket(OzoneBucket bucket, OzoneClient ozoneClient) throws 
IOException {
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+    for (Iterator<? extends OzoneKey> it = bucket.listKeys(null); 
it.hasNext();) {
+      OzoneKey key = it.next();
+//    TODO: Remove this check once HDDS-12094 is fixed
+      if (key.getName().endsWith("/")) {
+        continue;
+      }
+      processKey(ozoneClient, volumeName, bucketName, key.getName());
+    }
+  }
+
+  private void processKey(OzoneClient client, String volumeName, String 
bucketName, String keyName) throws IOException {
+    System.out.println("Processing key : " + volumeName + "/" + bucketName + 
"/" + keyName);
+    boolean isChecksumVerifyEnabled
+        = getConf().getBoolean("ozone.client.verify.checksum", true);
+    OzoneConfiguration configuration = new OzoneConfiguration(getConf());
+    configuration.setBoolean("ozone.client.verify.checksum",
+        !isChecksumVerifyEnabled);
+    RpcClient newClient = new RpcClient(configuration, null);

Review Comment:
   also worth noting that the command will process one key at a time with no 
parallelism so could take a long time for a big bucket.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java:
##########
@@ -246,4 +204,91 @@ private File createDirectory(String volumeName, String 
bucketName,
     }
     return dir;
   }
+
+  private void findCandidateKeys(OzoneClient ozoneClient, OzoneAddress 
address) throws IOException {
+    ObjectStore objectStore = ozoneClient.getObjectStore();
+    String volumeName = address.getVolumeName();
+    String bucketName = address.getBucketName();
+    String keyName = address.getKeyName();
+    if (!keyName.isEmpty()) {
+      processKey(ozoneClient, volumeName, bucketName, keyName);
+    } else if (!bucketName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      OzoneBucket bucket = volume.getBucket(bucketName);
+      checkBucket(bucket, ozoneClient);
+    } else if (!volumeName.isEmpty()) {
+      OzoneVolume volume = objectStore.getVolume(volumeName);
+      checkVolume(volume, ozoneClient);
+    } else {
+      for (Iterator<? extends OzoneVolume> it = objectStore.listVolumes(null); 
it.hasNext();) {
+        checkVolume(it.next(), ozoneClient);
+      }
+    }
+  }
+
+  private void checkVolume(OzoneVolume volume, OzoneClient ozoneClient) throws 
IOException {
+    for (Iterator<? extends OzoneBucket> it = volume.listBuckets(null); 
it.hasNext();) {
+      OzoneBucket bucket = it.next();
+      checkBucket(bucket, ozoneClient);
+    }
+  }
+
+  private void checkBucket(OzoneBucket bucket, OzoneClient ozoneClient) throws 
IOException {
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+    for (Iterator<? extends OzoneKey> it = bucket.listKeys(null); 
it.hasNext();) {
+      OzoneKey key = it.next();
+//    TODO: Remove this check once HDDS-12094 is fixed
+      if (key.getName().endsWith("/")) {
+        continue;
+      }
+      processKey(ozoneClient, volumeName, bucketName, key.getName());
+    }
+  }
+
+  private void processKey(OzoneClient client, String volumeName, String 
bucketName, String keyName) throws IOException {
+    System.out.println("Processing key : " + volumeName + "/" + bucketName + 
"/" + keyName);
+    boolean isChecksumVerifyEnabled
+        = getConf().getBoolean("ozone.client.verify.checksum", true);
+    OzoneConfiguration configuration = new OzoneConfiguration(getConf());
+    configuration.setBoolean("ozone.client.verify.checksum",
+        !isChecksumVerifyEnabled);
+    RpcClient newClient = new RpcClient(configuration, null);
+
+    try {
+      ClientProtocol noChecksumClient;
+      ClientProtocol checksumClient;
+      if (isChecksumVerifyEnabled) {
+        checksumClient = client.getObjectStore().getClientProxy();
+        noChecksumClient = newClient;
+      } else {
+        checksumClient = newClient;
+        noChecksumClient = client.getObjectStore().getClientProxy();
+      }
+
+      File dir = createDirectory(volumeName, bucketName, keyName);
+      OzoneKeyDetails keyInfoDetails = 
checksumClient.getKeyDetails(volumeName, bucketName, keyName);
+      Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>> replicas =
+          checksumClient.getKeysEveryReplicas(volumeName, bucketName, keyName);

Review Comment:
   Not totally related, but RpcClient.getKeysEveryReplicas() doesn't seems to 
create input stream that refreshes container cache upon failure. Could that 
lead problems in such a corner case?
   
   
https://github.com/apache/ozone/blob/master/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L1609



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to