re apache#3724: Tablet transaction log

* Reverted to just logging

core/src/main/java/org/apache/accumulo/core/conf/Property.java

@@ -1193,18 +1193,8 @@ public enum Property {
   TABLE_BLOCKCACHE_ENABLED("table.cache.block.enable", "false", PropertyType.BOOLEAN,
       "Determines whether data block cache is enabled for a table.", "1.3.5"),
   TABLE_OPERATION_LOG_MAX_SIZE("table.operation.log.max.size", "0", PropertyType.COUNT,
-      "The maximum number of logged operations logged in memory for diagnostic purposes. "
-          + "The log will can be used for diagnosing if and when in-memory file list diverges from metadata.",
-      "2.1.3"),
-  TABLE_OPERATION_LOG_RECOVERY("table.operation.log.recovery.action", LogSync.log.name(),
-      PropertyType.DATAFILE_RECOVERY_ACTION,
-      "The action to take when the in-memory file list is found to be different than the metadata.\n1)"
-          + LogSync.log.name() + ": simply log the operation log,\n2) " + LogSync.logsync.name()
-          + ": sync in-memory and metadata with the log depending on what the operation log agrees with,\n3) "
-          + LogSync.metasync.name()
-          + ": update memory with what the metadata holds if operation log agrees with metadata,\n4) "
-          + LogSync.memsync.name()
-          + ": update metadata with what in-memory holds if operation log agrees with memory.",
+      "The maximum number of operations logged in memory for diagnostic purposes when "
+          + "the in-memory file list diverges from the metadata table.",
       "2.1.3"),
   TABLE_ITERATOR_PREFIX("table.iterator.", null, PropertyType.PREFIX,
       "Properties in this category specify iterators that are applied at"

core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java

@@ -129,11 +129,6 @@ public enum PropertyType {
   GC_POST_ACTION("gc_post_action", in(true, null, "none", "flush", "compact"),
       "One of 'none', 'flush', or 'compact'."),
 
-  DATAFILE_RECOVERY_ACTION("datafile_recovery_action",
-      in(true, null, LogSync.log.name(), LogSync.logsync.name(), LogSync.metasync.name(),
-          LogSync.memsync.name()),
-      "One of '" + LogSync.log.name() + "', '" + LogSync.logsync.name() + "', '"
-          + LogSync.metasync.name() + "', '" + LogSync.memsync.name() + "'"),
   LAST_LOCATION_MODE("last_location_mode", in(true, null, "assignment", "compaction"),
       "Defines how to update the last location.  One of 'assignment', or 'compaction'."),
 

core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java

@@ -145,12 +145,6 @@ public void testTypeGC_POST_ACTION() {
     invalid("", "other");
   }
 
-  @Test
-  public void testTypeDATAFILE_RECOVERY_ACTION() {
-    valid(null, "log", "logsync", "metasync", "memsync");
-    invalid("", "other");
-  }
-
   @Test
   public void testTypeLAST_LOCATION_MODE() {
     valid(null, "compaction", "assignment");

server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java

@@ -206,19 +206,6 @@ public static void replaceDatafiles(ServerContext context, KeyExtent extent,
     tablet.mutate();
   }
 
-  public static void replaceDatafiles(ServerContext context, KeyExtent extent,
-      Set<StoredTabletFile> datafilesToDelete, Map<StoredTabletFile,DataFileValue> datafilesToAdd) {
-
-    context.getAmple().putGcCandidates(extent.tableId(), datafilesToDelete);
-
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-
-    datafilesToDelete.forEach(tablet::deleteFile);
-    datafilesToAdd.forEach(tablet::putFile);
-
-    tablet.mutate();
-  }
-
   /**
    * Update tablet file data from flush. Returns a StoredTabletFile if there are data entries.
    */

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java

@@ -21,7 +21,6 @@
 import static org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -36,20 +35,17 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
-import org.apache.accumulo.core.conf.LogSync;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.logging.TabletLogger;
 import org.apache.accumulo.core.metadata.StoredTabletFile;
 import org.apache.accumulo.core.metadata.TabletFile;
 import org.apache.accumulo.core.metadata.schema.DataFileValue;
 import org.apache.accumulo.core.metadata.schema.ExternalCompactionId;
-import org.apache.accumulo.core.metadata.schema.TabletMetadata;
 import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
 import org.apache.accumulo.core.trace.TraceUtil;
 import org.apache.accumulo.core.util.MapCounter;
 import org.apache.accumulo.core.util.Pair;
-import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.fs.VolumeManager;
 import org.apache.accumulo.server.replication.proto.Replication.Status;
 import org.apache.accumulo.server.util.ManagerMetadataUtil;
@@ -283,6 +279,7 @@ public Collection<StoredTabletFile> importMapFiles(long tid, Map<TabletFile,Data
           }
           datafileSizes.put(tpath.getKey(), tpath.getValue());
           tabletLog.bulkImported(tpath.getKey());
+          checkTransactionLog();
         }
 
         tablet.getTabletResources().importedMapFiles();
@@ -429,6 +426,7 @@ Optional<StoredTabletFile> bringMinorCompactionOnline(TabletFile tmpDatafile,
           datafileSizes.put(newFileStored, dfv);
         }
         tabletLog.flushed(newFile);
+        checkTransactionLog();
 
         tablet.flushComplete(flushId);
 
@@ -519,6 +517,7 @@ Optional<StoredTabletFile> bringMajorCompactionOnline(Set<StoredTabletFile> oldD
           // could be used by a follow on compaction in a multipass compaction
         }
         tabletLog.compacted(oldDatafiles, newFile);
+        checkTransactionLog();
 
         tablet.computeNumEntries();
 
@@ -584,121 +583,16 @@ public MetadataUpdateCount getUpdateCount() {
     return metadataUpdateCount.get();
   }
 
-  /**
-   * This is called when metadata file list is determined to be out of sync withe in-memory file
-   * list. Firstly the operation log is dumped. Then metadata is compared again with memory. If
-   * still out of sync, then recovery is performed dependent on the action: 1) log: nothing else is
-   * done 2) logsync: memory is synched with the metadata IFF the operation log agrees with one or
-   * the other 3) metasync: metadata files are reloaded into memory IFF the operation log agrees
-   * with metadata 4) memsync: metadata is reset with the memory files IFF the operation log agrees
-   * with memory
-   *
-   * @param context The server context
-   */
-  public void handleMetadataDiff(ServerContext context) {
-    LogSync action = LogSync.log;
-
-    try {
-      action = LogSync
-          .valueOf(tablet.getTableConfiguration().get(Property.TABLE_OPERATION_LOG_RECOVERY));
-    } catch (Exception e) {
-      log.error("Failed to parse property value {} into one of {}.  Defaulting to {}.",
-          tablet.getTableConfiguration().get(Property.TABLE_OPERATION_LOG_RECOVERY),
-          Arrays.asList(LogSync.values()), action);
-    }
-
-    synchronized (tablet) {
-      // always log the operation log regardless of the requested action
-      log.error("Operation log: {}", tabletLog.dumpLog());
-
-      // rescan for the tablet metadata
-      var tabletMeta =
-          context.getAmple().readTablet(tablet.getExtent(), TabletMetadata.ColumnType.FILES);
-      if (tabletMeta == null) {
-        log.error("Tablet {} not found in metadata", tablet.getExtent());
-      } else {
-        Set<StoredTabletFile> metadata = new HashSet<>(tabletMeta.getFiles());
-        Set<StoredTabletFile> memory = new HashSet<>(datafileSizes.keySet());
-        Set<StoredTabletFile> expected = tabletLog.getExpectedFiles();
-
-        // verify we are still out of sync
-        if (metadata.equals(memory)) {
-          log.debug("Metadata and in-memory file list are back in-sync: {}", metadata);
-          if (!expected.equals(memory)) {
-            log.error("Resetting operation log {} with metadata and memory {}", expected, memory);
-            tabletLog.reset(memory);
-          }
-        } else if (!action.equals(LogSync.log)) {
-          if (action.equals(LogSync.logsync)) {
-            // LogSync.logsync means that we will sync depending on which side the log agrees with
-            if (expected.equals(memory)) {
-              // we agree with memory, so sync the metadata with the memory
-              action = LogSync.memsync;
-            } else if (expected.equals(metadata)) {
-              // we agree with the metadata, so sync the memory with the metadata
-              action = LogSync.metasync;
-            } else {
-              // nothing agrees.... so nothing more
-              log.error(
-                  "Not syncing files because the operation log {}"
-                      + " does not agree with metadata {}  nor memory {}",
-                  expected, metadata, memory);
-            }
-          }
-
-          if (action.equals(LogSync.metasync)) {
-            // metasync means update memory with that of metadata
-            if (!expected.equals(metadata)) {
-              log.error(
-                  "Not synching memory because the operation log {} does not agree with metadata ",
-                  expected, metadata);
-            } else {
-              log.warn(
-                  "Synching memory with metadata because the operation log agrees with metadata {}",
-                  expected);
-              resetMemoryWithMetadata(tabletMeta.getFilesMap());
-            }
-          } else if (action.equals(LogSync.memsync)) {
-            // memsync means update metadata with that of memory
-            if (!expected.equals(memory)) {
-              log.error(
-                  "Not synching metadata because the operation log {} does not agree with memory {}",
-                  expected, memory);
-            } else {
-              log.warn(
-                  "Synching metadata with memory because the operation log agrees with memory {}",
-                  expected);
-              resetMetadataWithMemory(metadata, datafileSizes);
-            }
-          }
-        }
-      }
-    }
-  }
-
-  private void resetMemoryWithMetadata(Map<StoredTabletFile,DataFileValue> metadata) {
-    // increment start count before metadata update AND updating in memory map of files
-    metadataUpdateCount.updateAndGet(MetadataUpdateCount::incrementStart);
-    try {
-      datafileSizes.clear();
-      datafileSizes.putAll(metadata);
-    } finally {
-      // increment start count before metadata update AND updating in memory map of files
-      metadataUpdateCount.updateAndGet(MetadataUpdateCount::incrementFinish);
-    }
+  public void logTransactions() {
+    // always log the operation log regardless of the requested action
+    log.error("Operation log: {}", tabletLog.dumpLog());
   }
 
-  private void resetMetadataWithMemory(Set<StoredTabletFile> metadata,
-      Map<StoredTabletFile,DataFileValue> memory) {
-    // increment start count before metadata update AND updating in memory map of files
-    metadataUpdateCount.updateAndGet(MetadataUpdateCount::incrementStart);
-    try {
-      ManagerMetadataUtil.replaceDatafiles(tablet.getContext(), tablet.getExtent(), metadata,
-          memory);
-    } finally {
-      // increment start count before metadata update AND updating in memory map of files
-      metadataUpdateCount.updateAndGet(MetadataUpdateCount::incrementFinish);
+  public void checkTransactionLog() {
+    Set<StoredTabletFile> files = datafileSizes.keySet();
+    if (!tabletLog.isExpectedFiles(files)) {
+      log.error("In-memory files {} do not match transaction log", files);
+      logTransactions();
     }
   }
-
 }

server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileTransaction.java

@@ -70,9 +70,7 @@ public Optional<StoredTabletFile> getDestination() {
     @Override
     public void apply(Set<StoredTabletFile> files) {
       files.removeAll(compactedFiles);
-      if (destination.isPresent()) {
-        files.add(destination.orElseThrow());
-      }
+      destination.ifPresent(files::add);
     }
 
     @Override
@@ -108,9 +106,7 @@ public Optional<StoredTabletFile> getFlushFile() {
 
     @Override
     public void apply(Set<StoredTabletFile> files) {
-      if (flushFile.isPresent()) {
-        files.add(flushFile.orElseThrow());
-      }
+      flushFile.ifPresent(files::add);
     }
 
     @Override