dlmarion commented on code in PR #4072:
URL: https://github.com/apache/accumulo/pull/4072#discussion_r1426675765


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1302,63 +1304,77 @@ public Optional<StoredTabletFile> 
updateTabletDataFile(long maxCommittedTime,
     // Read these once in case of buggy race conditions will get consistent 
logging. If all other
     // code is locking properly these should not change during this method.
     var lastTabletMetadata = getMetadata();
-    var expectedTime = lastTabletMetadata.getTime();
-
-    // Expect time to only move forward from what was recently seen in 
metadata table.
-    Preconditions.checkArgument(maxCommittedTime >= expectedTime.getTime());
-
-    // The tablet time is used to determine if the write succeeded, in order 
to do this the tablet
-    // time needs to be different from what is currently stored in the 
metadata table.
-    while (maxCommittedTime == expectedTime.getTime()) {
-      var nextTime = tabletTime.getAndUpdateTime();
-      Preconditions.checkState(nextTime >= maxCommittedTime);
-      if (nextTime > maxCommittedTime) {
-        maxCommittedTime++;
-      }
-    }
-
-    try (var tabletsMutator = 
getContext().getAmple().conditionallyMutateTablets()) {
 
-      var expectedLocation = mincReason == MinorCompactionReason.RECOVERY
-          ? Location.future(tabletServer.getTabletSession())
-          : Location.current(tabletServer.getTabletSession());
+    while (true) {
+      try (var tabletsMutator = 
getContext().getAmple().conditionallyMutateTablets()) {
 
-      var tablet = 
tabletsMutator.mutateTablet(extent).requireLocation(expectedLocation)
-          .requireSame(lastTabletMetadata, ColumnType.TIME);
+        var expectedLocation = mincReason == MinorCompactionReason.RECOVERY
+            ? Location.future(tabletServer.getTabletSession())
+            : Location.current(tabletServer.getTabletSession());
 
-      Optional<StoredTabletFile> newFile = Optional.empty();
+        var tablet = 
tabletsMutator.mutateTablet(extent).requireLocation(expectedLocation);
 
-      // if entries are present, write to path to metadata table
-      if (dfv.getNumEntries() > 0) {
-        tablet.putFile(newDatafile, dfv);
-        newFile = Optional.of(newDatafile.insert());
-      }
+        Optional<StoredTabletFile> newFile = Optional.empty();
 
-      var newTime = tabletTime.getMetadataTime(maxCommittedTime);
-      tablet.putTime(newTime);
+        // if entries are present, write to path to metadata table
+        if (dfv.getNumEntries() > 0) {
+          tablet.putFile(newDatafile, dfv);
+          newFile = Optional.of(newDatafile.insert());
+        }
 
-      tablet.putFlushId(flushId);
+        boolean setTime = false;
+        // bulk imports can also update time in the metadata table, so only 
update if we are moving
+        // time forward
+        if (maxCommittedTime > lastTabletMetadata.getTime().getTime()) {
+          // TODO throw exception if requireSame called more than once

Review Comment:
   Should this `TODO` be an `ELASTICITY_TODO`?



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