ctubbsii commented on code in PR #3501:
URL: https://github.com/apache/accumulo/pull/3501#discussion_r1231552393


##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -506,6 +518,12 @@ public static <E extends Entry<Key,Value>> TabletMetadata 
convertRow(Iterator<E>
     if (buildKeyValueMap) {
       te.keyValues = kvBuilder.build();
     }
+    te.filesSupplier = Suppliers.memoize(() -> {
+      ImmutableMap.Builder<StoredTabletFile,DataFileValue> fb = 
ImmutableMap.builder();
+      te.rawFiles.forEach(pair -> fb.put(new StoredTabletFile(pair.getFirst()),
+          new DataFileValue((pair.getSecond()))));
+      return fb.build();

Review Comment:
   For what it's worth, I think the other code could change, too. I seem to 
remember there was some resistance to getting rid of `ImmutableX.builder()` 
code when I last did a pass through the code to replace our use of immutable 
Guava types with builtin immutables. The builders are nice. Even if we keep 
them, I think they could be expressed as a custom stream Collector. But that 
change would be out of scope of this PR.
   
   One other consideration we have to be careful about is checking for null 
values `.contains(null)` fails on built-in Java immutables, like that returned 
from `List.of()`, `Set.of()`, etc., instead of just always returning false, 
which would make more sense. I don't think Guava has the same issue. And, I 
know `Collections.unmodifiableX()` does not suffer from the same problem.



-- 
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]

Reply via email to