ivakegg commented on code in PR #3139:
URL: https://github.com/apache/accumulo/pull/3139#discussion_r1058376265


##########
server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java:
##########
@@ -319,21 +324,30 @@ private static void updateRootTabletDataFile(KeyExtent 
extent, FileRef path, Fil
    *
    * @return A Mutation to update a tablet from the given information
    */
-  private static Mutation getUpdateForTabletDataFile(KeyExtent extent, FileRef 
path,
-      FileRef mergeFile, DataFileValue dfv, String time, Set<FileRef> 
filesInUseByScans,
-      String address, ZooLock zooLock, Set<String> unusedWalLogs, 
TServerInstance lastLocation,
-      long flushId) {
+  private static Mutation getUpdateForTabletDataFile(ClientContext context, 
KeyExtent extent,
+      FileRef path, FileRef mergeFile, DataFileValue dfv, String time,
+      Set<FileRef> filesInUseByScans, String address, ZooLock zooLock, 
Set<String> unusedWalLogs,
+      TServerInstance lastLocation, long flushId) {
     Mutation m = new Mutation(extent.getMetadataEntry());
 
     if (dfv.getNumEntries() > 0) {
       m.put(DataFileColumnFamily.NAME, path.meta(), new Value(dfv.encode()));
       TabletsSection.ServerColumnFamily.TIME_COLUMN.put(m, new 
Value(time.getBytes(UTF_8)));
-      // stuff in this location
-      TServerInstance self = getTServerInstance(address, zooLock);
-      self.putLastLocation(m);
-      // erase the old location
-      if (lastLocation != null && !lastLocation.equals(self))
-        lastLocation.clearLastLocation(m);
+
+      // if the location mode is 'locality'', then preserve the current 
compaction location in the
+      // last location value
+      if 
("locality".equals(context.getConfiguration().get(Property.TABLE_LOCATION_MODE)))
 {
+        // stuff in this location
+        TServerInstance self = getTServerInstance(address, zooLock);
+        self.putLastLocation(m);
+        // erase the old location
+        if (lastLocation != null && !lastLocation.equals(self))
+          // @TODO: this does not make any sense to me as this will simply 
clear out the location we
+          // just set a couple lines previously
+          // clearLastLocation will clear the "last" location no matter what 
the value is
+          lastLocation.clearLastLocation(m);

Review Comment:
   well crud.  I did not read the code correctly.  thanks.



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