epugh commented on code in PR #2391:
URL: https://github.com/apache/solr/pull/2391#discussion_r2435321334
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -342,6 +344,185 @@ public MiniSolrCloudCluster(
}
}
+ /**
+ * Create a MiniSolrCloudCluster with embedded ZooKeeper quorum mode. Each
Solr node runs its own
+ * embedded ZooKeeper server, and together they form a quorum.
+ *
+ * @param numServers number of Solr servers (must be at least 3 for quorum)
+ * @param baseDir base directory that the mini cluster should be run from
+ * @param solrXml solr.xml file content
+ * @param jettyConfig Jetty configuration
+ * @param securityJson Optional security.json configuration
+ * @param trackJettyMetrics whether to track Jetty metrics
+ * @throws Exception if there was an error starting the cluster
+ */
+ MiniSolrCloudCluster(
+ int numServers,
+ Path baseDir,
+ String solrXml,
+ JettyConfig jettyConfig,
+ Optional<String> securityJson,
+ boolean trackJettyMetrics,
+ boolean useEmbeddedZkQuorum)
+ throws Exception {
+
+ if (!useEmbeddedZkQuorum) {
+ throw new IllegalArgumentException("This constructor is only for
embedded ZK quorum mode");
+ }
+ if (numServers < 3) {
+ throw new IllegalArgumentException(
+ "ZooKeeper quorum requires at least 3 nodes, got: " + numServers);
+ }
+
+ Objects.requireNonNull(securityJson);
+ this.baseDir = Objects.requireNonNull(baseDir);
+ this.jettyConfig = Objects.requireNonNull(jettyConfig);
+ this.solrXml = solrXml == null ? DEFAULT_CLOUD_SOLR_XML : solrXml;
+ this.trackJettyMetrics = trackJettyMetrics;
+ this.externalZkServer = true; // No ZkTestServer in quorum mode
+ this.zkServer = null; // No single ZK server
+
+ log.info("Starting cluster of {} servers with embedded ZK quorum in {}",
numServers, baseDir);
+ Files.createDirectories(baseDir);
+
+ // Phase 1: Reserve random ports for all nodes
+ int[] ports = new int[numServers];
+ for (int i = 0; i < numServers; i++) {
+ try (java.net.ServerSocket socket = new java.net.ServerSocket(0)) {
+ ports[i] = socket.getLocalPort();
+ }
+ }
+
+ // Build the zkHost string with all ZK ports (Solr port + 1000)
+ StringBuilder zkHostBuilder = new StringBuilder();
+ for (int i = 0; i < numServers; i++) {
+ if (i > 0) {
+ zkHostBuilder.append(",");
+ }
+ int zkPort = ports[i] + 1000;
+ zkHostBuilder.append("127.0.0.1:").append(zkPort);
+ }
+ this.zkHost = zkHostBuilder.toString(); // Save for later use
+
+ if (log.isInfoEnabled()) {
+ log.info("Reserved ports for {} nodes: {}", numServers,
java.util.Arrays.toString(ports));
+ log.info("ZK connection string: {}", this.zkHost);
+ }
+
+ // Set system properties for embedded ZK quorum mode
+ System.setProperty("solr.zookeeper.server.enabled", "true");
Review Comment:
I am a bit curious if we need this system property AND
`zookeeper_quorum:on`? I wonder if the existence of `zookeeper_quorum:on` is
enough? if this system property is what turns on zookeeper, I wish maybe it
was `solr.mode=clustered` or something to specificy that it's the clustered
(cloud) mode...
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -342,6 +344,185 @@ public MiniSolrCloudCluster(
}
}
+ /**
+ * Create a MiniSolrCloudCluster with embedded ZooKeeper quorum mode. Each
Solr node runs its own
+ * embedded ZooKeeper server, and together they form a quorum.
+ *
+ * @param numServers number of Solr servers (must be at least 3 for quorum)
+ * @param baseDir base directory that the mini cluster should be run from
+ * @param solrXml solr.xml file content
+ * @param jettyConfig Jetty configuration
+ * @param securityJson Optional security.json configuration
+ * @param trackJettyMetrics whether to track Jetty metrics
+ * @throws Exception if there was an error starting the cluster
+ */
+ MiniSolrCloudCluster(
+ int numServers,
+ Path baseDir,
+ String solrXml,
+ JettyConfig jettyConfig,
+ Optional<String> securityJson,
+ boolean trackJettyMetrics,
+ boolean useEmbeddedZkQuorum)
+ throws Exception {
+
+ if (!useEmbeddedZkQuorum) {
+ throw new IllegalArgumentException("This constructor is only for
embedded ZK quorum mode");
+ }
+ if (numServers < 3) {
+ throw new IllegalArgumentException(
+ "ZooKeeper quorum requires at least 3 nodes, got: " + numServers);
+ }
+
+ Objects.requireNonNull(securityJson);
+ this.baseDir = Objects.requireNonNull(baseDir);
+ this.jettyConfig = Objects.requireNonNull(jettyConfig);
+ this.solrXml = solrXml == null ? DEFAULT_CLOUD_SOLR_XML : solrXml;
+ this.trackJettyMetrics = trackJettyMetrics;
+ this.externalZkServer = true; // No ZkTestServer in quorum mode
+ this.zkServer = null; // No single ZK server
+
+ log.info("Starting cluster of {} servers with embedded ZK quorum in {}",
numServers, baseDir);
+ Files.createDirectories(baseDir);
+
+ // Phase 1: Reserve random ports for all nodes
+ int[] ports = new int[numServers];
+ for (int i = 0; i < numServers; i++) {
+ try (java.net.ServerSocket socket = new java.net.ServerSocket(0)) {
+ ports[i] = socket.getLocalPort();
+ }
+ }
+
+ // Build the zkHost string with all ZK ports (Solr port + 1000)
+ StringBuilder zkHostBuilder = new StringBuilder();
+ for (int i = 0; i < numServers; i++) {
+ if (i > 0) {
+ zkHostBuilder.append(",");
+ }
+ int zkPort = ports[i] + 1000;
+ zkHostBuilder.append("127.0.0.1:").append(zkPort);
+ }
+ this.zkHost = zkHostBuilder.toString(); // Save for later use
+
+ if (log.isInfoEnabled()) {
+ log.info("Reserved ports for {} nodes: {}", numServers,
java.util.Arrays.toString(ports));
+ log.info("ZK connection string: {}", this.zkHost);
+ }
+
+ // Set system properties for embedded ZK quorum mode
+ System.setProperty("solr.zookeeper.server.enabled", "true");
+ System.setProperty("solr.security.manager.enabled", "false");
+ System.setProperty("solr.node.roles",
"data:on,overseer:allowed,zookeeper_quorum:on");
+ System.setProperty("solr.test.sys.prop1", "propone");
+ System.setProperty("solr.test.sys.prop2", "proptwo");
+ System.setProperty("solr.zookeeper.client.timeout", "300000"); // 5 minutes
+
+ // Phase 2: Start all nodes in parallel
+ List<Callable<JettySolrRunner>> startups = new ArrayList<>(numServers);
+ for (int i = 0; i < numServers; i++) {
+ final int solrPort = ports[i];
+ final String nodeName = newNodeName();
+ startups.add(
+ () -> {
+ Path runnerPath = createInstancePath(nodeName);
+ Files.write(runnerPath.resolve("solr.xml"),
solrXml.getBytes(StandardCharsets.UTF_8));
+
+ Properties nodeProps = new Properties();
+ nodeProps.setProperty("zkHost", this.zkHost);
+ nodeProps.setProperty("hostPort", String.valueOf(solrPort));
+
+ JettyConfig newConfig =
JettyConfig.builder(jettyConfig).setPort(solrPort).build();
+
+ JettySolrRunner jetty =
+ !trackJettyMetrics
+ ? new JettySolrRunner(runnerPath.toString(), nodeProps,
newConfig)
+ : new JettySolrRunnerWithMetrics(runnerPath.toString(),
nodeProps, newConfig);
+
+ int zkPort = solrPort + 1000;
+ log.info("Starting {} on port {} with ZK on port {}", nodeName,
solrPort, zkPort);
+ jetty.start();
+ log.info("Node {} started successfully", nodeName);
+
+ jettys.add(jetty);
+ synchronized (startupWait) {
+ startupWait.notifyAll();
+ }
+ return jetty;
+ });
+ }
+
+ final ExecutorService executorLauncher =
+ ExecutorUtil.newMDCAwareCachedThreadPool(new
SolrNamedThreadFactory("jetty-launcher"));
+ Collection<Future<JettySolrRunner>> futures =
executorLauncher.invokeAll(startups);
+ ExecutorUtil.shutdownAndAwaitTermination(executorLauncher);
+ Exception startupError =
+ checkForExceptions(
+ "Error starting up MiniSolrCloudCluster with embedded ZK quorum",
futures);
+ if (startupError != null) {
+ try {
+ this.shutdown();
+ } catch (Throwable t) {
+ startupError.addSuppressed(t);
+ }
+ throw startupError;
+ }
+
+ log.info("All {} nodes started, waiting for quorum formation...",
numServers);
+ Thread.sleep(10000); // Wait for ZK quorum to fully form
+
+ // Initialize ZK paths and security (if provided)
+ try (SolrZkClient zkClient =
+ new SolrZkClient.Builder()
+ .withUrl(this.zkHost)
+ .withTimeout(60000, TimeUnit.MILLISECONDS)
+ .build()) {
+ if (!zkClient.exists("/solr", true)) {
+ zkClient.makePath("/solr", false, true);
+ }
+ if (!zkClient.exists("/solr/initialized", true)) {
Review Comment:
is this a concept in solr? this path? New to me!
##########
solr/core/src/test/org/apache/solr/cloud/TestEmbeddedZkQuorum.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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 java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.embedded.JettySolrRunner;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Test embedded ZooKeeper running in quorum mode within Solr nodes.
+ *
+ * <p>This test verifies that:
+ *
+ * <ul>
+ * <li>Multiple Solr nodes can start with embedded ZK in quorum mode
+ * <li>The ZK quorum forms correctly
+ * <li>Collections can be created and used
+ * <li>Documents can be indexed and queried
+ * <li>All resources are properly closed on shutdown
+ * </ul>
+ */
+public class TestEmbeddedZkQuorum extends
SolrCloudWithEmbeddedZkQuorumTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String COLLECTION_NAME = "test_quorum_collection";
+ private static final int NUM_NODES = 3;
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ // Get path to a test config
+ Path configPath = TEST_PATH().resolve("collection1").resolve("conf");
+
+ // Configure cluster with 3 nodes, each running embedded ZK
+ cluster =
+ configureClusterWithEmbeddedZkQuorum(NUM_NODES).addConfig("conf1",
configPath).build();
+
+ log.info("Cluster configured with {} nodes", NUM_NODES);
+ }
+
+ @Test
+ public void testBasicQuorumFunctionality() throws Exception {
+ log.info("Starting testBasicQuorumFunctionality");
Review Comment:
I wonder if claude tools could go back and revamp old unit tests to be
clearer?
##########
solr/core/src/test/org/apache/solr/cloud/TestEmbeddedZkQuorum.java:
##########
Review Comment:
This test is verifying the happy path! I think that we need a test that
demonstrates that when you LOSE a node from a three node cluster, we still
function, just degraded. I don't know if we have a test that verifies this
behavior elsewhere (a stress test??) that could be reused with embedded zk?
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -342,6 +344,185 @@ public MiniSolrCloudCluster(
}
}
+ /**
+ * Create a MiniSolrCloudCluster with embedded ZooKeeper quorum mode. Each
Solr node runs its own
+ * embedded ZooKeeper server, and together they form a quorum.
+ *
+ * @param numServers number of Solr servers (must be at least 3 for quorum)
+ * @param baseDir base directory that the mini cluster should be run from
+ * @param solrXml solr.xml file content
+ * @param jettyConfig Jetty configuration
+ * @param securityJson Optional security.json configuration
+ * @param trackJettyMetrics whether to track Jetty metrics
+ * @throws Exception if there was an error starting the cluster
+ */
+ MiniSolrCloudCluster(
+ int numServers,
+ Path baseDir,
+ String solrXml,
+ JettyConfig jettyConfig,
+ Optional<String> securityJson,
+ boolean trackJettyMetrics,
+ boolean useEmbeddedZkQuorum)
+ throws Exception {
+
+ if (!useEmbeddedZkQuorum) {
+ throw new IllegalArgumentException("This constructor is only for
embedded ZK quorum mode");
+ }
+ if (numServers < 3) {
+ throw new IllegalArgumentException(
+ "ZooKeeper quorum requires at least 3 nodes, got: " + numServers);
+ }
+
+ Objects.requireNonNull(securityJson);
+ this.baseDir = Objects.requireNonNull(baseDir);
+ this.jettyConfig = Objects.requireNonNull(jettyConfig);
+ this.solrXml = solrXml == null ? DEFAULT_CLOUD_SOLR_XML : solrXml;
+ this.trackJettyMetrics = trackJettyMetrics;
+ this.externalZkServer = true; // No ZkTestServer in quorum mode
Review Comment:
something about this being `true` seems odd... Should it be false since
there isn't a externalZkServer?
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudWithEmbeddedZkQuorumTestCase.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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 java.lang.invoke.MethodHandles;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base class for SolrCloud tests that use embedded ZooKeeper running in
quorum mode
Review Comment:
I really like the javadocs! as someone who doesn't live this java code
everyday, they are helpful.
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -1174,16 +1379,33 @@ public MiniSolrCloudCluster build() throws Exception {
}
JettyConfig jettyConfig = jettyConfigBuilder.build();
- MiniSolrCloudCluster cluster =
- new MiniSolrCloudCluster(
- nodeCount,
- baseDir,
- solrXml,
- jettyConfig,
- null,
- securityJson,
- trackJettyMetrics,
- formatZkServer);
+ MiniSolrCloudCluster cluster;
+
+ if (useEmbeddedZkQuorum) {
Review Comment:
this big if statement seems challenging... Could a builder pattern make it
easier? Thir are a lot of params and overlap with subtle differences ;-).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
