swamirishi commented on code in PR #4387:
URL: https://github.com/apache/ozone/pull/4387#discussion_r1138937832
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java:
##########
@@ -91,64 +93,72 @@ public Class<?> getParentType() {
protected void execute(OzoneClient client, OzoneAddress address)
throws IOException, OzoneClientException {
+ address.ensureKeyAddress();
+
boolean isChecksumVerifyEnabled
= getConf().getBoolean("ozone.client.verify.checksum", true);
OzoneConfiguration configuration = new OzoneConfiguration(getConf());
configuration.setBoolean("ozone.client.verify.checksum",
!isChecksumVerifyEnabled);
- if (isChecksumVerifyEnabled) {
- clientProtocol = client.getObjectStore().getClientProxy();
- clientProtocolWithoutChecksum = new RpcClient(configuration, null);
- } else {
- clientProtocol = new RpcClient(configuration, null);
- clientProtocolWithoutChecksum = client.getObjectStore().getClientProxy();
- }
+ 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();
+ }
- address.ensureKeyAddress();
- String volumeName = address.getVolumeName();
- String bucketName = address.getBucketName();
- String keyName = address.getKeyName();
+ String volumeName = address.getVolumeName();
+ String bucketName = address.getBucketName();
+ String keyName = address.getKeyName();
- String directoryName = createDirectory(volumeName, bucketName, keyName);
+ File dir = createDirectory(volumeName, bucketName, keyName);
- OzoneKeyDetails keyInfoDetails
- = clientProtocol.getKeyDetails(volumeName, bucketName, keyName);
+ OzoneKeyDetails keyInfoDetails
+ = checksumClient.getKeyDetails(volumeName, bucketName, keyName);
- Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>> replicas
- = clientProtocol.getKeysEveryReplicas(volumeName, bucketName, keyName);
+ Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>> replicas =
+ checksumClient.getKeysEveryReplicas(volumeName, bucketName, keyName);
- Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>>
- replicasWithoutChecksum = clientProtocolWithoutChecksum
- .getKeysEveryReplicas(volumeName, bucketName, keyName);
+ Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>>
+ replicasWithoutChecksum = noChecksumClient
+ .getKeysEveryReplicas(volumeName, bucketName, keyName);
- JsonObject result = new JsonObject();
- result.addProperty(JSON_PROPERTY_FILE_NAME,
- volumeName + "/" + bucketName + "/" + keyName);
- result.addProperty(JSON_PROPERTY_FILE_SIZE, keyInfoDetails.getDataSize());
+ JsonObject result = new JsonObject();
+ result.addProperty(JSON_PROPERTY_FILE_NAME,
+ volumeName + "/" + bucketName + "/" + keyName);
+ result.addProperty(JSON_PROPERTY_FILE_SIZE,
keyInfoDetails.getDataSize());
- JsonArray blocks = new JsonArray();
- downloadReplicasAndCreateManifest(keyName, replicas,
- replicasWithoutChecksum, directoryName, blocks);
- result.add(JSON_PROPERTY_FILE_BLOCKS, blocks);
+ JsonArray blocks = new JsonArray();
+ downloadReplicasAndCreateManifest(keyName, replicas,
+ replicasWithoutChecksum, dir, blocks);
+ result.add(JSON_PROPERTY_FILE_BLOCKS, blocks);
- Gson gson = new GsonBuilder().setPrettyPrinting().create();
- String prettyJson = gson.toJson(result);
+ Gson gson = new GsonBuilder().setPrettyPrinting().create();
+ String prettyJson = gson.toJson(result);
- String manifestFileName = keyName + "_manifest";
- System.out.println("Writing manifest file : " + manifestFileName);
- File manifestFile
- = new File(outputDir + "/" + directoryName + "/" + manifestFileName);
- Files.write(manifestFile.toPath(),
- prettyJson.getBytes(StandardCharsets.UTF_8));
+ String manifestFileName = keyName + "_manifest";
+ System.out.println("Writing manifest file : " + manifestFileName);
+ File manifestFile
+ = new File(dir, manifestFileName);
+ Files.write(manifestFile.toPath(),
+ prettyJson.getBytes(StandardCharsets.UTF_8));
+ } finally {
+ newClient.close();
Review Comment:
Oh I meant if the newClient variable is reassigned another value due to some
other change, it might still lead to the initial value newClient not being
closed. In this case try(newClient) would be better & would ensure we are
closing the stream we opened.
--
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]
