epugh commented on code in PR #4710:
URL: https://github.com/apache/solr/pull/4710#discussion_r3729317454
##########
solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java:
##########
@@ -73,7 +76,14 @@ public SolrProcessManager() {
ProcessHandle::pid,
ph ->
new SolrProcess(
- ph.pid(), parsePortFromProcess(ph).orElseThrow(),
isProcessSsl(ph))));
+ ph.pid(),
Review Comment:
SolrProcessManager continues to grow!
##########
solr/core/src/java/org/apache/solr/cli/StatusTool.java:
##########
@@ -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
Review Comment:
I wonder if at this point we should just make the change in `solr.sh` too?
i hate adding in more "only used by windows". Back int he day I _knew_ all
the java code was the same, and it was only the `solr.cmd` or `solr.sh` that
had differences.. Maybe that was a false assumption. But now we are adding
more custom logic. Let's just make solr.sh work the same???
##########
solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java:
##########
@@ -143,13 +153,47 @@ public Collection<SolrProcess> getAllRunning() {
return pidProcessMap.values();
}
- private Optional<Integer> parsePortFromProcess(ProcessHandle ph) {
- Optional<String> 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<String> 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<SolrProcess> waitForProcessOnPort(int port, int maxWaitSecs)
+ throws InterruptedException {
+ Optional<SolrProcess> 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<String> advertiseHost,
Optional<String> bindHost) {
+ String host =
+ advertiseHost
+ .map(String::trim)
+ .filter(h -> !h.isEmpty())
+ .orElseGet(() -> bindHost.map(String::trim).orElse(""));
+ return switch (host) {
Review Comment:
wow... this is pretty.
##########
solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java:
##########
@@ -237,11 +281,15 @@ private static List<String> 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) {
Review Comment:
we like `record` these days!
--
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]
