use json to serialize application executor info

Author: squito

network/shuffle/pom.xml

@@ -49,6 +49,11 @@
       <version>1.8</version>
     </dependency>
 
+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-databind</artifactId>
+    </dependency>
+
     <!-- Provided dependencies -->
     <dependency>
       <groupId>org.slf4j</groupId>

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockHandler.java

@@ -51,7 +51,7 @@ public class ExternalShuffleBlockHandler extends RpcHandler {
   final ExternalShuffleBlockResolver blockManager;
   private final OneForOneStreamManager streamManager;
 
-  public ExternalShuffleBlockHandler(TransportConf conf, File registeredExecutorFile) {
+  public ExternalShuffleBlockHandler(TransportConf conf, File registeredExecutorFile) throws IOException {
     this(new OneForOneStreamManager(),
       new ExternalShuffleBlockResolver(conf, registeredExecutorFile));
   }

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolver.java

@@ -23,6 +23,9 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
 import com.google.common.base.Objects;
@@ -138,12 +141,10 @@ public void registerExecutor(
     synchronized (executors) {
       executors.put(fullId, executorInfo);
       try {
-        ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();
-        ObjectOutputStream out = new ObjectOutputStream(bytesOut);
-        out.writeObject(executorInfo);
-        out.close();
         if (db != null) {
-          db.put(dbAppExecKey(new AppExecId(appId, execId)), bytesOut.toByteArray());
+          byte[] key = dbAppExecKey(new AppExecId(appId, execId));
+          byte[] value = mapper.writeValueAsString(executorInfo).getBytes(Charsets.UTF_8);
+          db.put(key, value);
         }
       } catch (Exception e) {
         logger.error("Error saving registered executors", e);
@@ -204,7 +205,11 @@ public void applicationRemoved(String appId, boolean cleanupLocalDirs) {
       if (appId.equals(fullId.appId)) {
         it.remove();
         if (db != null) {
-          db.delete(dbAppExecKey(fullId));
+          try {
+            db.delete(dbAppExecKey(fullId));
+          } catch (IOException e) {
+            logger.error("Error deleting {} from executor state db", appId, e);
+          }
         }
 
         if (cleanupLocalDirs) {
@@ -301,11 +306,12 @@ void close() {
   }
 
   /** Simply encodes an executor's full ID, which is appId + execId. */
-  public static class AppExecId implements Serializable {
+  public static class AppExecId {
     public final String appId;
-    final String execId;
+    public final String execId;
 
-    public AppExecId(String appId, String execId) {
+    @JsonCreator
+    public AppExecId(@JsonProperty("appId") String appId, @JsonProperty("execId") String execId) {
       this.appId = appId;
       this.execId = execId;
     }
@@ -333,14 +339,20 @@ public String toString() {
     }
   }
 
-  private static byte[] dbAppExecKey(AppExecId appExecId) {
-    return (APP_KEY_PREFIX + ";" + appExecId.appId + ";" + appExecId.execId).getBytes(Charsets.UTF_8);
+  static ObjectMapper mapper = new ObjectMapper();
+
+  private static byte[] dbAppExecKey(AppExecId appExecId) throws IOException {
+    // we stick a common prefix on all the keys so we can find them in the DB
+    String appExecJson = mapper.writeValueAsString(appExecId);
+    String key = (APP_KEY_PREFIX + ";" + appExecJson);
+    return key.getBytes(Charsets.UTF_8);
   }
 
-  private static AppExecId parseDbAppExecKey(String s) {
+  private static AppExecId parseDbAppExecKey(String s) throws IOException {
     int p = s.indexOf(';');
-    int p2 = s.indexOf(';', p + 1);
-    return new AppExecId(s.substring(p + 1, p2), s.substring(p2 + 1));
+    String json = s.substring(p + 1);
+    AppExecId parsed = mapper.readValue(json, AppExecId.class);
+    return parsed;
   }
 
   private static final String APP_KEY_PREFIX = "AppExecShuffleInfo";
@@ -358,16 +370,9 @@ static ConcurrentMap<AppExecId, ExecutorShuffleInfo> reloadRegisteredExecutors(D
         if (!key.startsWith(APP_KEY_PREFIX))
           break;
         AppExecId id = parseDbAppExecKey(key);
-        ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(e.getValue()));
-        try {
-          registeredExecutors.put(
-            id,
-            (ExecutorShuffleInfo) in.readObject()
-          );
-        } catch (ClassNotFoundException e1) {
-          throw new IOException(e1);
-        }
-        in.close();
+        ExecutorShuffleInfo shuffleInfo =
+          mapper.readValue(new String(e.getValue(), Charsets.UTF_8), ExecutorShuffleInfo.class);
+        registeredExecutors.put(id, shuffleInfo);
       }
     }
     return registeredExecutors;

network/shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/ExecutorShuffleInfo.java

@@ -20,22 +20,28 @@
 import java.io.Serializable;
 import java.util.Arrays;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Objects;
 import io.netty.buffer.ByteBuf;
 
 import org.apache.spark.network.protocol.Encodable;
 import org.apache.spark.network.protocol.Encoders;
 
 /** Contains all configuration necessary for locating the shuffle files of an executor. */
-public class ExecutorShuffleInfo implements Encodable, Serializable {
+public class ExecutorShuffleInfo implements Encodable {
   /** The base set of local directories that the executor stores its shuffle files in. */
   public final String[] localDirs;
   /** Number of subdirectories created within each localDir. */
   public final int subDirsPerLocalDir;
   /** Shuffle manager (SortShuffleManager or HashShuffleManager) that the executor is using. */
   public final String shuffleManager;
 
-  public ExecutorShuffleInfo(String[] localDirs, int subDirsPerLocalDir, String shuffleManager) {
+  @JsonCreator
+  public ExecutorShuffleInfo(
+      @JsonProperty("localDirs")String[] localDirs,
+      @JsonProperty("subDirsPerLocalDir") int subDirsPerLocalDir,
+      @JsonProperty("shuffleManager") String shuffleManager) {
     this.localDirs = localDirs;
     this.subDirsPerLocalDir = subDirsPerLocalDir;
     this.shuffleManager = shuffleManager;

network/shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java

@@ -22,8 +22,10 @@
 import java.io.InputStreamReader;
 
 import com.google.common.io.CharStreams;
+import org.apache.spark.network.shuffle.protocol.ExecutorShuffleInfo;
 import org.apache.spark.network.util.SystemPropertyConfigProvider;
 import org.apache.spark.network.util.TransportConf;
+import org.apache.spark.network.shuffle.ExternalShuffleBlockResolver.AppExecId;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -59,7 +61,7 @@ public static void afterAll() {
   }
 
   @Test
-  public void testBadRequests() {
+  public void testBadRequests() throws IOException {
     ExternalShuffleBlockResolver resolver = new ExternalShuffleBlockResolver(conf, null);
     // Unregistered executor
     try {
@@ -126,4 +128,30 @@ public void testHashShuffleBlocks() throws IOException {
     block1Stream.close();
     assertEquals(hashBlock1, block1);
   }
+
+  @Test
+  public void jsonSerializationOfExecutorRegistration() throws IOException {
+    AppExecId appId = new AppExecId("foo", "bar");
+    String appIdJson = ExternalShuffleBlockResolver.mapper.writeValueAsString(appId);
+    AppExecId parsedAppId =
+      ExternalShuffleBlockResolver.mapper.readValue(appIdJson, AppExecId.class);
+    assertEquals(parsedAppId, appId);
+
+    ExecutorShuffleInfo shuffleInfo =
+      new ExecutorShuffleInfo(new String[]{"/bippy", "/flippy"}, 7, "hash");
+    String shuffleJson = ExternalShuffleBlockResolver.mapper.writeValueAsString(shuffleInfo);
+    ExecutorShuffleInfo parsedShuffleInfo =
+      ExternalShuffleBlockResolver.mapper.readValue(shuffleJson, ExecutorShuffleInfo.class);
+    assertEquals(parsedShuffleInfo, shuffleInfo);
+
+    // Intentionally keep these hard-coded strings in here, to check backwards-compatability.
+    // its not legacy yet, but keeping this here in case anybody changes it
+    String legacyAppIdJson = "{\"appId\":\"foo\", \"execId\":\"bar\"}";
+    assertEquals(appId,
+      ExternalShuffleBlockResolver.mapper.readValue(legacyAppIdJson, AppExecId.class));
+    String legacyShuffleJson = "{\"localDirs\": [\"/bippy\", \"/flippy\"], " +
+      "\"subDirsPerLocalDir\": 7, \"shuffleManager\": \"hash\"}";
+    assertEquals(shuffleInfo,
+      ExternalShuffleBlockResolver.mapper.readValue(legacyShuffleJson, ExecutorShuffleInfo.class));
+  }
 }

network/shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleSecuritySuite.java

@@ -43,7 +43,7 @@ public class ExternalShuffleSecuritySuite {
   TransportServer server;
 
   @Before
-  public void beforeEach() {
+  public void beforeEach() throws IOException {
     TransportContext context =
       new TransportContext(conf, new ExternalShuffleBlockHandler(conf, null));
     TransportServerBootstrap bootstrap = new SaslServerBootstrap(conf,