[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3271: Performs some minor cleanup of TabletLocator

["ctubbsii (via GitHub)" <gi...@apache.org>]

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


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/RootTabletLocator.java:
##########
@@ -56,11 +56,12 @@ public <T extends Mutation> void binMutations(ClientContext context, List<T> mut
       Map<String,TabletServerMutations<T>> binnedMutations, List<T> failures) {
     TabletLocation rootTabletLocation = getRootTabletLocation(context);
     if (rootTabletLocation != null) {
-      TabletServerMutations<T> tsm = new TabletServerMutations<>(rootTabletLocation.tablet_session);
+      TabletServerMutations<T> tsm =
+          new TabletServerMutations<>(rootTabletLocation.getTserverSession());

Review Comment:
   I'm not sure if this fits on one line after this suggestion or not, but this would be a good use of the `var` keyword, to avoid the redundant long class name in the variable declaration.
   
   ```suggestion
         var tsm = new TabletServerMutations<T>(rootTabletLocation.getTserverSession());
   ```



##########
server/base/src/test/java/org/apache/accumulo/server/client/BulkImporterTest.java:
##########
@@ -147,25 +148,25 @@ public void testFindOverlappingTablets() throws Exception {
       List<TabletLocation> overlaps =
           BulkImporter.findOverlappingTablets(context, vm, locator, new Path(file), null, null, cs);
       assertEquals(5, overlaps.size());
-      Collections.sort(overlaps);
-      assertEquals(new KeyExtent(tableId, new Text("a"), null), overlaps.get(0).tablet_extent);
+      Collections.sort(overlaps, Comparator.comparing(tl -> tl.getExtent()));

Review Comment:
   The previous Comparable compared by extent, then by location, then by session. A few questions:
   
   1. Did those secondary sorts not really matter?
   2. Did I miss something, or is this test the only time we care about these being ordered? Was that Comparable stuff only there for this test?



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