[GitHub] [accumulo] EdColeman commented on a diff in pull request #2854: Update TabletFile methods

[GitBox <gi...@apache.org>]

EdColeman commented on code in PR #2854:
URL: https://github.com/apache/accumulo/pull/2854#discussion_r940621599


##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -128,24 +128,42 @@ public Path getPath() {
     return metaPath;
   }
 
+  /**
+   * The ordering of this class depends on the normal String ordering of the normalized paths first,
+   * then any original path, if one exists as a StoredTabletFile, second. To ensure subclasses don't
+   * break things, and to preserve commutativity, subclasses should defer to this implementation.
+   */

Review Comment:
   The addition of using StoredTabletFile original path in the comparison breaks the implied contract that
   
   `(x.compareTo(y) == 0 ) ==> (x.equals(y))`
   
    Collections that use hashCode and equals (HashSet / Map) will behave differently than collections that use compareTo (TreeSet / Map)  This is similar to the way `BigDecimal` handles `compareTo` and `equals` But is is unusual.
   
   This seems that it could introduce issues. If somehow there were two metadata entries that had different "original" paths but resolved to the same normalized path and if they were put into a HashSet and a TreeSet the collections will have different contents.
   
   `
   For example:
   
   If you have two StoredTabletFiles:
   
       StoredTabletFile file1 = new StoredTabletFile(
               "hdfs://nn.somewhere.com:86753/accumulo/tables/" + id + "/" + dir + "/" + filename);
   
       StoredTabletFile file2 = new StoredTabletFile(
               "hdfs://nn.somewhere.com:86753/./accumulo/tables/" + id + "/" + dir + "/" + filename);
   
   And stored them in a TreeSet and a HashSet you end up with different collections (both show the "original name(s)" in the collection:
   
   TreeSet - 2 entries
      hdfs://nn.somewhere.com:86753/./accumulo/tables/2a/t-0003/C0004.rf
      hdfs://nn.somewhere.com:86753/accumulo/tables/2a/t-0003/C0004.rf
   
   HashSet - 1 entry
      hdfs://nn.somewhere.com:86753/accumulo/tables/2a/t-0003/C0004.rf
   
   `



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org