elek commented on a change in pull request #1451:
URL: https://github.com/apache/hadoop-ozone/pull/1451#discussion_r502352349
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -919,12 +920,32 @@ private boolean isKeyEmpty(OmKeyInfo keyInfo) {
// underlying table using an iterator. That automatically creates a
// snapshot of the data, so we don't need these locks at a higher level
// when we iterate.
+
+ startKey = normalizeListKeyPath(startKey);
+ keyPrefix = normalizeListKeyPath(keyPrefix);
Review comment:
Sorry, I don't get it. Why do we need to check it _inside
normalizeListKeyPath_.
As I wrote it's a very minor thing, but it seems to be better to move out
the check from the method because it improves the readability (IMHO!).
When somebody read the `listKeys` method it suggest the the keys are
normalized. but in fact it's normalized only if enableFileSystemPaths. This can
be confusing as the method name is `normalizeListKeyPath` and not something
like `normalizeListKeyPathIfNormalizationIsEnabled`.
I suggested moving out this condition from this method to improve the
readibility, but if you think it's a bad idea, it can be ignored.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
