madrob commented on a change in pull request #1528:
URL: https://github.com/apache/lucene-solr/pull/1528#discussion_r427580023
##
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##
@@ -491,6 +493,40 @@ public boolean isClosed() {
assert ObjectReleaseTracker.track(this);
}
+ /**
+ * Verifies if /clusterstate.json exists in Zookeepeer, and if it does
and is not empty, refuses to start and outputs
+ * a helpful message regarding collection migration.
+ *
+ * If /clusterstate.json exists and is empty, it is removed.
+ */
+ private void checkNoOldClusterstate(final SolrZkClient zkClient) throws
InterruptedException {
+try {
+ if (!zkClient.exists(ZkStateReader.UNSUPPORTED_CLUSTER_STATE, true)) {
+return;
+ }
+
+ final byte[] data =
zkClient.getData(ZkStateReader.UNSUPPORTED_CLUSTER_STATE, null, null, true);
+
+ if (data.length < 5) {
Review comment:
Can we be more rigorous on the test here?
##
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##
@@ -236,21 +235,19 @@ public void call(ClusterState clusterState, ZkNodeProps
message, NamedList resul
}
String baseUrl = zkStateReader.getBaseUrlForNodeName(nodeName);
-//in the new mode, create the replica in clusterstate prior to
creating the core.
+//create the replica in clusterstate (i.e. ZK) prior to creating the
core.
Review comment:
s/clusterstate/state?
##
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##
@@ -491,6 +493,40 @@ public boolean isClosed() {
assert ObjectReleaseTracker.track(this);
}
+ /**
+ * Verifies if /clusterstate.json exists in Zookeepeer, and if it does
and is not empty, refuses to start and outputs
+ * a helpful message regarding collection migration.
+ *
+ * If /clusterstate.json exists and is empty, it is removed.
+ */
+ private void checkNoOldClusterstate(final SolrZkClient zkClient) throws
InterruptedException {
+try {
+ if (!zkClient.exists(ZkStateReader.UNSUPPORTED_CLUSTER_STATE, true)) {
+return;
+ }
+
+ final byte[] data =
zkClient.getData(ZkStateReader.UNSUPPORTED_CLUSTER_STATE, null, null, true);
+
+ if (data.length < 5) {
+// less than 5 chars is empty (it's likely just "{}"). This log will
only occur once.
+log.warn("{} no longer supported starting with Solr 9. Found empty
file on Zookeeper, deleting it.", ZkStateReader.UNSUPPORTED_CLUSTER_STATE);
+zkClient.delete(ZkStateReader.UNSUPPORTED_CLUSTER_STATE, -1, true);
+ } else {
+// /clusterstate.json not empty: refuse to start but do not
automatically delete. A bit of a pain but user shouldn't
+// have older collections at this stage anyway.
+String message = ZkStateReader.UNSUPPORTED_CLUSTER_STATE + " no longer
supported starting with Solr 9. "
++ "It is present and not empty. Cannot start Solr. Please first
migrate collections to stateFormat=2 using an "
++ "older version of Solr or if you don't care about the data then
delete the file from "
++ "Zookeeper using a command line tool, for example: bin/solr zk
rm /clusterstate.json -z host:port";
+log.error(message);
+throw new SolrException(SolrException.ErrorCode.INVALID_STATE,
message);
+ }
+} catch (KeeperException e) {
+ log.error("", e);
Review comment:
What are the conditions where this happens? log-and-throw is usually an
anti-pattern.
##
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
##
@@ -468,36 +467,6 @@ void checkResults(String label, NamedList results,
boolean failureIsFata
}
}
-
- //TODO should we not remove in the next release ?
- private void migrateStateFormat(ClusterState state, ZkNodeProps message,
NamedList results) throws Exception {
Review comment:
Yay!
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
