HDDS-9944. NSSummary commands should close OzoneClient.

[devmadhuu]

What changes were proposed in this pull request?

This PR fixes the Ozone Client connection leak by closing all opened client connections in NSSummaryAdmin.java

NSSummaryAdmin commands leaves the Ozone client connections open which is not a good practice and may consume resources. So. this PR fixes that by closing all opened Ozone client connections.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-9944

How was this patch tested?

This patch was tested using existing integration test org.apache.hadoop.ozone.shell.TestNSSummaryAdmin

[devmadhuu]

@adoroszlai Pls review.

[adoroszlai]

Thanks @devmadhuu for working on this.

There is a second item in the task:

it should reuse client (or even bucket after lookup) for all checks

All namespace subcommands use these methods as the following:

ozone/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/nssummary/DiskUsageSubCommand.java

Lines 109 to 112 in 2616f8e

	if (parent.isObjectStoreBucket(path) ||
	!parent.bucketIsPresentInThePath(path)) {
	printBucketReminder();
	}

I don't think we need to use two separate clients for checking properties of the same bucket. The two methods can be combined.

Also, isFileSystemOptimizedBucket seems to be unused.

[devmadhuu]

Thanks @devmadhuu for working on this.

There is a second item in the task:

it should reuse client (or even bucket after lookup) for all checks

All namespace subcommands use these methods as the following:

ozone/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/nssummary/DiskUsageSubCommand.java

Lines 109 to 112 in 2616f8e

	if (parent.isObjectStoreBucket(path) ||
	!parent.bucketIsPresentInThePath(path)) {
	printBucketReminder();
	}

I don't think we need to use two separate clients for checking properties of the same bucket. The two methods can be combined.

Also, isFileSystemOptimizedBucket seems to be unused.

Thanks @adoroszlai for your review. I have implemented 2nd part of this JIRA :
--- NSSummaryAdmin all sub-commands should reuse ozone client (or even bucket after lookup) for all checks

[adoroszlai]

Thanks @devmadhuu for updating the patch with re-use of client. Some minor items suggested.

[adoroszlai]

Thanks @devmadhuu for updating the patch.

[adoroszlai]

@ArafatKhan2198 please review

[xBis7]

@devmadhuu Thanks for the patch. I've tested it locally. LGTM!

[xBis7]

I can see that for every command, this check is preceded by a check to whether the path exists. Therefore, there is no issue of a NPException.

[ArafatKhan2198]

Thank you for the modifications, @devmadhuu. I was just curious about why we are directly utilizing println() instead of employing logging. I believe even for a CLI command it usually does log the message on the console right?

System.out.println(
    "Bucket layout couldn't be verified for path: " + ofsPath +
        ". Exception: " + e);

[adoroszlai]

Thanks @devmadhuu for the fix, @ArafatKhan2198, @xBis7 for the review.

[devmadhuu]

Thank you for the modifications, @devmadhuu. I was just curious about why we are directly utilizing println() instead of employing logging. I believe even for a CLI command it usually does log the message on the console right?

System.out.println(
    "Bucket layout couldn't be verified for path: " + ofsPath +
        ". Exception: " + e);

Thanks @ArafatKhan2198 for review. These are CLI based interface, so need to print on console.