You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/10/26 13:23:27 UTC
[GitHub] [solr] janhoy opened a new pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
janhoy opened a new pull request #371:
URL: https://github.com/apache/solr/pull/371
https://issues.apache.org/jira/browse/SOLR-15718
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r738220950
##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
##########
@@ -34,40 +33,22 @@
* <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 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<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
return new OverseerSolrResponse(response);
} catch (IOException|RuntimeException e) {
- if (useUnsafeDeserialization()) {
- return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
Review comment:
There may be other features or APIs in Solr that also have a min-version requirement for a rolling upgrade. The page [Upgrading a Solr Cluster](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/src/upgrading-a-solr-cluster.adoc) does not seem to mention such a limitation either.
@HoustonPutman How will the solr-operator handle upgrade from 8.x to 9.0? Will it even attempt an automated rolling upgrade it, or will it have some prior-version check?
Ideally we should have some tool `upgradeChecker.py <old-cluster-url>` distributed with Solr that you could point to an old cluster, and it would introspect various APIs to query for version, core-index-version, iterate every schema and solrconfig file and look for removed fieldType classes etc., and output a report whether you can safely upgrade and if not, what needs to be done before you can upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r738208768
##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
##########
@@ -34,40 +33,22 @@
* <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 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<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
return new OverseerSolrResponse(response);
} catch (IOException|RuntimeException e) {
- if (useUnsafeDeserialization()) {
- return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
Review comment:
You mean for people attempting an upgrade directly from <8.5 to 9.0? I wonder if the RefGuide chapter "[Solr 9 Upgrade Planning](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/src/major-changes-in-solr-9.adoc#solr-9-upgrade-planning)" needs to specify the minimum version from which an upgrade is supported. Currently it talks about 8.1, but it should probably tell people to first upgrade to 8.11, then to 9.0?
But if people don't listen to that advice, could we perhaps check for this sysProp immediately on startup, e.g. in `SolrDispatchFilter#init()`, and refuse to start at all?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r739025136
##########
File path: solr/solr-ref-guide/src/major-changes-in-solr-9.adoc
##########
@@ -181,6 +181,8 @@ The only reason QEC supported the data directory was to support loading its chan
That feature now works from the configset dir too.
SolrCloud doesn't support that but may sometime.
+* If you plan to use a rolling upgrade strategy from an 8.x version prior to 8.5, you'll first need to do a rolling upgrade to the latest 8.x version. This is due to a change in the Overseer response format serialization in version 8.5. See <<solr-upgrade-notes.adoc#_solr_8_5,8.5 Upgrade Notes>> for details.
+
Review comment:
@ctargett I found no way to link directly to the sub-header. My IntelliJ has [auto-complete functionality for asciidoc](https://plugins.jetbrains.com/plugin/7391-asciidoc), and suggested to use the `_solr_5_5` anchor, and then it suggested to add that explicit anchor right above the sub header with the `[#anchor]` syntax, see below.
I see we're using a `[[anchor]]` syntax elsewhere. Should we standardize on one or the other?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r738375211
##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
##########
@@ -34,42 +33,32 @@
* <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 Utils.toJavabin(responseObject.getResponse()).readAllBytes();
} catch (IOException|RuntimeException e) {
+ checkUnsafeSerializationProperty();
Review comment:
Hmm, this may never be necessary I suppose. This would be if the overseer node is upgraded to 9.0 and the serialization fails.
Question is if we should throw exception here if the sysProp is set and never send a response, or if we only need to check for this flag during `deserialize()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r738370235
##########
File path: solr/solr-ref-guide/src/major-changes-in-solr-9.adoc
##########
@@ -181,6 +181,8 @@ The only reason QEC supported the data directory was to support loading its chan
That feature now works from the configset dir too.
SolrCloud doesn't support that but may sometime.
+* If you plan to use a rolling upgrade strategy from an 8.x version prior to 8.5, you'll first need to do a rolling upgrade to the latest 8.x version. This is due to a change in the Overseer response format serialization in version 8.5. See <<solr-upgrade-notes.adoc#_solr_8_5,8.5 Upgrade Notes>> for details.
+
Review comment:
I committed this release-note to highlight the importance of first upgrading to latest 8.x if you want to do rolling upgrade to 9.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] tflobbe commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
tflobbe commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r737933517
##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
##########
@@ -34,40 +33,22 @@
* <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 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<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
return new OverseerSolrResponse(response);
} catch (IOException|RuntimeException e) {
- if (useUnsafeDeserialization()) {
- return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
Review comment:
WDYT about throwing an exception here instead of completely removing? something like:
```
throw new SolrException("Unsafe deserialization is no longer supported. See SOLR-15718");
```
or something like that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ctargett commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
ctargett commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r739196929
##########
File path: solr/solr-ref-guide/src/major-changes-in-solr-9.adoc
##########
@@ -181,6 +181,8 @@ The only reason QEC supported the data directory was to support loading its chan
That feature now works from the configset dir too.
SolrCloud doesn't support that but may sometime.
+* If you plan to use a rolling upgrade strategy from an 8.x version prior to 8.5, you'll first need to do a rolling upgrade to the latest 8.x version. This is due to a change in the Overseer response format serialization in version 8.5. See <<solr-upgrade-notes.adoc#_solr_8_5,8.5 Upgrade Notes>> for details.
+
Review comment:
Instead of underscores, use hyphens, so `#solr-8-5`.
Because what you're trying to link to is a header (h3 in this case), the link is auto-created during conversion to HTML. The syntax is described here: https://github.com/apache/solr/blob/main/dev-docs/ref-guide/asciidoc-syntax.adoc#link-to-a-section-on-another-page.
Since this is an existing section that's been published in several versions of the Guide, you could also look at the existing published page to determine the auto-created link.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #371: SOLR-15718 Remove backcompat feature solr.useUnsafeOverseerResponse
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #371:
URL: https://github.com/apache/solr/pull/371#discussion_r738234381
##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerSolrResponseSerializer.java
##########
@@ -34,40 +33,22 @@
* <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 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<Object> response = (NamedList<Object>) Utils.fromJavabin(responseBytes);
return new OverseerSolrResponse(response);
} catch (IOException|RuntimeException e) {
- if (useUnsafeDeserialization()) {
- return (OverseerSolrResponse) SolrResponse.deserialize(responseBytes);
Review comment:
I added a check for the existence of the param with a different exception msg, and also a test for it. Marked these as deprecated, so we can remove these checks in 10.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org