diff --git a/changelog/unreleased/SOLR-18336-windows-start-wait.yml b/changelog/unreleased/SOLR-18336-windows-start-wait.yml new file mode 100644 index 000000000000..5dfca0d8a1cf --- /dev/null +++ b/changelog/unreleased/SOLR-18336-windows-start-wait.yml @@ -0,0 +1,12 @@ +title: > + Fixed bin/solr.cmd start on Windows waiting for all running Solr instances + instead of only the one being started. Also fixed the wait to connect using + the advertised or bind host instead of always localhost. +type: fixed +authors: + - name: Jan Høydahl + url: https://home.apache.org/phonebook.html?uid=janhoy + - name: Mikael Sterner +links: + - name: SOLR-18336 + url: https://issues.apache.org/jira/browse/SOLR-18336 diff --git a/solr/bin/solr.cmd b/solr/bin/solr.cmd index 743d8fb7cdc3..778509f7e12d 100755 --- a/solr/bin/solr.cmd +++ b/solr/bin/solr.cmd @@ -1178,7 +1178,7 @@ IF "%FG%"=="1" ( "%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% %SOLR_TOOL_OPTS% -Dsolr.install.dir="%SOLR_TIP%" ^ -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^ -classpath "%SOLR_TIP%\lib\*;%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^ - org.apache.solr.cli.SolrCLI status --max-wait-secs !SOLR_START_WAIT! + org.apache.solr.cli.SolrCLI status -p %SOLR_PORT_LISTEN% --max-wait-secs !SOLR_START_WAIT! IF NOT "!ERRORLEVEL!"=="0" ( set "SCRIPT_ERROR=Solr did not start or was not reachable. Check the logs for errors." goto err diff --git a/solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java b/solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java index 3001fd2afd53..fe4322a80714 100644 --- a/solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java +++ b/solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java @@ -34,6 +34,7 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -41,6 +42,8 @@ import org.apache.lucene.util.Constants; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.EnvUtils; +import org.apache.solr.common.util.TimeSource; +import org.apache.solr.util.TimeOut; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,7 +76,14 @@ public SolrProcessManager() { ProcessHandle::pid, ph -> new SolrProcess( - ph.pid(), parsePortFromProcess(ph).orElseThrow(), isProcessSsl(ph)))); + ph.pid(), + parseSyspropFromProcess(ph, "solr.port.listen") + .map(Integer::parseInt) + .orElseThrow(), + isProcessSsl(ph), + localConnectHost( + parseSyspropFromProcess(ph, "solr.host.advertise"), + parseSyspropFromProcess(ph, "solr.host.bind"))))); portProcessMap = pidProcessMap.values().stream().collect(Collectors.toUnmodifiableMap(p -> p.port, p -> p)); String solrInstallDir = EnvUtils.getProperty(SOLR_INSTALL_DIR); @@ -143,13 +153,47 @@ public Collection getAllRunning() { return pidProcessMap.values(); } - private Optional parsePortFromProcess(ProcessHandle ph) { - Optional portStr = - arguments(ph).stream() - .filter(a -> a.contains("-Dsolr.port.listen=")) - .map(s -> s.split("=")[1]) - .findFirst(); - return portStr.isPresent() ? portStr.map(Integer::parseInt) : Optional.empty(); + /** Parses the value of the given system property from the process' command line arguments */ + private static Optional parseSyspropFromProcess(ProcessHandle ph, String sysprop) { + return arguments(ph).stream() + .filter(a -> a.contains("-D" + sysprop + "=")) + .map(s -> s.split("=", 2)[1]) + .findFirst(); + } + + /** + * Returns the process listening on the given port, if found, waiting up to {@code maxWaitSecs} + * for it to appear. A newly started process may not be visible in the process table right away, + * so the table is re-scanned once a second until the deadline. + */ + public Optional waitForProcessOnPort(int port, int maxWaitSecs) + throws InterruptedException { + Optional proc = processForPort(port); + TimeOut timeOut = new TimeOut(maxWaitSecs, TimeUnit.SECONDS, TimeSource.NANO_TIME); + while (proc.isEmpty() && !timeOut.hasTimedOut()) { + timeOut.sleep(1000); + proc = new SolrProcessManager().processForPort(port); + } + return proc; + } + + /** + * Resolves the host to use when connecting locally to a Solr process. The advertised host is + * preferred when set, as that is the name the node is reachable by and, with SSL, the name its + * certificate is issued for. Otherwise the bind host is used if it is a specific non-loopback + * address. Wildcard and loopback binds are reachable as {@code localhost}. IPv6 literals are + * bracketed for use in URLs. + */ + static String localConnectHost(Optional advertiseHost, Optional bindHost) { + String host = + advertiseHost + .map(String::trim) + .filter(h -> !h.isEmpty()) + .orElseGet(() -> bindHost.map(String::trim).orElse("")); + return switch (host) { + case "", "0.0.0.0", "::", "[::]", "127.0.0.1", "::1", "[::1]", "localhost" -> "localhost"; + default -> host.contains(":") && !host.startsWith("[") ? "[" + host + "]" : host; + }; } private boolean isProcessSsl(ProcessHandle ph) { @@ -237,11 +281,15 @@ private static List arguments(ProcessHandle ph) { } } - /** Represents a running Solr process */ - public record SolrProcess(long pid, int port, boolean isHttps) { + /** + * Represents a running Solr process. The {@code host} is the host to use when connecting to the + * process from the local machine, i.e. the advertised host if set, else the bind host if bound to + * a specific address, else {@code localhost}. + */ + public record SolrProcess(long pid, int port, boolean isHttps, String host) { public String getLocalUrl() { - return String.format(Locale.ROOT, "%s://localhost:%s/solr", isHttps ? "https" : "http", port); + return String.format(Locale.ROOT, "%s://%s:%s/solr", isHttps ? "https" : "http", host, port); } } } diff --git a/solr/core/src/java/org/apache/solr/cli/StatusTool.java b/solr/core/src/java/org/apache/solr/cli/StatusTool.java index 27a4840a3a4e..be7ae6755057 100644 --- a/solr/core/src/java/org/apache/solr/cli/StatusTool.java +++ b/solr/core/src/java/org/apache/solr/cli/StatusTool.java @@ -62,7 +62,7 @@ public class StatusTool extends ToolBase { .hasArg() .argName("PORT") .type(Integer.class) - .desc("Port on localhost to check status for") + .desc("Port of a local Solr process to check status for") .get(); public static final Option SHORT_OPTION = @@ -113,7 +113,6 @@ public void runImpl(CommandLine cli) throws Exception { // URL provided, do not consult local processes, as the URL may be remote if (maxWaitSecs > 0) { - // Used by Windows start script when starting Solr try { waitForSolrUpAndPrintStatus(solrUrl, cli, maxWaitSecs); runtime.exit(0); @@ -128,7 +127,8 @@ public void runImpl(CommandLine cli) throws Exception { } if (port != null) { - Optional proc = processMgr.processForPort(port); + // When asked to wait, this also waits for a newly started process to become visible + Optional proc = processMgr.waitForProcessOnPort(port, maxWaitSecs); if (proc.isEmpty()) { CLIO.err("Could not find a running Solr on port " + port); runtime.exit(1); @@ -168,6 +168,7 @@ private void printProcessStatus(SolrProcess process, CommandLine cli) throws Exc CLIO.out(pidUrl); } else { if (maxWaitSecs > 0) { + // Used by Windows start script, which passes the port of the newly started instance waitForSolrUpAndPrintStatus(pidUrl, cli, maxWaitSecs); } else { CLIO.out( diff --git a/solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java b/solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java index 2102e1e3d75d..1e428e900454 100644 --- a/solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java +++ b/solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java @@ -26,9 +26,12 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import org.apache.commons.math3.util.Pair; import org.apache.solr.SolrTestCase; @@ -44,24 +47,34 @@ public class SolrProcessManagerTest extends SolrTestCase { private static SolrProcessManager solrProcessManager; private static Pair processHttp; private static Pair processHttps; + private static Pair processBoundHttp; + private static Pair processAdvertisedHttps; @BeforeClass public static void beforeClass() throws Exception { boolean isWindows = random().nextBoolean(); String PID_SUFFIX = isWindows ? ".port" : ".pid"; log.info("Simulating pid file on {}", isWindows ? "Windows" : "Linux"); - processHttp = createProcess(findAvailablePort(), false); - processHttps = createProcess(findAvailablePort(), true); - long processHttpValue = isWindows ? processHttp.getKey() : processHttp.getValue().pid(); - long processHttpsValue = isWindows ? processHttps.getKey() : processHttps.getValue().pid(); + processHttp = createProcess(findAvailablePort(), false, null, null); + processHttps = createProcess(findAvailablePort(), true, "127.0.0.1", null); + // The mock process does not actually bind to these hosts, they are only command-line markers + processBoundHttp = createProcess(findAvailablePort(), false, "10.99.99.99", null); + processAdvertisedHttps = + createProcess(findAvailablePort(), true, "10.99.99.99", "myhost.example.com"); + for (Pair p : + List.of(processHttp, processHttps, processBoundHttp, processAdvertisedHttps)) { + awaitReady(p.getValue()); + } SolrProcessManager.enableTestingMode = true; System.setProperty("solr.port.listen", Integer.toString(processHttp.getKey())); Path pidDir = createTempDir("solr-pid-dir"); System.setProperty("solr.pid.dir", pidDir.toString()); - Files.writeString( - pidDir.resolve("solr-" + processHttpValue + PID_SUFFIX), Long.toString(processHttpValue)); - Files.writeString( - pidDir.resolve("solr-" + processHttpsValue + PID_SUFFIX), Long.toString(processHttpsValue)); + for (Pair p : + List.of(processHttp, processHttps, processBoundHttp, processAdvertisedHttps)) { + long pidFileValue = isWindows ? p.getKey() : p.getValue().pid(); + Files.writeString( + pidDir.resolve("solr-" + pidFileValue + PID_SUFFIX), Long.toString(pidFileValue)); + } Files.writeString(pidDir.resolve("solr-99999" + PID_SUFFIX), "99999"); // Invalid solrProcessManager = new SolrProcessManager(); } @@ -70,6 +83,8 @@ public static void beforeClass() throws Exception { public static void afterClass() throws Exception { processHttp.getValue().destroyForcibly(); processHttps.getValue().destroyForcibly(); + processBoundHttp.getValue().destroyForcibly(); + processAdvertisedHttps.getValue().destroyForcibly(); SolrProcessManager.enableTestingMode = false; } @@ -80,7 +95,8 @@ private static int findAvailablePort() throws IOException { } @SuppressWarnings("SystemGetProperty") - private static Pair createProcess(int port, boolean https) throws IOException { + private static Pair createProcess( + int port, boolean https, String bindHost, String advertiseHost) throws IOException { // Get the path to the java executable from the current JVM String pathSeparator = System.getProperty("path.separator"); @@ -89,36 +105,78 @@ private static Pair createProcess(int port, boolean https) thr .filter(p -> p.contains("solr") && p.contains("core") && p.contains("build")) .collect(Collectors.joining(pathSeparator)); - ProcessBuilder processBuilder = - new ProcessBuilder( - System.getProperty("java.home") + "/bin/java", - "-Dsolr.port.listen=" + port, - "-DisHttps=" + https, - "-DmockSolr=true", - "-cp", - classPath, - "org.apache.solr.cli.SolrProcessManagerTest$MockSolrProcess", - https ? "--module=https" : "--module=http"); - - // Start the process and read first line of output - Process process = processBuilder.start(); + List command = new ArrayList<>(); + command.add(System.getProperty("java.home") + "/bin/java"); + command.add("-Dsolr.port.listen=" + port); + command.add("-DisHttps=" + https); + command.add("-DmockSolr=true"); + if (bindHost != null) { + command.add("-Dsolr.host.bind=" + bindHost); + } + if (advertiseHost != null) { + command.add("-Dsolr.host.advertise=" + advertiseHost); + } + command.add("-cp"); + command.add(classPath); + command.add("org.apache.solr.cli.SolrProcessManagerTest$MockSolrProcess"); + command.add(https ? "--module=https" : "--module=http"); + return new Pair<>(port, new ProcessBuilder(command).start()); + } + + /** Waits for the mock process to print its ready line, so the processes can start in parallel */ + private static void awaitReady(Process process) throws IOException { try (InputStream is = process.getInputStream(); InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) { System.out.println(br.readLine()); } - return new Pair<>(port, process); } public void testGetLocalUrl() { assertFalse(solrProcessManager.getAllRunning().isEmpty()); - solrProcessManager - .getAllRunning() - .forEach( - p -> - assertEquals( - (p.isHttps() ? "https" : "http") + "://localhost:" + p.port() + "/solr", - p.getLocalUrl())); + SolrProcess http = solrProcessManager.processForPort(processHttp.getKey()).orElseThrow(); + assertEquals("http://localhost:" + http.port() + "/solr", http.getLocalUrl()); + SolrProcess https = solrProcessManager.processForPort(processHttps.getKey()).orElseThrow(); + assertEquals("https://localhost:" + https.port() + "/solr", https.getLocalUrl()); + // Non-loopback bind host is used for the local URL + SolrProcess bound = solrProcessManager.processForPort(processBoundHttp.getKey()).orElseThrow(); + assertEquals("http://10.99.99.99:" + bound.port() + "/solr", bound.getLocalUrl()); + // Advertised host wins over the bind host + SolrProcess advertised = + solrProcessManager.processForPort(processAdvertisedHttps.getKey()).orElseThrow(); + assertEquals( + "https://myhost.example.com:" + advertised.port() + "/solr", advertised.getLocalUrl()); + } + + public void testLocalConnectHost() { + // No advertise host: bind host decides, loopback and wildcard binds map to localhost + assertEquals("localhost", localConnectHost(null, null)); + assertEquals("localhost", localConnectHost(null, "")); + assertEquals("localhost", localConnectHost(null, "0.0.0.0")); + assertEquals("localhost", localConnectHost(null, "::")); + assertEquals("localhost", localConnectHost(null, "127.0.0.1")); + assertEquals("localhost", localConnectHost(null, "::1")); + assertEquals("localhost", localConnectHost(null, "localhost")); + assertEquals("10.0.0.5", localConnectHost(null, "10.0.0.5")); + assertEquals("myhost.example.com", localConnectHost(null, "myhost.example.com")); + assertEquals("[fe80::1]", localConnectHost(null, "fe80::1")); + // Advertise host is preferred over the bind host when set + assertEquals("myhost.example.com", localConnectHost("myhost.example.com", "10.0.0.5")); + assertEquals("myhost.example.com", localConnectHost("myhost.example.com", null)); + assertEquals("localhost", localConnectHost("localhost", "10.0.0.5")); + // Blank advertise host falls back to the bind host + assertEquals("10.0.0.5", localConnectHost("", "10.0.0.5")); + assertEquals("10.0.0.5", localConnectHost(" ", "10.0.0.5")); + } + + private static String localConnectHost(String advertiseHost, String bindHost) { + return SolrProcessManager.localConnectHost( + Optional.ofNullable(advertiseHost), Optional.ofNullable(bindHost)); + } + + public void testWaitForProcessOnPort() throws Exception { + assertTrue(solrProcessManager.waitForProcessOnPort(processHttp.getKey(), 0).isPresent()); + assertTrue(solrProcessManager.waitForProcessOnPort(0, 0).isEmpty()); } public void testIsRunningWithPort() { @@ -153,12 +211,12 @@ public void testGetProcessForPid() { public void testScanSolrPidFiles() throws IOException { Collection processes = solrProcessManager.scanSolrPidFiles(); - assertEquals(2, processes.size()); + assertEquals(4, processes.size()); } public void testGetAllRunning() { Collection processes = solrProcessManager.getAllRunning(); - assertEquals(2, processes.size()); + assertEquals(4, processes.size()); } public void testSolrProcessMethods() {