janhoy commented on a change in pull request #1528:
URL: https://github.com/apache/lucene-solr/pull/1528#discussion_r429775163
##########
File path:
solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
##########
@@ -790,6 +790,33 @@ public void simRemoveReplica(String nodeId, String
collection, String coreNodeNa
}
/**
+<<<<<<< HEAD
+=======
+ * Save clusterstate.json to {@link DistribStateManager}.
+ * @return saved state
+ */
+ private ClusterState saveClusterState(ClusterState state) throws IOException
{
+ ensureNotClosed();
+
+ // TODO: this method is emptied of its content in order to compile. We're
not saving the cluster state that has to be saved collection per collection in
separate state.json files.
+ // TODO: DO NOT CHECK THIS IN. Check with AB how to update sim to
stateFormat 2
+
+// byte[] data = Utils.toJSON(state);
+// try {
+// VersionedData oldData =
stateManager.getData(ZkStateReader.CLUSTER_STATE);
+// int version = oldData != null ? oldData.getVersion() : 0;
+// assert clusterStateVersion == version : "local clusterStateVersion out
of sync";
+// stateManager.setData(ZkStateReader.CLUSTER_STATE, data, version);
+// log.debug("** saved cluster state version {}", version);
+// clusterStateVersion++;
+// } catch (Exception e) {
+// throw new IOException(e);
+// }
+ return state;
+ }
+
+ /**
+>>>>>>> SOLR-12823: remove /clusterstate.json
Review comment:
???
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -491,6 +494,41 @@ public boolean isClosed() {
assert ObjectReleaseTracker.track(this);
}
+ /**
+ * <p>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.</p>
+ *
+ * <p>If /clusterstate.json exists and is empty, it is removed.</p>
+ */
+ 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 (Arrays.equals("{}".getBytes(StandardCharsets.UTF_8), data)) {
+ // Empty json. 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);
Review comment:
I was thinking of the rolling upgrade scenario - if someone upgrades
from 8.x to 9.0 one node at a time. Then the first node upgraded will delete
/clusterstate.json. Will that cause any kind of failures or exceptions in the
remaining nodes, if they have a watch on the znode or something?
A way to mitigate it could be to let only the Overseer do the delete, and
tell people to upgrade overseer last?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
##########
@@ -160,9 +159,6 @@ public void call(ClusterState state, ZkNodeProps message,
NamedList results) thr
Map<String, Object> propMap = new HashMap<>();
propMap.put(Overseer.QUEUE_OPERATION, CREATE.toString());
propMap.put("fromApi", "true"); // mostly true. Prevents
autoCreated=true in the collection state.
- if (properties.get(STATE_FORMAT) == null) {
Review comment:
What happens if someone backs up a 8.5 collection with stateFormat=1 and
then tries to restore in 9.0? Not very likely since that collection was
probably created pre-7.0 and it would not load in 9.0 anyway. But should we
simply throw an exception here if STATE_FORMAT is 1?
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java
##########
@@ -138,8 +138,7 @@ private ClusterState fetchClusterState(SolrClient client,
String collection, Map
Set<String> liveNodes = new
HashSet((List<String>)(cluster.get("live_nodes")));
this.liveNodes = liveNodes;
liveNodesTimestamp = System.nanoTime();
- //TODO SOLR-11877 we don't know the znode path; CLUSTER_STATE is probably
wrong leading to bad stateFormat
Review comment:
Remember to close SOLR-11877 after this
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
##########
@@ -210,47 +200,42 @@ public boolean liveNodesContain(String name) {
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
- sb.append("znodeVersion: ").append(znodeVersion);
- sb.append("\n");
sb.append("live nodes:").append(liveNodes);
sb.append("\n");
sb.append("collections:").append(collectionStates);
return sb.toString();
}
- public static ClusterState load(Integer version, byte[] bytes, Set<String>
liveNodes) {
- return load(version, bytes, liveNodes, ZkStateReader.CLUSTER_STATE);
- }
/**
- * Create ClusterState from json string that is typically stored in
zookeeper.
+ * Create a ClusterState from Json.
*
- * @param version zk version of the clusterstate.json file (bytes)
- * @param bytes clusterstate.json as a byte array
+ * @param bytes a byte array of a Json representation of a mapping from
collection name to the Json representation of a
+ * {@link DocCollection} as written by {@link
#write(JSONWriter)}. It can represent
+ * one or more collections.
* @param liveNodes list of live nodes
* @return the ClusterState
*/
- public static ClusterState load(Integer version, byte[] bytes, Set<String>
liveNodes, String znode) {
- // System.out.println("######## ClusterState.load:" + (bytes==null ? null
: new String(bytes)));
+ public static ClusterState createFromJson(int version, byte[] bytes,
Set<String> liveNodes) {
if (bytes == null || bytes.length == 0) {
- return new ClusterState(version, liveNodes, Collections.<String,
DocCollection>emptyMap());
+ return new ClusterState(liveNodes, Collections.<String,
DocCollection>emptyMap());
}
Map<String, Object> stateMap = (Map<String, Object>) Utils.fromJSON(bytes);
- return load(version, stateMap, liveNodes, znode);
+ return createFromData(version, stateMap, liveNodes);
}
- public static ClusterState load(Integer version, Map<String, Object>
stateMap, Set<String> liveNodes, String znode) {
+ public static ClusterState createFromData(int version, Map<String, Object>
stateMap, Set<String> liveNodes) {
Review comment:
Would `createFromMap` be a more descriptive name?
##########
File path: solr/core/src/test/org/apache/solr/cloud/BasicZkTest.java
##########
@@ -43,8 +44,12 @@
public static void beforeClass() {
}
-
+
@Test
+ @Ignore
+ // This test doesn't work (anymore) following
https://issues.apache.org/jira/browse/SOLR-12823
Review comment:
We should have plenty of coverage elsewhere, so +1 to remove this test?
##########
File path: solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java
##########
@@ -27,8 +27,10 @@
import org.apache.solr.core.CoreContainer;
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
+// TODO: this class tries to test Zookeeper using Solr abstractions, but ZK
implies the code is running in cloud mode. It doesn't work.
Review comment:
Perhaps MiniSolrCloud should have a `.withChroot` option which can be
asserted somewhere? What about deleting this test and create a separate
followup JIRA "Add zk chroot test" which can be fixed as a followup?
##########
File path: solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
##########
@@ -181,16 +180,21 @@ public void close() {
zkStateReader.close();
}
+ /**
+ * Create a collection.
+ * Note there's a similar but slightly different {@link
OverseerTest#createCollection(String, int)}.
Review comment:
Did you consider alernatives to creating a new method with same
signature and 95% same code? If duplication is necessary perhaps give the new
method a more descriptive name?
----------------------------------------------------------------
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
