Tejaskriya commented on code in PR #8898:
URL: https://github.com/apache/ozone/pull/8898#discussion_r2265951697
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java:
##########
@@ -72,22 +83,70 @@ public class ReplicasVerify extends Handler {
private List<ReplicaVerifier> replicaVerifiers;
+ private static final String DURATION_FORMAT = "HH:mm:ss,SSS";
+ private long startTime;
+ private long endTime;
+ private String verificationScope;
+ private final List<String> verificationTypes = new ArrayList<>();
+ private final AtomicInteger volumesProcessed = new AtomicInteger(0);
+ private final AtomicInteger bucketsProcessed = new AtomicInteger(0);
+ private final AtomicInteger keysProcessed = new AtomicInteger(0);
+ private final AtomicInteger keysPassed = new AtomicInteger(0);
+ private final AtomicInteger keysFailed = new AtomicInteger(0);
+ private final Map<String, AtomicInteger> failuresByType = new
ConcurrentHashMap<>();
+ private volatile Throwable exception;
+
@Override
protected void execute(OzoneClient client, OzoneAddress address) throws
IOException {
+ startTime = System.nanoTime();
+
+ if (!address.getKeyName().isEmpty()) {
+ verificationScope = "Key";
+ } else if (!address.getBucketName().isEmpty()) {
+ verificationScope = "Bucket";
+ } else if (!address.getVolumeName().isEmpty()) {
+ verificationScope = "Volume";
+ } else {
+ verificationScope = "All Volumes";
+ }
+
replicaVerifiers = new ArrayList<>();
if (verification.doExecuteChecksums) {
- replicaVerifiers.add(new ChecksumVerifier(getConf()));
+ ChecksumVerifier checksumVerifier = new ChecksumVerifier(getConf());
+ replicaVerifiers.add(checksumVerifier);
+ String checksumType = checksumVerifier.getType();
+ verificationTypes.add(checksumType);
+ failuresByType.put(checksumType, new AtomicInteger(0));
}
if (verification.doExecuteBlockExistence) {
- replicaVerifiers.add(new BlockExistenceVerifier(getConf()));
+ BlockExistenceVerifier blockVerifier = new
BlockExistenceVerifier(getConf());
+ replicaVerifiers.add(blockVerifier);
+ String blockType = blockVerifier.getType();
+ verificationTypes.add(blockType);
+ failuresByType.put(blockType, new AtomicInteger(0));
}
+
if (verification.doExecuteReplicaState) {
- replicaVerifiers.add(new ContainerStateVerifier(getConf(),
containerCacheSize));
+ ContainerStateVerifier stateVerifier = new
ContainerStateVerifier(getConf(), containerCacheSize);
+ replicaVerifiers.add(stateVerifier);
+ String stateType = stateVerifier.getType();
+ verificationTypes.add(stateType);
+ failuresByType.put(stateType, new AtomicInteger(0));
}
- findCandidateKeys(client, address);
+ // Add shutdown hook to ensure summary is printed even if interrupted
+ addShutdownHook();
+
+ try {
+ findCandidateKeys(client, address);
+ endTime = System.nanoTime();
+ } catch (Exception e) {
+ exception = e;
+ endTime = System.nanoTime();
+ throw e;
+ }
Review Comment:
The capturing of endTime can be moved to a finally block.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java:
##########
@@ -207,15 +271,95 @@ void processKey(OzoneClient ozoneClient, String
volumeName, String bucketName, S
}
keyNode.put("pass", keyPass);
- if (!keyPass) {
+ if (keyPass) {
+ keysPassed.incrementAndGet();
+ } else {
+ keysFailed.incrementAndGet();
allKeysPassed.set(false);
+
+ for (String failedType : failedVerificationTypes) {
+ AtomicInteger counter = failuresByType.get(failedType);
+ if (counter != null) {
+ counter.incrementAndGet();
+ } else {
+ failuresByType.computeIfAbsent(failedType, k -> new
AtomicInteger(0)).incrementAndGet();
+ }
+ }
}
if (!keyPass || allResults) {
keysArray.add(keyNode);
}
}
+ /**
+ * Adds ShutdownHook to print summary statistics.
+ */
+ private void addShutdownHook() {
+ ShutdownHookManager.get().addShutdownHook(() -> {
+ if (endTime == 0) {
+ endTime = System.nanoTime();
+ }
+ printSummary(System.out);
Review Comment:
+1 to this
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java:
##########
@@ -207,15 +271,95 @@ void processKey(OzoneClient ozoneClient, String
volumeName, String bucketName, S
}
keyNode.put("pass", keyPass);
- if (!keyPass) {
+ if (keyPass) {
+ keysPassed.incrementAndGet();
+ } else {
+ keysFailed.incrementAndGet();
allKeysPassed.set(false);
+
+ for (String failedType : failedVerificationTypes) {
+ AtomicInteger counter = failuresByType.get(failedType);
+ if (counter != null) {
+ counter.incrementAndGet();
+ } else {
+ failuresByType.computeIfAbsent(failedType, k -> new
AtomicInteger(0)).incrementAndGet();
+ }
+ }
Review Comment:
The null check is not needed as computIfAbsent will handle that case,
```suggestion
failedVerificationTypes.forEach(failedType -> failuresByType
.computeIfAbsent(failedType, k -> new AtomicInteger(0))
.incrementAndGet()
);
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java:
##########
@@ -72,22 +83,70 @@ public class ReplicasVerify extends Handler {
private List<ReplicaVerifier> replicaVerifiers;
+ private static final String DURATION_FORMAT = "HH:mm:ss,SSS";
+ private long startTime;
+ private long endTime;
+ private String verificationScope;
+ private final List<String> verificationTypes = new ArrayList<>();
+ private final AtomicInteger volumesProcessed = new AtomicInteger(0);
+ private final AtomicInteger bucketsProcessed = new AtomicInteger(0);
+ private final AtomicInteger keysProcessed = new AtomicInteger(0);
+ private final AtomicInteger keysPassed = new AtomicInteger(0);
+ private final AtomicInteger keysFailed = new AtomicInteger(0);
+ private final Map<String, AtomicInteger> failuresByType = new
ConcurrentHashMap<>();
+ private volatile Throwable exception;
+
@Override
protected void execute(OzoneClient client, OzoneAddress address) throws
IOException {
+ startTime = System.nanoTime();
+
+ if (!address.getKeyName().isEmpty()) {
+ verificationScope = "Key";
+ } else if (!address.getBucketName().isEmpty()) {
+ verificationScope = "Bucket";
+ } else if (!address.getVolumeName().isEmpty()) {
+ verificationScope = "Volume";
+ } else {
+ verificationScope = "All Volumes";
+ }
+
replicaVerifiers = new ArrayList<>();
if (verification.doExecuteChecksums) {
- replicaVerifiers.add(new ChecksumVerifier(getConf()));
+ ChecksumVerifier checksumVerifier = new ChecksumVerifier(getConf());
+ replicaVerifiers.add(checksumVerifier);
+ String checksumType = checksumVerifier.getType();
+ verificationTypes.add(checksumType);
+ failuresByType.put(checksumType, new AtomicInteger(0));
Review Comment:
Code similar to this is repeated a few times, we can extract this code out
to a method, something like below:
```
// introduce below method
private void addVerifier(boolean condition, Supplier<ReplicaVerifier>
verifierSupplier) {
if flag is true:
then perform the required steps
}
// call the method as below:
addVerifier(verification.doExecuteChecksums, () -> new
ChecksumVerifier(getConf()));
```
--
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]
