HDDS-16059. Optimize varargs in IOzoneManagerLock.

[szetszwo]

What changes were proposed in this pull request?

A standard optimization is to override the methods with fixed parameter number. For example, see the info(..) methods in https://www.slf4j.org/apidocs/org/slf4j/Logger.html

In IOzoneManagerLock, since most cases only have one or two keys, we will override it with one parameter and two parameters.

What is the link to the Apache JIRA

HDDS-16059

How was this patch tested?

By updating existing tests.

[spacemonkd]

Wont this already throw NPE if first or second is null?
We should move the Object.requreNonNull() checks above this.

[szetszwo]

@spacemonkd , It would be easier to understand if you could quote also line 42 and 43.

Or, if you showed below instead.

+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/CompositeKey.java
@@ -38,9 +38,9 @@ private static final class TwoComponents extends CompositeKey {
     private final Object second;

     private TwoComponents(Object first, Object second) {
-      this.hashCode = hash(hash(1, first), second);
       this.first = Objects.requireNonNull(first, "first == null");
       this.second = Objects.requireNonNull(second, "second == null");
+      this.hashCode = hash(hash(1, first), second);
     }

[spacemonkd]

Thanks for this patch @szetszwo.
Just have some nits, but overall the patch looks good to me!

[szetszwo]

@spacemonkd , thanks a lot for reviewing this!

For the documentation suggestions, I mostly disagree. Also, bad documentation (in the existing code) is much worse than no documentation. Let's focus on the code but not spend too much energy on the javadoc; see https://en.wikipedia.org/wiki/Law_of_triviality 😀

[spacemonkd]

Thanks a lot @szetszwo for addressing the comments and the interesting article on the Bike-shed problem.
Overall looks good to me. But I see two comments still open - I would love to know your inputs on them.
#10919 (comment)
#10919 (comment)

[szetszwo]

... I would love to know your inputs on them. ...

@spacemonkd , do you agree that these two comments are still bike-shedding? What are the difference for doing them either way? Are you nitpicking everyone's PR in such a great details?

[spacemonkd]

do you agree that these two comments are still bike-shedding?

I feel only #10919 (comment) this might be important.
But the other one we can ignore, I guess yeah that is bike shedding as well

[spacemonkd]

Thanks @szetszwo this looks good to me!

[szetszwo]

I feel only #10919 (comment) this might be important.

Okay, I will change it. That is indeed a better comment, although it neither affect the correctness nor the performance.

[spacemonkd]

Thanks for the patch @szetszwo ! Merging this.

[szetszwo]

@spacemonkd , Sorry that I might have lost my patient since I had received a review with 10+ comments, where some of them were incorrect and the other were nits. I did spend unexpectedly long time to deal those comments.

I hope you understand that everyone and the community as a whole have a very limited time and energy. As described in https://en.wikipedia.org/wiki/Law_of_triviality , if we focus on the nits, nothing can be done.

[spacemonkd]

Yup yup, no worries. It is understandable. Thanks again for the inputs and the patch @szetszwo