From c4f68bdab91d79d10986470b775f2dffd35f084a Mon Sep 17 00:00:00 2001 From: Tomas Fernandez Lobbe Date: Fri, 20 Dec 2019 14:55:01 -0800 Subject: [PATCH] SOLR-14095: Let the overseer use javabin to store responses in ZooKeeper (#1095) The Overseer used java serialization to store command responses in ZooKeeper. This commit changes the code to use Javabin instead, while allowing Java serialization with a System property in case it's needed for compatibility --- solr/CHANGES.txt | 34 ++++++++ .../OverseerConfigSetMessageHandler.java | 25 +++--- .../solr/cloud/OverseerMessageHandler.java | 3 +- .../solr/cloud/OverseerSolrResponse.java | 53 ++++++++++++- .../solr/cloud/OverseerTaskProcessor.java | 9 +-- .../OverseerCollectionMessageHandler.java | 4 +- .../handler/admin/CollectionsHandler.java | 46 +++++------ .../solr/handler/admin/ConfigSetsHandler.java | 2 +- ...rseerCollectionConfigSetProcessorTest.java | 2 +- .../solr/cloud/OverseerSolrResponseTest.java | 79 +++++++++++++++++++ ...erSolrResponseUnsafeSerializationTest.java | 61 ++++++++++++++ .../solr/cloud/OverseerTaskQueueTest.java | 5 +- .../solr/client/solrj/SolrResponse.java | 14 ++-- .../org/apache/solr/common/util/Utils.java | 63 ++++++++------- 14 files changed, 314 insertions(+), 86 deletions(-) create mode 100644 solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java create mode 100644 solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index c70a2ad4ab5..c9f5f1d7cc8 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -118,6 +118,38 @@ Upgrade Notes * SOLR-13983: Process execution is removed from SystemInfoHandler. A best-effort attempt to execute "uname -a" and "uptime" on non-Windows platforms is no longer made. (rmuir) + +* SOLR-14095 introduces a change in the format used for the elements in the Overseer queues and maps (see the Jira + issue for details on the reasons for the change). This queue is used internally by the Overseer to reliably handle + operations, to communicate operation results between the Overseer and the coordinator node, and by the + REQUESTSTATUS API for displaying information about async Collection operations. + This change won’t require you to change any client-side code you should see no differences on the client side, + however, it does require some care when upgrading an existing SolrCloud cluster: + - If you are upgrading Solr with an atomic restart strategy: + - If you don’t use async or REQUESTSTATUS operations, you should be able to restart and not see any issues. + - If you do use Collection API operations: + 1. pause Collection API operations + 2. cleanup queues (https://lucene.apache.org/solr/guide/8_3/collections-api.html#examples-using-deletestatus) + if you use async operations + 3. upgrade and restart the nodes + - If you are upgrading Solr with a rolling restart strategy: + - If you don’t use Collection API operations, you should be able to do a rolling restart and not see + any issues. + - If you do use Collection API operations, but you can pause their use during the restart the easiest + way is to: + 1. pause Collection API operations + 2. upgrade and restart all nodes + 3. cleanup queues (https://lucene.apache.org/solr/guide/8_3/collections-api.html#examples-using-deletestatus) + if you use async operations + 4. Resume all normal operations + - If you use Collection API operations and can’t pause them during the upgrade: + 1. Start 8.5 nodes with the system property: `-Dsolr.useUnsafeOverseerResponse=deserialization`. Ensure the + Overseer node is upgraded last + 2. Once all nodes are in 8.5 and once you don’t need to read old status anymore, restart again removing the + system property + If you prefer to keep the old (but insecure) serialization strategy, you can start your nodes using the + property: `-Dsolr.useUnsafeOverseerResponse=true`. Keep in mind that this will be removed in future version of Solr. + New Features --------------------- @@ -127,6 +159,8 @@ Improvements --------------------- * SOLR-14042: Fix varargs precommit warnings (Andraas Salamon via Jason Gerlowski) +* SOLR-14095: Replace Java serialization with Javabin in the Overseer queues (Tomás Fernández Löbbe) + Optimizations --------------------- (No changes) diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java index 945aba5b404..ca29196b55d 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java @@ -16,18 +16,6 @@ */ package org.apache.solr.cloud; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.lang.invoke.MethodHandles; -import java.nio.charset.StandardCharsets; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; - -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.cloud.DocCollection; @@ -44,6 +32,17 @@ import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; + import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.params.ConfigSetParams.ConfigSetAction.CREATE; import static org.apache.solr.common.util.Utils.toJSONString; @@ -90,7 +89,7 @@ public class OverseerConfigSetMessageHandler implements OverseerMessageHandler { } @Override - public SolrResponse processMessage(ZkNodeProps message, String operation) { + public OverseerSolrResponse processMessage(ZkNodeProps message, String operation) { NamedList results = new NamedList(); try { if (!operation.startsWith(CONFIGSETS_ACTION_PREFIX)) { diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerMessageHandler.java index c4027ccd5fd..1a40a0a539e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerMessageHandler.java @@ -16,7 +16,6 @@ */ package org.apache.solr.cloud; -import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.common.cloud.ZkNodeProps; /** @@ -30,7 +29,7 @@ public interface OverseerMessageHandler { * * @return response */ - SolrResponse processMessage(ZkNodeProps message, String operation); + OverseerSolrResponse processMessage(ZkNodeProps message, String operation); /** * @return the name of the OverseerMessageHandler 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 92f6443eace..8ecce58d967 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponse.java @@ -17,11 +17,17 @@ package org.apache.solr.cloud; 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 responseList = null; + NamedList responseList = null; private long elapsedTime; @@ -48,5 +54,50 @@ public class OverseerSolrResponse extends SolrResponse { public NamedList getResponse() { return responseList; } + + /** + * This method serializes the content of an {@code OverseerSolrResponse}. Note that: + *
    + *
  • The elapsed time is not serialized
    • + *
  • "Unknown" elements for the Javabin format will be serialized as Strings. See {@link org.apache.solr.common.util.JavaBinCodec#writeVal}
    • + *
+ */ + @SuppressWarnings("deprecation") + public static byte[] serialize(OverseerSolrResponse responseObject) { + Objects.requireNonNull(responseObject); + if (useUnsafeSerialization()) { + return SolrResponse.serializable(responseObject); + } + try { + return Utils.toJavabin(responseObject.getResponse()).readAllBytes(); + } 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 response = (NamedList) 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/OverseerTaskProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java index 4afe7c4720b..618b5b6f8ca 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java @@ -34,7 +34,6 @@ import java.util.function.Predicate; import com.codahale.metrics.Timer; import com.google.common.collect.ImmutableSet; import org.apache.commons.io.IOUtils; -import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.cloud.Overseer.LeaderStatus; import org.apache.solr.cloud.OverseerTaskQueue.QueueEvent; import org.apache.solr.common.AlreadyClosedException; @@ -476,7 +475,7 @@ public class OverseerTaskProcessor implements Runnable, Closeable { protected class Runner implements Runnable { ZkNodeProps message; String operation; - SolrResponse response; + OverseerSolrResponse response; QueueEvent head; OverseerMessageHandler messageHandler; private final OverseerMessageHandler.Lock lock; @@ -511,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, SolrResponse.serializable(response)); + failureMap.put(asyncId, OverseerSolrResponse.serialize(response)); log.debug("Updated failed map for task with zkid:[{}]", head.getId()); } else { - completedMap.put(asyncId, SolrResponse.serializable(response)); + completedMap.put(asyncId, OverseerSolrResponse.serialize(response)); log.debug("Updated completed map for task with zkid:[{}]", head.getId()); } } else { - head.setBytes(SolrResponse.serializable(response)); + head.setBytes(OverseerSolrResponse.serialize(response)); log.debug("Completed task:[{}]", head.getId()); } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java index 54b7f5b5d6f..72b39630684 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java @@ -249,7 +249,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler, @Override @SuppressWarnings("unchecked") - public SolrResponse processMessage(ZkNodeProps message, String operation) { + public OverseerSolrResponse processMessage(ZkNodeProps message, String operation) { MDCLoggingContext.setCollection(message.getStr(COLLECTION)); MDCLoggingContext.setShard(message.getStr(SHARD_ID_PROP)); MDCLoggingContext.setReplica(message.getStr(REPLICA_PROP)); @@ -277,7 +277,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler, } results.add("Operation " + operation + " caused exception:", e); - SimpleOrderedMap nl = new SimpleOrderedMap(); + SimpleOrderedMap nl = new SimpleOrderedMap<>(); nl.add("msg", e.getMessage()); nl.add("rspCode", e instanceof SolrException ? ((SolrException)e).code() : -1); results.add("exception", nl); 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 5843a949d5d..fbb6e7cae16 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 @@ -16,25 +16,6 @@ */ package org.apache.solr.handler.admin; -import java.io.IOException; -import java.lang.invoke.MethodHandles; -import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.stream.Collectors; - import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import org.apache.commons.io.IOUtils; @@ -102,6 +83,25 @@ import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; + import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.POLICY; import static org.apache.solr.client.solrj.response.RequestStatusState.COMPLETED; import static org.apache.solr.client.solrj.response.RequestStatusState.FAILED; @@ -149,10 +149,10 @@ import static org.apache.solr.common.params.CollectionParams.CollectionAction.*; import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonAdminParams.IN_PLACE_MOVE; import static org.apache.solr.common.params.CommonAdminParams.NUM_SUB_SHARDS; +import static org.apache.solr.common.params.CommonAdminParams.SPLIT_BY_PREFIX; import static org.apache.solr.common.params.CommonAdminParams.SPLIT_FUZZ; import static org.apache.solr.common.params.CommonAdminParams.SPLIT_METHOD; import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; -import static org.apache.solr.common.params.CommonAdminParams.SPLIT_BY_PREFIX; import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.params.CommonParams.TIMING; import static org.apache.solr.common.params.CommonParams.VALUE_LONG; @@ -368,7 +368,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission .getOverseerCollectionQueue() .offer(Utils.toJSON(m), timeout); if (event.getBytes() != null) { - return SolrResponse.deserialize(event.getBytes()); + return OverseerSolrResponse.deserialize(event.getBytes()); } else { if (System.nanoTime() - time >= TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS)) { throw new SolrException(ErrorCode.SERVER_ERROR, operation @@ -874,11 +874,11 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission final NamedList results = new NamedList<>(); if (zkController.getOverseerCompletedMap().contains(requestId)) { final byte[] mapEntry = zkController.getOverseerCompletedMap().get(requestId); - rsp.getValues().addAll(SolrResponse.deserialize(mapEntry).getResponse()); + rsp.getValues().addAll(OverseerSolrResponse.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(SolrResponse.deserialize(mapEntry).getResponse()); + rsp.getValues().addAll(OverseerSolrResponse.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 706b82df6d7..22d5d3a1878 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 @@ -212,7 +212,7 @@ public class ConfigSetsHandler extends RequestHandlerBase implements PermissionN .getOverseerConfigSetQueue() .offer(Utils.toJSON(m), timeout); if (event.getBytes() != null) { - SolrResponse response = SolrResponse.deserialize(event.getBytes()); + SolrResponse response = OverseerSolrResponse.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 6bb769368d4..e582f03c08f 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 = SolrResponse.deserialize( bytes); + lastProcessMessageResult = OverseerSolrResponse.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 new file mode 100644 index 00000000000..b2e2f60a1c1 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseTest.java @@ -0,0 +1,79 @@ +/* + * 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 org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; + +public class OverseerSolrResponseTest extends SolrTestCaseJ4 { + + public void testEmpty() { + assertSerializeDeserialize(new NamedList()); + } + + public void testWithSingleObject() { + NamedList responseNl = new NamedList<>(); + responseNl.add("foo", "bar"); + assertSerializeDeserialize(responseNl); + } + + public void testWithMultipleObject() { + NamedList responseNl = new NamedList<>(); + responseNl.add("foo", "bar"); + responseNl.add("foobar", "foo"); + assertSerializeDeserialize(responseNl); + } + + public void testRepeatedKeys() { + NamedList responseNl = new NamedList<>(); + responseNl.add("foo", "bar"); + responseNl.add("foo", "zoo"); + assertSerializeDeserialize(responseNl); + } + + public void testNested() { + NamedList responseNl = new NamedList<>(); + NamedList response2 = new NamedList<>(); + response2.add("foo", "bar"); + responseNl.add("foo", response2); + assertSerializeDeserialize(responseNl); + } + + public void testException() { + NamedList responseNl = new NamedList<>(); + SolrException e = new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Foo"); + SimpleOrderedMap exceptionNl = new SimpleOrderedMap<>(); + exceptionNl.add("msg", e.getMessage()); + exceptionNl.add("rspCode", e.code()); + responseNl.add("exception", exceptionNl); + OverseerSolrResponse deserialized = OverseerSolrResponse.deserialize(OverseerSolrResponse.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()); + assertEquals("Unexpected exception message in deserialized response", e.getMessage(), deserialized.getException().getMessage()); + } + + private void assertSerializeDeserialize(NamedList content) { + OverseerSolrResponse response = new OverseerSolrResponse(content); + byte[] serialized = OverseerSolrResponse.serialize(response); + OverseerSolrResponse deserialized = OverseerSolrResponse.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 new file mode 100644 index 00000000000..1c104515d88 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerSolrResponseUnsafeSerializationTest.java @@ -0,0 +1,61 @@ +/* + * 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 org.junit.AfterClass; +import org.junit.BeforeClass; + +public class OverseerSolrResponseUnsafeSerializationTest extends OverseerSolrResponseTest { + + @BeforeClass + public static void setUpClass() { + System.setProperty("solr.useUnsafeOverseerResponse", "true"); + } + + @AfterClass + public static void tearDownClass() { + System.clearProperty("solr.useUnsafeOverseerResponse"); + } + + + public void testUnsafeSerializartionToggles() { + assertToggles("true", true, true); + assertToggles("deserialization", false, true); + assertToggles(null, false, false); // By default, don't use unsafe + assertToggles("foo", false, false); + assertToggles("false", false, false); + assertToggles("serialization", false, false); // This is not an option + } + + private void assertToggles(String propertyValue, boolean serializationEnabled, boolean deserializationEnabled) { + String previousValue = System.getProperty("solr.useUnsafeOverseerResponse"); + try { + if (propertyValue == null) { + System.clearProperty("solr.useUnsafeOverseerResponse"); + } 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()); + } finally { + if (previousValue != null) { + System.setProperty("solr.useUnsafeOverseerResponse", previousValue); + } + } + } + +} diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java index 5f20d7d4acd..331bf41b426 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java @@ -16,12 +16,11 @@ */ package org.apache.solr.cloud; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.apache.solr.client.solrj.SolrResponse; -import org.apache.solr.client.solrj.response.SolrResponseBase; import org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionAdminParams; @@ -86,7 +85,7 @@ public class OverseerTaskQueueTest extends DistributedQueueTest { } } assertNotNull("Didn't find event with requestid " + requestId2, requestId2Event); - requestId2Event.setBytes(SolrResponse.serializable(new SolrResponseBase())); + requestId2Event.setBytes("foo bar".getBytes(StandardCharsets.UTF_8)); tq.remove(requestId2Event); // Make sure this call to check if requestId exists doesn't barf with Json parse exception diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrResponse.java index f267755b6e4..7abde81fa49 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrResponse.java @@ -16,6 +16,12 @@ */ package org.apache.solr.client.solrj; +import org.apache.solr.common.MapWriter; +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.SuppressForbidden; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -23,12 +29,6 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; -import org.apache.solr.common.MapWriter; -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.SuppressForbidden; - /** * @@ -62,6 +62,7 @@ public abstract class SolrResponse implements Serializable, MapWriter { } @SuppressForbidden(reason = "XXX: security hole") + @Deprecated public static byte[] serializable(SolrResponse response) { try { ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); @@ -74,6 +75,7 @@ public abstract class SolrResponse implements Serializable, MapWriter { } @SuppressForbidden(reason = "XXX: security hole") + @Deprecated public static SolrResponse deserialize(byte[] bytes) { try { ByteArrayInputStream byteStream = new ByteArrayInputStream(bytes); diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index 7fcba6448cf..fdea6de1f87 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -16,6 +16,34 @@ */ package org.apache.solr.common.util; +import org.apache.http.HttpEntity; +import org.apache.http.HttpResponse; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.util.EntityUtils; +import org.apache.solr.client.solrj.cloud.DistribStateManager; +import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData; +import org.apache.solr.client.solrj.impl.BinaryRequestWriter; +import org.apache.solr.common.IteratorWriter; +import org.apache.solr.common.LinkedHashMapWriter; +import org.apache.solr.common.MapWriter; +import org.apache.solr.common.MapWriterMap; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.SpecProvider; +import org.apache.solr.common.cloud.SolrZkClient; +import org.apache.solr.common.cloud.ZkOperation; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.CommonParams; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.server.ByteBufferInputStream; +import org.noggit.CharArr; +import org.noggit.JSONParser; +import org.noggit.JSONWriter; +import org.noggit.ObjectBuilder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -54,34 +82,6 @@ import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.util.EntityUtils; -import org.apache.solr.client.solrj.cloud.DistribStateManager; -import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData; -import org.apache.solr.client.solrj.impl.BinaryRequestWriter; -import org.apache.solr.common.IteratorWriter; -import org.apache.solr.common.LinkedHashMapWriter; -import org.apache.solr.common.MapWriter; -import org.apache.solr.common.MapWriterMap; -import org.apache.solr.common.SolrException; -import org.apache.solr.common.SpecProvider; -import org.apache.solr.common.cloud.SolrZkClient; -import org.apache.solr.common.cloud.ZkOperation; -import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.CommonParams; -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.server.ByteBufferInputStream; -import org.noggit.CharArr; -import org.noggit.JSONParser; -import org.noggit.JSONWriter; -import org.noggit.ObjectBuilder; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.slf4j.MDC; - import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; import static java.util.Collections.unmodifiableList; @@ -175,6 +175,12 @@ public class Utils { return new ByteBufferInputStream(ByteBuffer.wrap(baos.getbuf(), 0, baos.size())); } } + + public static Object fromJavabin(byte[] bytes) throws IOException { + try (JavaBinCodec jbc = new JavaBinCodec()) { + return jbc.unmarshal(bytes); + } + } public static Collection getDeepCopy(Collection c, int maxDepth, boolean mutable) { return getDeepCopy(c, maxDepth, mutable, false); @@ -295,7 +301,6 @@ public class Utils { } } - public static final Function STANDARDOBJBUILDER = jsonParser -> { try { return new ObjectBuilder(jsonParser);