(accumulo) branch elasticity updated: Fixed TabletManagmentParameters serialization of replacements (#4184)

[dl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
     new 57cb4ba7f4 Fixed TabletManagmentParameters serialization of replacements (#4184)
57cb4ba7f4 is described below

commit 57cb4ba7f4487c3cebfdb0aaa6b520a3d5bdd9fa
Author: Dave Marion <dl...@apache.org>
AuthorDate: Wed Jan 24 16:56:25 2024 -0500

    Fixed TabletManagmentParameters serialization of replacements (#4184)
    
    TabletManagementParameters was still using a List<Pair<Path,Path>>
    for volume replacements serialization. This was changed to a
    Map<Path,Path> in other places of the code because there was an
    issue with Gson serialization. I added a test for serialization
    and deserialization and was able to successfully use a Map when
    setting enableComplexMapKeySerialization() on the Gson object.
    
    Fixes #4107
    
    
    Co-authored-by: Keith Turner <kt...@apache.org>
---
 .../manager/state/TabletManagementParameters.java  | 28 ++++----
 .../state/TabletManagementParametersTest.java      | 76 ++++++++++++++++++++++
 2 files changed, 91 insertions(+), 13 deletions(-)

diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java
index c6bfbe3fc3..6dbcee6d03 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java
@@ -31,7 +31,6 @@ import java.util.Base64;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Supplier;
@@ -42,7 +41,6 @@ import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.manager.thrift.ManagerState;
 import org.apache.accumulo.core.metadata.TServerInstance;
 import org.apache.accumulo.core.metadata.schema.Ample;
-import org.apache.accumulo.core.util.Pair;
 import org.apache.accumulo.server.manager.LiveTServerSet;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.io.DataInputBuffer;
@@ -57,7 +55,10 @@ import com.google.gson.GsonBuilder;
  * {@link TabletManagementIterator} to make decisions about tablets.
  */
 public class TabletManagementParameters {
-  // ELASTICITY_TODO need to unit test serialization and deserialization of this class.
+
+  private static final Gson GSON =
+      new GsonBuilder().enableComplexMapKeySerialization().disableHtmlEscaping().create();
+
   private final ManagerState managerState;
   private final Map<Ample.DataLevel,Boolean> parentUpgradeMap;
   private final Set<TableId> onlineTables;
@@ -99,7 +100,7 @@ public class TabletManagementParameters {
       return Map.copyOf(resourceGroups);
     });
     this.canSuspendTablets = canSuspendTablets;
-    this.volumeReplacements = volumeReplacements;
+    this.volumeReplacements = Map.copyOf(volumeReplacements);
   }
 
   private TabletManagementParameters(JsonData jdata) {
@@ -125,14 +126,17 @@ public class TabletManagementParameters {
       return Map.copyOf(resourceGroups);
     });
     this.canSuspendTablets = jdata.canSuspendTablets;
-    this.volumeReplacements = jdata.volumeReplacements.stream()
-        .collect(toUnmodifiableMap(Pair::getFirst, Pair::getSecond));
+    this.volumeReplacements = Collections.unmodifiableMap(jdata.volumeReplacements);
   }
 
   public ManagerState getManagerState() {
     return managerState;
   }
 
+  public Map<Ample.DataLevel,Boolean> getParentUpgradeMap() {
+    return parentUpgradeMap;
+  }
+
   public boolean isParentLevelUpgraded() {
     return parentUpgradeMap.get(level);
   }
@@ -189,6 +193,7 @@ public class TabletManagementParameters {
   }
 
   private static class JsonData {
+
     final ManagerState managerState;
     final Map<Ample.DataLevel,Boolean> parentUpgradeMap;
     final Collection<String> onlineTables;
@@ -203,7 +208,7 @@ public class TabletManagementParameters {
     final Map<Long,Map<String,String>> compactionHints;
 
     final boolean canSuspendTablets;
-    final List<Pair<Path,Path>> volumeReplacements;
+    final Map<Path,Path> volumeReplacements;
 
     private static String toString(KeyExtent extent) {
       DataOutputBuffer buffer = new DataOutputBuffer();
@@ -245,20 +250,17 @@ public class TabletManagementParameters {
               .map(TServerInstance::getHostPortSession).collect(toSet())));
       compactionHints = params.compactionHints;
       canSuspendTablets = params.canSuspendTablets;
-      volumeReplacements =
-          params.volumeReplacements.entrySet().stream().map(Pair::fromEntry).collect(toList());
+      volumeReplacements = params.volumeReplacements;
     }
 
   }
 
   public String serialize() {
-    Gson gson = new GsonBuilder().disableHtmlEscaping().create();
-    return gson.toJson(new JsonData(this));
+    return GSON.toJson(new JsonData(this));
   }
 
   public static TabletManagementParameters deserialize(String json) {
-    Gson gson = new GsonBuilder().disableHtmlEscaping().create();
-    JsonData jdata = gson.fromJson(json, JsonData.class);
+    JsonData jdata = GSON.fromJson(json, JsonData.class);
     return new TabletManagementParameters(jdata);
   }
 
diff --git a/server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletManagementParametersTest.java b/server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletManagementParametersTest.java
new file mode 100644
index 0000000000..101157c44b
--- /dev/null
+++ b/server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletManagementParametersTest.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.accumulo.core.Constants;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.manager.thrift.ManagerState;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.server.manager.LiveTServerSet;
+import org.apache.hadoop.fs.Path;
+import org.junit.jupiter.api.Test;
+
+public class TabletManagementParametersTest {
+
+  @Test
+  public void testDeSer() {
+
+    final ManagerState managerState = ManagerState.NORMAL;
+    final Map<Ample.DataLevel,Boolean> parentUpgradeMap = Map.of(Ample.DataLevel.ROOT, true,
+        Ample.DataLevel.USER, true, Ample.DataLevel.METADATA, true);
+    final Set<TableId> onlineTables = Set.of(TableId.of("1"), TableId.of("2"), TableId.of("3"));
+    final Set<TServerInstance> tservers = Set.of(new TServerInstance("127.0.0.1:10000", 0),
+        new TServerInstance("127.0.0.1:10001", 1));
+    final LiveTServerSet.LiveTServersSnapshot serverSnapshot =
+        new LiveTServerSet.LiveTServersSnapshot(tservers,
+            Map.of(Constants.DEFAULT_RESOURCE_GROUP_NAME, tservers));
+    final Set<TServerInstance> serversToShutdown = Set.of();
+    final Map<KeyExtent,TServerInstance> migrations = Map.of();
+    final Ample.DataLevel dataLevel = Ample.DataLevel.USER;
+    final Map<Long,Map<String,String>> compactionHints = Map.of();
+    final boolean canSuspendTablets = true;
+    final Map<Path,Path> replacements =
+        Map.of(new Path("file:/vol1/accumulo/inst_id"), new Path("file:/vol2/accumulo/inst_id"));
+
+    final TabletManagementParameters tmp = new TabletManagementParameters(managerState,
+        parentUpgradeMap, onlineTables, serverSnapshot, serversToShutdown, migrations, dataLevel,
+        compactionHints, canSuspendTablets, replacements);
+
+    String jsonString = tmp.serialize();
+    TabletManagementParameters tmp2 = TabletManagementParameters.deserialize(jsonString);
+
+    assertEquals(managerState, tmp2.getManagerState());
+    assertEquals(parentUpgradeMap, tmp2.getParentUpgradeMap());
+    assertEquals(onlineTables, tmp2.getOnlineTables());
+    assertEquals(tservers, tmp2.getOnlineTsevers());
+    assertEquals(serversToShutdown, tmp2.getServersToShutdown());
+    assertEquals(migrations, tmp2.getMigrations());
+    assertEquals(dataLevel, tmp2.getLevel());
+    assertEquals(compactionHints, tmp2.getCompactionHints());
+    assertEquals(canSuspendTablets, tmp2.canSuspendTablets());
+    assertEquals(replacements, tmp2.getVolumeReplacements());
+  }
+}