SOLR-14219: Revert changes in OverseerSolrRespose and move serialization (#1227)

SOLR-14095 Introduced an issue for rolling restarts (Incompatible Java serialization). This change fixes the compatibility issue while keeping the functionality in SOLR-14095

solr/CHANGES.txt

@@ -181,6 +181,9 @@ Bug Fixes
 
 * SOLR-14090: Handle the case in `delete-copy-field` when source is a dynamic field (Frank Iversen, Munendra S N)
 
+* SOLR-14219: SOLR-14095 Introduced an issue for rolling restarts (Incompatible Java serialization). This change
+  Fixes the compatibility issue while keeping the functionality in SOLR-14095. (Andy Webb, Tomás Fernández Löbbe)
+
 Other Changes
 ---------------------
 

solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java

@@ -16,19 +16,12 @@
  */
 package org.apache.solr.cloud;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.solr.client.solrj.SolrResponse;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.Utils;
-
-import java.io.IOException;
-import java.util.Objects;
 
 public class OverseerSolrResponse extends SolrResponse {
   
-  NamedList<Object> responseList = null;
+  NamedList responseList = null;
 
   private long elapsedTime;
   
@@ -56,49 +49,4 @@ public class OverseerSolrResponse extends SolrResponse {
     return responseList;
   }
   
-  /**
-   * This method serializes the content of an {@code OverseerSolrResponse}. Note that:
-   * <ul>
-   * <li>The elapsed time is not serialized</li>
-   * <li>"Unknown" elements for the Javabin format will be serialized as Strings. See {@link org.apache.solr.common.util.JavaBinCodec#writeVal}</li>
-   * </ul>
-   */
-  @SuppressWarnings("deprecation")
-  public static byte[] serialize(OverseerSolrResponse responseObject) {
-    Objects.requireNonNull(responseObject);
-    if (useUnsafeSerialization()) {
-      return SolrResponse.serializable(responseObject);
-    }
-    try {
-      return IOUtils.toByteArray(Utils.toJavabin(responseObject.getResponse()));
-    } catch (IOException|RuntimeException e) {
-      throw new SolrException(ErrorCode.SERVER_ERROR, "Exception serializing response to Javabin", e);
-    }
-  }
-
-  static boolean useUnsafeSerialization() {
-    String useUnsafeOverseerResponse = System.getProperty("solr.useUnsafeOverseerResponse");
-    return useUnsafeOverseerResponse != null && ("true".equals(useUnsafeOverseerResponse));
-  }
-  
-  static boolean useUnsafeDeserialization() {
-    String useUnsafeOverseerResponse = System.getProperty("solr.useUnsafeOverseerResponse");
-    return useUnsafeOverseerResponse != null && ("true".equals(useUnsafeOverseerResponse) || "deserialization".equals(useUnsafeOverseerResponse));
-  }
-
-  @SuppressWarnings("deprecation")
-  public static OverseerSolrResponse deserialize(byte[] responseBytes) {
-    Objects.requireNonNull(responseBytes);
-    try {
-      @SuppressWarnings("unchecked")
-      NamedList<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
-      return new OverseerSolrResponse(response);
-    } catch (IOException|RuntimeException e) {
-      if (useUnsafeDeserialization()) {
-        return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
-      }
-      throw new SolrException(ErrorCode.SERVER_ERROR, "Exception deserializing response from Javabin", e);
-    }
-  }
-  
 }

solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.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
+ *
+ *     http://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.solr.cloud;
+
+import java.io.IOException;
+import java.util.Objects;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
+
+public class OverseerSolrResponseSerializer {
+  
+  /**
+   * This method serializes the content of an {@code OverseerSolrResponse}. Note that:
+   * <ul>
+   * <li>The elapsed time is not serialized</li>
+   * <li>"Unknown" elements for the Javabin format will be serialized as Strings. See {@link org.apache.solr.common.util.JavaBinCodec#writeVal}</li>
+   * </ul>
+   */
+  @SuppressWarnings("deprecation")
+  public static byte[] serialize(OverseerSolrResponse responseObject) {
+    Objects.requireNonNull(responseObject);
+    if (useUnsafeSerialization()) {
+      return SolrResponse.serializable(responseObject);
+    }
+    try {
+      return IOUtils.toByteArray(Utils.toJavabin(responseObject.getResponse()));
+    } catch (IOException|RuntimeException e) {
+      throw new SolrException(ErrorCode.SERVER_ERROR, "Exception serializing response to Javabin", e);
+    }
+  }
+  
+  static boolean useUnsafeSerialization() {
+    String useUnsafeOverseerResponse = System.getProperty("solr.useUnsafeOverseerResponse");
+    return useUnsafeOverseerResponse != null && ("true".equals(useUnsafeOverseerResponse));
+  }
+  
+  static boolean useUnsafeDeserialization() {
+    String useUnsafeOverseerResponse = System.getProperty("solr.useUnsafeOverseerResponse");
+    return useUnsafeOverseerResponse != null && ("true".equals(useUnsafeOverseerResponse) || "deserialization".equals(useUnsafeOverseerResponse));
+  }
+
+  @SuppressWarnings("deprecation")
+  public static OverseerSolrResponse deserialize(byte[] responseBytes) {
+    Objects.requireNonNull(responseBytes);
+    try {
+      @SuppressWarnings("unchecked")
+      NamedList<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
+      return new OverseerSolrResponse(response);
+    } catch (IOException|RuntimeException e) {
+      if (useUnsafeDeserialization()) {
+        return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
+      }
+      throw new SolrException(ErrorCode.SERVER_ERROR, "Exception deserializing response from Javabin", e);
+    }
+  }
+
+}

solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java

@@ -510,14 +510,14 @@ public class OverseerTaskProcessor implements Runnable, Closeable {
         if (asyncId != null) {
           if (response != null && (response.getResponse().get("failure") != null 
               || response.getResponse().get("exception") != null)) {
-            failureMap.put(asyncId, OverseerSolrResponse.serialize(response));
+            failureMap.put(asyncId, OverseerSolrResponseSerializer.serialize(response));
             log.debug("Updated failed map for task with zkid:[{}]", head.getId());
           } else {
-            completedMap.put(asyncId, OverseerSolrResponse.serialize(response));
+            completedMap.put(asyncId, OverseerSolrResponseSerializer.serialize(response));
             log.debug("Updated completed map for task with zkid:[{}]", head.getId());
           }
         } else {
-          head.setBytes(OverseerSolrResponse.serialize(response));
+          head.setBytes(OverseerSolrResponseSerializer.serialize(response));
           log.debug("Completed task:[{}]", head.getId());
         }
 

solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java

@@ -29,6 +29,7 @@ import org.apache.solr.client.solrj.request.CoreAdminRequest.RequestSyncShard;
 import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.client.solrj.util.SolrIdentifierValidator;
 import org.apache.solr.cloud.OverseerSolrResponse;
+import org.apache.solr.cloud.OverseerSolrResponseSerializer;
 import org.apache.solr.cloud.OverseerTaskQueue;
 import org.apache.solr.cloud.OverseerTaskQueue.QueueEvent;
 import org.apache.solr.cloud.ZkController;
@@ -368,7 +369,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
         .getOverseerCollectionQueue()
         .offer(Utils.toJSON(m), timeout);
     if (event.getBytes() != null) {
-      return OverseerSolrResponse.deserialize(event.getBytes());
+      return OverseerSolrResponseSerializer.deserialize(event.getBytes());
     } else {
       if (System.nanoTime() - time >= TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS)) {
         throw new SolrException(ErrorCode.SERVER_ERROR, operation
@@ -874,11 +875,11 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       final NamedList<Object> results = new NamedList<>();
       if (zkController.getOverseerCompletedMap().contains(requestId)) {
         final byte[] mapEntry = zkController.getOverseerCompletedMap().get(requestId);
-        rsp.getValues().addAll(OverseerSolrResponse.deserialize(mapEntry).getResponse());
+        rsp.getValues().addAll(OverseerSolrResponseSerializer.deserialize(mapEntry).getResponse());
         addStatusToResponse(results, COMPLETED, "found [" + requestId + "] in completed tasks");
       } else if (zkController.getOverseerFailureMap().contains(requestId)) {
         final byte[] mapEntry = zkController.getOverseerFailureMap().get(requestId);
-        rsp.getValues().addAll(OverseerSolrResponse.deserialize(mapEntry).getResponse());
+        rsp.getValues().addAll(OverseerSolrResponseSerializer.deserialize(mapEntry).getResponse());
         addStatusToResponse(results, FAILED, "found [" + requestId + "] in failed tasks");
       } else if (zkController.getOverseerRunningMap().contains(requestId)) {
         addStatusToResponse(results, RUNNING, "found [" + requestId + "] in running tasks");

solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java

@@ -33,6 +33,7 @@ import org.apache.hadoop.fs.Path;
 import org.apache.solr.api.Api;
 import org.apache.solr.client.solrj.SolrResponse;
 import org.apache.solr.cloud.OverseerSolrResponse;
+import org.apache.solr.cloud.OverseerSolrResponseSerializer;
 import org.apache.solr.cloud.OverseerTaskQueue.QueueEvent;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
@@ -212,7 +213,7 @@ public class ConfigSetsHandler extends RequestHandlerBase implements PermissionN
         .getOverseerConfigSetQueue()
         .offer(Utils.toJSON(m), timeout);
     if (event.getBytes() != null) {
-      SolrResponse response = OverseerSolrResponse.deserialize(event.getBytes());
+      SolrResponse response = OverseerSolrResponseSerializer.deserialize(event.getBytes());
       rsp.getValues().addAll(response.getResponse());
       SimpleOrderedMap exp = (SimpleOrderedMap) response.getResponse().get("exception");
       if (exp != null) {

solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java

@@ -564,7 +564,7 @@ public class OverseerCollectionConfigSetProcessorTest extends SolrTestCaseJ4 {
     QueueEvent qe = new QueueEvent("id", Utils.toJSON(props), null){
       @Override
       public void setBytes(byte[] bytes) {
-        lastProcessMessageResult = OverseerSolrResponse.deserialize(bytes);
+        lastProcessMessageResult = OverseerSolrResponseSerializer.deserialize(bytes);
       }
     };
     queue.add(qe);

solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java

@@ -62,7 +62,7 @@ public class OverseerSolrResponseTest extends SolrTestCaseJ4 {
     exceptionNl.add("msg", e.getMessage());
     exceptionNl.add("rspCode", e.code());
     responseNl.add("exception", exceptionNl);
-    OverseerSolrResponse deserialized = OverseerSolrResponse.deserialize(OverseerSolrResponse.serialize(new OverseerSolrResponse(responseNl)));
+    OverseerSolrResponse deserialized = OverseerSolrResponseSerializer.deserialize(OverseerSolrResponseSerializer.serialize(new OverseerSolrResponse(responseNl)));
     assertNotNull("Expecting an exception", deserialized.getException());
     assertEquals("Unexpected exception type in deserialized response", SolrException.class, deserialized.getException().getClass());
     assertEquals("Unexpected exception code in deserialized response", e.code(), ((SolrException)deserialized.getException()).code());
@@ -71,8 +71,8 @@ public class OverseerSolrResponseTest extends SolrTestCaseJ4 {
   
   private void assertSerializeDeserialize(NamedList<Object> content) {
     OverseerSolrResponse response = new OverseerSolrResponse(content);
-    byte[] serialized = OverseerSolrResponse.serialize(response);
+    byte[] serialized = OverseerSolrResponseSerializer.serialize(response);
-    OverseerSolrResponse deserialized = OverseerSolrResponse.deserialize(serialized);
+    OverseerSolrResponse deserialized = OverseerSolrResponseSerializer.deserialize(serialized);
     assertEquals("Deserialized response is different than original", response.getResponse(), deserialized.getResponse());
   }
 

solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java

@@ -49,8 +49,8 @@ public class OverseerSolrResponseUnsafeSerializationTest extends OverseerSolrRes
       } else {
         System.setProperty("solr.useUnsafeOverseerResponse", propertyValue);
       }
-      assertEquals("Unexpected serialization toggle for value: " + propertyValue, serializationEnabled, OverseerSolrResponse.useUnsafeSerialization());
+      assertEquals("Unexpected serialization toggle for value: " + propertyValue, serializationEnabled, OverseerSolrResponseSerializer.useUnsafeSerialization());
-      assertEquals("Unexpected serialization toggle for value: " + propertyValue, deserializationEnabled, OverseerSolrResponse.useUnsafeDeserialization());
+      assertEquals("Unexpected serialization toggle for value: " + propertyValue, deserializationEnabled, OverseerSolrResponseSerializer.useUnsafeDeserialization());
     } finally {
       if (previousValue != null) {
         System.setProperty("solr.useUnsafeOverseerResponse", previousValue);