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]