From df268644b766bbf84e34e7f6fd30a609ab85cc61 Mon Sep 17 00:00:00 2001
From: Dom G <domgarguilo@apache.org>
Date: Fri, 5 Jan 2024 10:55:28 -0500
Subject: [PATCH] Fix ImportConfiguration.builder().setKeepOffline() (#4115)

* fixes a bug where `setKeepOffline` would never transition a table from the NEW state to OFFLINE
---
 .../org/apache/accumulo/manager/FateServiceHandler.java    | 7 +++----
 .../manager/tableOps/tableImport/FinishImportTable.java    | 5 ++---
 .../accumulo/manager/tableOps/tableImport/ImportTable.java | 4 ++--
 .../manager/tableOps/tableImport/ImportedTableInfo.java    | 2 +-
 .../main/java/org/apache/accumulo/test/ImportExportIT.java | 2 ++
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
index f4d1257128b..6c6d0f5a32c 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
@@ -607,10 +607,9 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe
 
         goalMessage += "Import table with new name: " + tableName + " from " + exportDirs;
         manager.fate()
-            .seedTransaction(
-                op.toString(), opid, new TraceRepo<>(new ImportTable(c.getPrincipal(), tableName,
-                    exportDirs, namespaceId, keepMappings, !keepOffline)),
-                autoCleanup, goalMessage);
+            .seedTransaction(op.toString(), opid, new TraceRepo<>(new ImportTable(c.getPrincipal(),
+                tableName, exportDirs, namespaceId, keepMappings, keepOffline)), autoCleanup,
+                goalMessage);
         break;
       }
       case TABLE_EXPORT: {
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/FinishImportTable.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/FinishImportTable.java
index 0df0771ecf0..e5df7220430 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/FinishImportTable.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/FinishImportTable.java
@@ -52,9 +52,8 @@ public Repo<Manager> call(long tid, Manager env) throws Exception {
       }
     }
 
-    if (tableInfo.onlineTable) {
-      env.getTableManager().transitionTableState(tableInfo.tableId, TableState.ONLINE);
-    }
+    final TableState newState = tableInfo.keepOffline ? TableState.OFFLINE : TableState.ONLINE;
+    env.getTableManager().transitionTableState(tableInfo.tableId, newState);
 
     Utils.unreserveNamespace(env, tableInfo.namespaceId, tid, false);
     Utils.unreserveTable(env, tableInfo.tableId, tid, true);
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportTable.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportTable.java
index 49e67f8e839..484db2b9636 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportTable.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportTable.java
@@ -62,14 +62,14 @@ public class ImportTable extends ManagerRepo {
   private final ImportedTableInfo tableInfo;
 
   public ImportTable(String user, String tableName, Set<String> exportDirs, NamespaceId namespaceId,
-      boolean keepMappings, boolean onlineTable) {
+      boolean keepMappings, boolean keepOffline) {
     tableInfo = new ImportedTableInfo();
     tableInfo.tableName = tableName;
     tableInfo.user = user;
     tableInfo.namespaceId = namespaceId;
     tableInfo.directories = parseExportDir(exportDirs);
     tableInfo.keepMappings = keepMappings;
-    tableInfo.onlineTable = onlineTable;
+    tableInfo.keepOffline = keepOffline;
   }
 
   @Override
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportedTableInfo.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportedTableInfo.java
index f3d84a9c22d..306c7a38e52 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportedTableInfo.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportedTableInfo.java
@@ -35,7 +35,7 @@ class ImportedTableInfo implements Serializable {
   public List<DirectoryMapping> directories;
   public String exportFile;
   public boolean keepMappings;
-  public boolean onlineTable;
+  public boolean keepOffline;
 
   static class DirectoryMapping implements Serializable {
     private static final long serialVersionUID = 1L;
diff --git a/test/src/main/java/org/apache/accumulo/test/ImportExportIT.java b/test/src/main/java/org/apache/accumulo/test/ImportExportIT.java
index 5e40ea3751b..49f01c1b361 100644
--- a/test/src/main/java/org/apache/accumulo/test/ImportExportIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/ImportExportIT.java
@@ -47,6 +47,7 @@
 import org.apache.accumulo.core.data.Mutation;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.manager.state.tables.TableState;
 import org.apache.accumulo.core.metadata.MetadataTable;
 import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
 import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
@@ -294,6 +295,7 @@ public void testExportImportOffline() throws Exception {
 
       // verify the new table is offline
       assertFalse(client.tableOperations().isOnline(destTable), "Table should have been offline.");
+      assertEquals(getServerContext().getTableState(TableId.of(tableId)), TableState.OFFLINE);
       client.tableOperations().online(destTable, true);
 
       // Get all `file` colfams from the metadata table for the new table