Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16911: Remove ability to change hostContext, keep it as /solr #1810

Merged
merged 15 commits into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void bootstrapJettyServer() throws Exception {
Path.of("src/resources/configs/minimal/conf"), defaultConfigsetDir.resolve("conf"));
PathUtils.copyFileToDirectory(Path.of("src/resources/solr.xml"), tmpSolrHome);

solrRunner = new JettySolrRunner(tmpSolrHome.toString(), buildJettyConfig("/solr"));
solrRunner = new JettySolrRunner(tmpSolrHome.toString(), buildJettyConfig());
solrRunner.start(false);
try (SolrClient client = solrRunner.newClient()) {
for (int i = 0; i < NUM_CORES; i++) {
Expand Down Expand Up @@ -113,8 +113,8 @@ public void destroyJettyServer() throws Exception {
IOUtils.rm(tmpSolrHome);
}

private static JettyConfig buildJettyConfig(String context) {
return JettyConfig.builder().setContext(context).stopAtShutdown(true).build();
private static JettyConfig buildJettyConfig() {
return JettyConfig.builder().stopAtShutdown(true).build();
}
}
}
5 changes: 2 additions & 3 deletions solr/benchmark/src/resources/solr.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@
<solrcloud>
<str name="host">127.0.0.1</str>
<int name="hostPort">${hostPort:8983}</int>
<str name="hostContext">${hostContext:solr}</str>
<int name="zkClientTimeout">${solr.zkclienttimeout:60000}</int> <!-- This should be high by default - dc's are expensive -->
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
<int name="leaderVoteWait">${leaderVoteWait:15000}</int> <!-- We are running tests - the default should be low, not like production -->
<int name="leaderConflictResolveWait">${leaderConflictResolveWait:45000}</int>
<int name="leaderConflictResolveWait">${leaderConflictResolveWait:45000}</int>
<int name="distribUpdateConnTimeout">${distribUpdateConnTimeout:5000}</int>
<int name="distribUpdateSoTimeout">${distribUpdateSoTimeout:30000}</int> <!-- We are running tests - the default should be low, not like production -->
</solrcloud>

</solr>
38 changes: 3 additions & 35 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import java.lang.reflect.Array;
import java.net.InetAddress;
import java.net.NetworkInterface;
import java.net.URLEncoder;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand Down Expand Up @@ -324,17 +323,10 @@ public ZkController(

this.genericCoreNodeNames = cloudConfig.getGenericCoreNodeNames();

// be forgiving and strip this off leading/trailing slashes
// this allows us to support users specifying hostContext="/" in
// solr.xml to indicate the root context, instead of hostContext=""
// which means the default of "solr"
String localHostContext = trimLeadingAndTrailingSlashes(cloudConfig.getSolrHostContext());

this.zkServerAddress = zkServerAddress;
this.localHostPort = cloudConfig.getSolrHostPort();
this.hostName = normalizeHostName(cloudConfig.getHost());
this.nodeName =
generateNodeName(this.hostName, Integer.toString(this.localHostPort), localHostContext);
this.nodeName = generateNodeName(this.hostName, Integer.toString(this.localHostPort));
MDCLoggingContext.setNode(nodeName);
this.leaderVoteWait = cloudConfig.getLeaderVoteWait();
this.leaderConflictResolveWait = cloudConfig.getLeaderConflictResolveWait();
Expand Down Expand Up @@ -2364,35 +2356,11 @@ public LeaderElector getOverseerElector() {
*
* @param hostName - must not be null or the empty string
* @param hostPort - must consist only of digits, must not be null or the empty string
* @param hostContext - should not begin or end with a slash (leading/trailin slashes will be
* ignored), must not be null, may be the empty string to denote the root context
* @lucene.experimental
* @see ZkStateReader#getBaseUrlForNodeName
*/
static String generateNodeName(
final String hostName, final String hostPort, final String hostContext) {
return hostName
+ ':'
+ hostPort
+ '_'
+ URLEncoder.encode(trimLeadingAndTrailingSlashes(hostContext), StandardCharsets.UTF_8);
}

/**
* Utility method for trimming and leading and/or trailing slashes from its input. May return the
* empty string. May return null if and only if the input is null.
*/
public static String trimLeadingAndTrailingSlashes(final String in) {
if (null == in) return in;

String out = in;
if (out.startsWith("/")) {
out = out.substring(1);
}
if (out.endsWith("/")) {
out = out.substring(0, out.length() - 1);
}
return out;
static String generateNodeName(final String hostName, final String hostPort) {
return hostName + ':' + hostPort + '_' + "solr";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to consider dropping the _solr here but this may need to be a 10.0 only thing. Deserves a JIRA issue if we can get this committed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if this happens it should be a separate jira and definitely only be Solr 10x. I think leaving it would be fine, but a discussion for a different PR anyways

}

public void rejoinOverseerElection(String electionNode, boolean joinAtHead) {
Expand Down
19 changes: 0 additions & 19 deletions solr/core/src/java/org/apache/solr/core/CloudConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ public class CloudConfig {

private final String hostName;

private final String hostContext;

private final boolean useGenericCoreNames;

private final int leaderVoteWait;
Expand Down Expand Up @@ -63,7 +61,6 @@ public class CloudConfig {
int zkClientTimeout,
int hostPort,
String hostName,
String hostContext,
boolean useGenericCoreNames,
int leaderVoteWait,
int leaderConflictResolveWait,
Expand All @@ -82,7 +79,6 @@ public class CloudConfig {
this.zkClientTimeout = zkClientTimeout;
this.hostPort = hostPort;
this.hostName = hostName;
this.hostContext = hostContext;
this.useGenericCoreNames = useGenericCoreNames;
this.leaderVoteWait = leaderVoteWait;
this.leaderConflictResolveWait = leaderConflictResolveWait;
Expand All @@ -107,10 +103,6 @@ public class CloudConfig {
if (this.hostPort == -1)
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR, "'hostPort' must be configured to run SolrCloud");
if (this.hostContext == null)
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
"'hostContext' must be configured to run SolrCloud");
}

public String getZkHost() {
Expand All @@ -125,10 +117,6 @@ public int getSolrHostPort() {
return hostPort;
}

public String getSolrHostContext() {
return hostContext;
}

public String getHost() {
return hostName;
}
Expand Down Expand Up @@ -203,7 +191,6 @@ public static class CloudConfigBuilder {
private int zkClientTimeout = Integer.getInteger("zkClientTimeout", DEFAULT_ZK_CLIENT_TIMEOUT);
private final int hostPort;
private final String hostName;
private final String hostContext;
private boolean useGenericCoreNames;
private int leaderVoteWait = DEFAULT_LEADER_VOTE_WAIT;
private int leaderConflictResolveWait = DEFAULT_LEADER_CONFLICT_RESOLVE_WAIT;
Expand All @@ -222,13 +209,8 @@ public static class CloudConfigBuilder {
private String stateCompressorClass;

public CloudConfigBuilder(String hostName, int hostPort) {
this(hostName, hostPort, null);
}

public CloudConfigBuilder(String hostName, int hostPort, String hostContext) {
this.hostName = hostName;
this.hostPort = hostPort;
this.hostContext = hostContext;
}

public CloudConfigBuilder setZkHost(String zkHost) {
Expand Down Expand Up @@ -323,7 +305,6 @@ public CloudConfig build() {
zkClientTimeout,
hostPort,
hostName,
hostContext,
useGenericCoreNames,
leaderVoteWait,
leaderConflictResolveWait,
Expand Down
18 changes: 8 additions & 10 deletions solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -411,13 +411,6 @@ private static List<String> separateStrings(String commaSeparatedString) {
return Arrays.asList(COMMA_SEPARATED_PATTERN.split(commaSeparatedString));
}

private static Set<String> separateStringsToSet(String commaSeparatedString) {
if (StrUtils.isNullOrEmpty(commaSeparatedString)) {
return Collections.emptySet();
}
return Set.of(COMMA_SEPARATED_PATTERN.split(commaSeparatedString));
}

private static Set<Path> separatePaths(String commaSeparatedString) {
if (StrUtils.isNullOrEmpty(commaSeparatedString)) {
return Collections.emptySet();
Expand Down Expand Up @@ -516,10 +509,15 @@ private static CloudConfig fillSolrCloudSection(NamedList<Object> nl, String def
hostPort = parseInt("jetty.port", System.getProperty("jetty.port", "8983"));
}
String hostName = required("solrcloud", "host", removeValue(nl, "host"));
String hostContext = required("solrcloud", "hostContext", removeValue(nl, "hostContext"));

CloudConfig.CloudConfigBuilder builder =
new CloudConfig.CloudConfigBuilder(hostName, hostPort, hostContext);
// We no longer require or support the hostContext property, but legacy users may have it, so
// remove it from the list.
String hostContext = removeValue(nl, "hostContext");
if (hostContext != null) {
log.warn("solr.xml hostContext -- hostContext is deprecated and ignored.");
}

CloudConfig.CloudConfigBuilder builder = new CloudConfig.CloudConfigBuilder(hostName, hostPort);
// set the defaultZkHost until/unless it's overridden in the "cloud section" (below)...
builder.setZkHost(defaultZkHost);

Expand Down
1 change: 0 additions & 1 deletion solr/core/src/test-files/solr/solr-50-all.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
<int name="maxUpdateConnectionsPerHost">37</int>
<int name="leaderVoteWait">55</int>
<str name="host">testHost</str>
<str name="hostContext">testHostContext</str>
<int name="hostPort">${hostPort:44}</int>
<int name="zkClientTimeout">77</int>
<str name="zkHost">testZkHost</str>
Expand Down
1 change: 0 additions & 1 deletion solr/core/src/test-files/solr/solr-jmxreporter.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
<solrcloud>
<str name="host">127.0.0.1</str>
<int name="hostPort">${hostPort:8983}</int>
<str name="hostContext">${hostContext:solr}</str>
<int name="zkClientTimeout">${solr.zkclienttimeout:30000}</int>
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
<int name="leaderVoteWait">${leaderVoteWait:10000}</int>
Expand Down
1 change: 0 additions & 1 deletion solr/core/src/test-files/solr/solr-stress-new.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
<solrcloud>
<str name="host">127.0.0.1</str>
<int name="hostPort">8983</int>
<str name="hostContext">${hostContext:solr}</str>
</solrcloud>

<shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

<solrcloud>
<str name="host">127.0.0.1</str>
<str name="hostContext">${hostContext:solr}</str>
<int name="hostPort">${hostPort:8983}</int>
<int name="zkClientTimeout">${solr.zkclienttimeout:30000}</int>
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
Expand Down
5 changes: 2 additions & 3 deletions solr/core/src/test-files/solr/solr.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@
<solrcloud>
<str name="host">127.0.0.1</str>
<int name="hostPort">${hostPort:8983}</int>
<str name="hostContext">${hostContext:solr}</str>
<int name="zkClientTimeout">${solr.zkclienttimeout:60000}</int> <!-- This should be high by default - dc's are expensive -->
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
<int name="leaderVoteWait">${leaderVoteWait:15000}</int> <!-- We are running tests - the default should be low, not like production -->
<int name="leaderConflictResolveWait">${leaderConflictResolveWait:45000}</int>
<int name="leaderConflictResolveWait">${leaderConflictResolveWait:45000}</int>
<int name="distribUpdateConnTimeout">${distribUpdateConnTimeout:5000}</int>
<int name="distribUpdateSoTimeout">${distribUpdateSoTimeout:30000}</int> <!-- We are running tests - the default should be low, not like production -->
</solrcloud>

</solr>
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static void beforeTest() throws Exception {
}
jetty =
new JettySolrRunner(
homeDir.toAbsolutePath().toString(), nodeProperties, buildJettyConfig("/solr"));
homeDir.toAbsolutePath().toString(), nodeProperties, buildJettyConfig());

jetty.start();
port = jetty.getLocalPort();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public int execute(org.apache.commons.exec.CommandLine cmd) throws IOException {
Files.readAllBytes(Paths.get(solrHomeDir).resolve("solr.xml")),
Charset.defaultCharset());

JettyConfig jettyConfig = JettyConfig.builder().setContext("/solr").setPort(port).build();
JettyConfig jettyConfig = JettyConfig.builder().setPort(port).build();
try {
if (solrCloudCluster == null) {
Path logDir = createTempDir("solr_logs");
Expand Down Expand Up @@ -230,7 +230,7 @@ protected int startStandaloneSolr(String[] args) {
System.setProperty("jetty.port", String.valueOf(port));
System.setProperty("solr.log.dir", createTempDir("solr_logs").toString());

standaloneSolr = new JettySolrRunner(solrHomeDir.getAbsolutePath(), "/solr", port);
standaloneSolr = new JettySolrRunner(solrHomeDir.getAbsolutePath(), port);
Thread bg =
new Thread() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ private SolrZkClient electNewOverseer(String address)
"/admin/cores",
reader,
null,
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983, "solr").build());
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983).build());
overseer.close();
ElectionContext ec =
new OverseerElectionContext(zkClient, overseer, address.replace("/", "_"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ public void testBuildClusterState_Simple() {
assertNotNull(replica1);
assertEquals("baseUrl1:8983_", replica1.getNodeName());
assertEquals("slice1_replica1", replica1.getCoreName());
assertEquals("http://baseUrl1:8983", replica1.getBaseUrl());
assertEquals("http://baseUrl1:8983/slice1_replica1/", replica1.getCoreUrl());
assertEquals("http://baseUrl1:8983/solr", replica1.getBaseUrl());
assertEquals("http://baseUrl1:8983/solr/slice1_replica1/", replica1.getCoreUrl());
assertEquals(Replica.State.ACTIVE, replica1.getState());
assertEquals(Replica.Type.NRT, replica1.getType());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class ConcurrentCreateRoutedAliasTest extends SolrTestCaseJ4 {
@Before
public void setUp() throws Exception {
super.setUp();
solrCluster = new MiniSolrCloudCluster(4, createTempDir(), buildJettyConfig("/solr"));
solrCluster = new MiniSolrCloudCluster(4, createTempDir(), buildJettyConfig());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public class CreateCollectionCleanupTest extends SolrCloudTestCase {
+ " <solrcloud>\n"
+ " <str name=\"host\">127.0.0.1</str>\n"
+ " <int name=\"hostPort\">${hostPort:8983}</int>\n"
+ " <str name=\"hostContext\">${hostContext:solr}</str>\n"
+ " <int name=\"zkClientTimeout\">${solr.zkclienttimeout:30000}</int>\n"
+ " <bool name=\"genericCoreNodeNames\">${genericCoreNodeNames:true}</bool>\n"
+ " <int name=\"leaderVoteWait\">10000</int>\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ public void testCancelElection() throws Exception {
Thread.sleep(1000);

String urlScheme = zkStateReader.getClusterProperty(URL_SCHEME, "http");
String url1 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr/1", urlScheme) + "/";
String url2 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr/2", urlScheme) + "/";
String url1 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr", urlScheme) + "/1/";
String url2 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr", urlScheme) + "/2/";

assertEquals("original leader was not registered", url1, getLeaderUrl("collection2", "slice1"));

Expand Down
6 changes: 3 additions & 3 deletions solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@
public class NodeMutatorTest extends SolrTestCaseJ4Test {

private static final String NODE3 = "baseUrl3:8985_";
private static final String NODE3_URL = "http://baseUrl3:8985";
private static final String NODE3_URL = "http://baseUrl3:8985/solr";

private static final String NODE2 = "baseUrl2:8984_";
private static final String NODE2_URL = "http://baseUrl2:8984";
private static final String NODE2_URL = "http://baseUrl2:8984/solr";

private static final String NODE1 = "baseUrl1:8983_";
private static final String NODE1_URL = "http://baseUrl1:8983";
private static final String NODE1_URL = "http://baseUrl1:8983/solr";

@Test
public void downNodeReportsAllImpactedCollectionsAndNothingElse() {
Expand Down
4 changes: 2 additions & 2 deletions solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ public void testOverseerStatsReset() throws Exception {
"/admin/cores",
reader,
zkController,
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983, "")
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983)
.setUseDistributedClusterStateUpdates(false)
.setUseDistributedCollectionConfigSetExecution(false)
.build());
Expand Down Expand Up @@ -1863,7 +1863,7 @@ private SolrZkClient electNewOverseer(String address)
"/admin/cores",
reader,
zkController,
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983, "")
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983)
.setUseDistributedClusterStateUpdates(false)
.build());
overseers.add(overseer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ public void testMigrateSSL(SSLTestConfig sslConfig) throws Exception {
JettySolrRunner runner = jettys.get(i);
JettyConfig config =
JettyConfig.builder()
.setContext(context)
.setPort(runner.getLocalPort())
.stopAtShutdown(false)
.withServlets(getExtraServlets())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ public void testOnDiskOnly() throws Exception {
+ " <solrcloud>"
+ " <str name=\"host\">127.0.0.1</str>"
+ " <int name=\"hostPort\">9045</int>"
+ " <str name=\"hostContext\">${hostContext:solr}</str>"
+ " </solrcloud>"
+ " <shardHandlerFactory name=\"shardHandlerFactory\" class=\"HttpShardHandlerFactory\">"
+ " <int name=\"socketTimeout\">${socketTimeout:120000}</int>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class TestConfigSetsAPIExclusivity extends SolrTestCaseJ4 {
@Before
public void setUp() throws Exception {
super.setUp();
solrCluster = new MiniSolrCloudCluster(1, createTempDir(), buildJettyConfig("/solr"));
solrCluster = new MiniSolrCloudCluster(1, createTempDir(), buildJettyConfig());
}

@Override
Expand Down
Loading
Loading