You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by tf...@apache.org on 2020/02/04 19:08:49 UTC

[lucene-solr] branch branch_8x updated: SOLR-14219: Revert changes in OverseerSolrRespose and move serialization (#1227)

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

tflobbe pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 37d4121  SOLR-14219: Revert changes in OverseerSolrRespose and move serialization (#1227)
37d4121 is described below

commit 37d412177041fbaf62b17ac8d8234238aaab0038
Author: Tomas Fernandez Lobbe <tf...@apache.org>
AuthorDate: Tue Feb 4 10:26:57 2020 -0800

    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                                   |  3 ++
 .../apache/solr/cloud/OverseerSolrResponse.java    | 54 +---------------------
 ...se.java => OverseerSolrResponseSerializer.java} | 40 +++-------------
 .../apache/solr/cloud/OverseerTaskProcessor.java   |  6 +--
 .../solr/handler/admin/CollectionsHandler.java     |  7 +--
 .../solr/handler/admin/ConfigSetsHandler.java      |  3 +-
 .../OverseerCollectionConfigSetProcessorTest.java  |  2 +-
 .../solr/cloud/OverseerSolrResponseTest.java       |  6 +--
 ...verseerSolrResponseUnsafeSerializationTest.java |  4 +-
 9 files changed, 25 insertions(+), 100 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2dbcbcc..cf06872 100644
--- a/solr/CHANGES.txt
+++ b/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
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java b/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java
index 135bffb..92f6443 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java
+++ b/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;
   
@@ -55,50 +48,5 @@ public class OverseerSolrResponse extends SolrResponse {
   public NamedList<Object> getResponse() {
     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);
-    }
-  }
   
 }
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java b/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
similarity index 84%
copy from solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java
copy to solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
index 135bffb..85e74df 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
@@ -16,6 +16,9 @@
  */
 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;
@@ -23,39 +26,8 @@ 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;
-
-  private long elapsedTime;
-  
-  public OverseerSolrResponse(NamedList list) {
-    responseList = list;
-  }
-  
-  @Override
-  public long getElapsedTime() {
-    return elapsedTime;
-  }
+public class OverseerSolrResponseSerializer {
   
-  @Override
-  public void setResponse(NamedList<Object> rsp) {
-    this.responseList = rsp;
-  }
-
-  @Override
-  public void setElapsedTime(long elapsedTime) {
-    this.elapsedTime = elapsedTime;
-  }
-
-  @Override
-  public NamedList<Object> getResponse() {
-    return responseList;
-  }
-
   /**
    * This method serializes the content of an {@code OverseerSolrResponse}. Note that:
    * <ul>
@@ -75,7 +47,7 @@ public class OverseerSolrResponse extends SolrResponse {
       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));
@@ -100,5 +72,5 @@ public class OverseerSolrResponse extends SolrResponse {
       throw new SolrException(ErrorCode.SERVER_ERROR, "Exception deserializing response from Javabin", e);
     }
   }
-  
+
 }
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java
index 01cd829..374b9c3 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java
+++ b/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());
         }
 
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index fbb6e7c..d837be6 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/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");
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
index 22d5d3a..1486589 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
+++ b/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) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java
index e582f03..6660015 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java
+++ b/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);
diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java
index b2e2f60..3107ecd 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java
+++ b/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);
-    OverseerSolrResponse deserialized = OverseerSolrResponse.deserialize(serialized);
+    byte[] serialized = OverseerSolrResponseSerializer.serialize(response);
+    OverseerSolrResponse deserialized = OverseerSolrResponseSerializer.deserialize(serialized);
     assertEquals("Deserialized response is different than original", response.getResponse(), deserialized.getResponse());
   }
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java
index 1c10451..1d3d8e7 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java
+++ b/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, deserializationEnabled, OverseerSolrResponse.useUnsafeDeserialization());
+      assertEquals("Unexpected serialization toggle for value: " + propertyValue, serializationEnabled, OverseerSolrResponseSerializer.useUnsafeSerialization());
+      assertEquals("Unexpected serialization toggle for value: " + propertyValue, deserializationEnabled, OverseerSolrResponseSerializer.useUnsafeDeserialization());
     } finally {
       if (previousValue != null) {
         System.setProperty("solr.useUnsafeOverseerResponse", previousValue);