This is an automated email from the ASF dual-hosted git repository.
jinwoo pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new a80762516f GEODE-9478: Fix status --dir to use file controller (#6737)
a80762516f is described below
commit a80762516f55b91b5967e722b7a5fbb5755744f6
Author: Mario Salazar de Torres <[email protected]>
AuthorDate: Tue Oct 21 20:26:04 2025 +0200
GEODE-9478: Fix status --dir to use file controller (#6737)
* GEODE-9478: Fix status --dir to use file controller
- Previously when you only specified --dir option the PID was read from
the member workDir and the status request was attempted to solved by
using the attachment API, and after that JMX interface.
But given only --dir was specified the controller resolving the
request should be FileProcessController instead.
- Logic has been changed for both servers and locators to always use
FileProcessConroller whenever only --dir flag is specified.
- Added an UT to verify new code.
- Modified several ITs to verify the new behaviour.
- Deleted the following ITs which no longer apply with the new logic:
* statusWithEmptyPidFileThrowsIllegalArgumentException
* statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails
* statusWithStalePidFileReturnsNotResponding
* GEODE-9478: Revision 1
- Removed throws in javadoc given new constructors doesn't have any PID
---------
Co-authored-by: Mario Salazar de Torres <[email protected]>
---
.../LocatorLauncherLocalIntegrationTest.java | 62 +++-------------------
.../LocatorLauncherRemoteIntegrationTest.java | 61 +++------------------
.../ServerLauncherLocalIntegrationTest.java | 62 +++-------------------
.../ServerLauncherRemoteIntegrationTest.java | 59 +++-----------------
.../ProcessControllerFactoryIntegrationTest.java | 14 +++++
.../apache/geode/distributed/LocatorLauncher.java | 35 ++++--------
.../apache/geode/distributed/ServerLauncher.java | 30 ++++-------
.../internal/process/FileProcessController.java | 27 ++++++++++
.../internal/process/ProcessControllerFactory.java | 9 ++++
.../process/FileProcessControllerTest.java | 9 ++++
10 files changed, 112 insertions(+), 256 deletions(-)
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
index 8f07f1af01..c676ab4323 100755
---
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
@@ -14,8 +14,6 @@
*/
package org.apache.geode.distributed;
-import static java.lang.management.ManagementFactory.getRuntimeMXBean;
-import static
org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
import static
org.apache.geode.distributed.ConfigurationProperties.DISABLE_AUTO_RECONNECT;
@@ -217,6 +215,13 @@ public class LocatorLauncherLocalIntegrationTest extends
LocatorLauncherIntegrat
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws
Exception {
givenRunningLocator();
+ // Keep it as we are going to overwrite it
+ int pid = readPidFile();
+
+ // Sets a fake PID to ensure that the request is going through
FileProcessController,
+ // because if not, the request would fail due to the PID not existing
+ givenPidFile(fakePid);
+
LocatorState locatorState = new Builder()
.setWorkingDirectory(getWorkingDirectoryPath())
.build()
@@ -230,62 +235,11 @@ public class LocatorLauncherLocalIntegrationTest extends
LocatorLauncherIntegrat
assertThat(locatorState.getJvmArguments()).isEqualTo(getJvmArguments());
assertThat(locatorState.getLogFile()).isEqualTo(getLogFilePath());
assertThat(locatorState.getMemberName()).isEqualTo(getUniqueName());
- assertThat(locatorState.getPid().intValue()).isEqualTo(readPidFile());
+ assertThat(locatorState.getPid().intValue()).isEqualTo(pid);
assertThat(locatorState.getUptime()).isGreaterThan(0);
assertThat(locatorState.getWorkingDirectory()).isEqualTo(new
File(".").getCanonicalPath());
}
- @Test
- public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
- givenEmptyPidFile();
- LocatorLauncher launcher = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build();
-
- Throwable thrown = catchThrowable(launcher::status);
-
- assertThat(thrown)
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Invalid pid 'null' found in");
- }
-
- @Test
- public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails()
throws Exception {
- givenEmptyWorkingDirectory();
-
- LocatorState locatorState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
- assertThat(locatorState.getClasspath()).isNull();
-
assertThat(locatorState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
-
assertThat(locatorState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
-
assertThat(locatorState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
-
assertThat(locatorState.getJvmArguments()).isEqualTo(getRuntimeMXBean().getInputArguments());
- assertThat(locatorState.getLogFile()).isNull();
- assertThat(locatorState.getMemberName()).isNull();
- assertThat(locatorState.getPid()).isNull();
- assertThat(locatorState.getUptime().intValue()).isEqualTo(0);
-
assertThat(locatorState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
- }
-
- /**
- * This test takes > 1 minute to run in {@link
LocatorLauncherLocalFileIntegrationTest}.
- */
- @Test
- public void statusWithStalePidFileReturnsNotResponding() {
- givenPidFile(fakePid);
-
- LocatorState locatorState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
- }
-
@Test
public void stopWithPidReturnsStopped() {
givenRunningLocator();
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
index cc195055a5..65c16511fc 100755
---
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
@@ -14,12 +14,10 @@
*/
package org.apache.geode.distributed;
-import static
org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
import static org.apache.geode.internal.inet.LocalHostUtil.getLocalHost;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.catchThrowable;
import java.io.File;
import java.net.BindException;
@@ -149,6 +147,13 @@ public class LocatorLauncherRemoteIntegrationTest extends
LocatorLauncherRemoteI
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws
Exception {
givenRunningLocator(withPort(0));
+ // Keep it as we are going to overwrite it
+ int pid = readPidFile();
+
+ // Sets a fake PID to ensure that the request is going through
FileProcessController,
+ // because if not, the request would fail due to the PID not existing
+ givenPidFile(fakePid);
+
LocatorState locatorState = new Builder()
.setWorkingDirectory(getWorkingDirectoryPath())
.build()
@@ -161,62 +166,12 @@ public class LocatorLauncherRemoteIntegrationTest extends
LocatorLauncherRemoteI
assertThat(locatorState.getJvmArguments()).isEqualTo(getJvmArguments());
assertThat(locatorState.getLogFile()).isEqualTo(getLogFile().getCanonicalPath());
assertThat(locatorState.getMemberName()).isEqualTo(getUniqueName());
- assertThat(locatorState.getPid().intValue()).isEqualTo(readPidFile());
+ assertThat(locatorState.getPid().intValue()).isEqualTo(pid);
assertThat(locatorState.getStatus()).isEqualTo(ONLINE);
assertThat(locatorState.getUptime()).isGreaterThan(0);
assertThat(locatorState.getWorkingDirectory()).isEqualToIgnoringCase(getWorkingDirectoryPath());
}
- @Test
- public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
- givenEmptyPidFile();
- LocatorLauncher launcher = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build();
-
- Throwable thrown = catchThrowable(launcher::status);
-
- assertThat(thrown)
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Invalid pid 'null' found in");
- }
-
- @Test
- public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails()
throws Exception {
- givenEmptyWorkingDirectory();
-
- LocatorState locatorState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(locatorState.getClasspath()).isNull();
-
assertThat(locatorState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
-
assertThat(locatorState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
-
assertThat(locatorState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
- assertThat(locatorState.getLogFile()).isNull();
- assertThat(locatorState.getMemberName()).isNull();
- assertThat(locatorState.getPid()).isNull();
- assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
- assertThat(locatorState.getUptime().intValue()).isEqualTo(0);
-
assertThat(locatorState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
- }
-
- /**
- * This test takes > 1 minute to run in {@link
LocatorLauncherRemoteFileIntegrationTest}.
- */
- @Test
- public void statusWithStalePidFileReturnsNotResponding() {
- givenPidFile(fakePid);
-
- LocatorState locatorState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
- }
-
@Test
public void stopWithPidReturnsStopped() {
givenRunningLocator(withPort(0));
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java
index dd953e9f37..0d0c759f8c 100755
---
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.distributed;
-import static
org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
import static
org.apache.geode.distributed.ConfigurationProperties.DISABLE_AUTO_RECONNECT;
@@ -308,6 +307,13 @@ public class ServerLauncherLocalIntegrationTest extends
ServerLauncherLocalInteg
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws
UnknownHostException {
givenRunningServer();
+ // Keep it as we are going to overwrite it
+ int pid = readPidFile();
+
+ // Sets a fake PID to ensure that the request is going through
FileProcessController,
+ // because if not, the request would fail due to the PID not existing
+ givenPidFile(fakePid);
+
ServerState serverState = new Builder()
.setWorkingDirectory(getWorkingDirectoryPath())
.build()
@@ -320,64 +326,12 @@ public class ServerLauncherLocalIntegrationTest extends
ServerLauncherLocalInteg
assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
assertThat(serverState.getLogFile()).isEqualTo(getLogFilePath());
assertThat(serverState.getMemberName()).isEqualTo(getUniqueName());
- assertThat(serverState.getPid().intValue()).isEqualTo(readPidFile());
+ assertThat(serverState.getPid().intValue()).isEqualTo(pid);
assertThat(serverState.getStatus()).isEqualTo(ONLINE);
assertThat(serverState.getUptime()).isGreaterThan(0);
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
}
- @Test
- public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
- givenEmptyPidFile();
- ServerLauncher launcher = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build();
-
- Throwable thrown = catchThrowable(launcher::status);
-
- assertThat(thrown)
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Invalid pid 'null' found in");
- }
-
- @Test
- public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails()
- throws UnknownHostException {
- givenEmptyWorkingDirectory();
-
- ServerState serverState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(serverState.getClasspath()).isNull();
-
assertThat(serverState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
-
assertThat(serverState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
-
assertThat(serverState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
- assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
- assertThat(serverState.getLogFile()).isNull();
- assertThat(serverState.getMemberName()).isNull();
- assertThat(serverState.getPid()).isNull();
- assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
- assertThat(serverState.getUptime().intValue()).isEqualTo(0);
-
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
- }
-
- /**
- * This test takes > 1 minute to run in {@link
ServerLauncherLocalFileIntegrationTest}.
- */
- @Test
- public void statusWithStalePidFileReturnsNotResponding() {
- givenPidFile(fakePid);
-
- ServerState serverState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
- }
-
@Test
public void stopWithPidReturnsStopped() {
givenRunningServer();
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
index 7456c8de4e..c0d42f2971 100755
---
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
@@ -14,14 +14,12 @@
*/
package org.apache.geode.distributed;
-import static
org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
import static
org.apache.geode.distributed.ConfigurationProperties.CACHE_XML_FILE;
import static org.apache.geode.internal.inet.LocalHostUtil.getLocalHost;
import static org.apache.geode.util.internal.GeodeGlossary.GEMFIRE_PREFIX;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.catchThrowable;
import java.io.File;
import java.io.IOException;
@@ -210,13 +208,20 @@ public class ServerLauncherRemoteIntegrationTest extends
ServerLauncherRemoteInt
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws
IOException {
givenRunningServer();
+ // Keep it as we are going to overwrite it
+ int pid = readPidFile();
+
+ // Sets a fake PID to ensure that the request is going through
FileProcessController,
+ // because if not, the request would fail due to the PID not existing
+ givenPidFile(fakePid);
+
ServerState serverState = new Builder()
.setWorkingDirectory(getWorkingDirectoryPath())
.build()
.status();
assertThat(serverState.getStatus()).isEqualTo(ONLINE);
- assertThat(serverState.getPid().intValue()).isEqualTo(readPidFile());
+ assertThat(serverState.getPid().intValue()).isEqualTo(pid);
assertThat(serverState.getUptime()).isGreaterThan(0);
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
@@ -228,54 +233,6 @@ public class ServerLauncherRemoteIntegrationTest extends
ServerLauncherRemoteInt
assertThat(serverState.getMemberName()).isEqualTo(getUniqueName());
}
- @Test
- public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
- givenEmptyPidFile();
-
- ServerLauncher launcher = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build();
-
- Throwable thrown = catchThrowable(launcher::status);
-
- assertThat(thrown)
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Invalid pid 'null' found in");
- }
-
- @Test
- public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails()
throws IOException {
- givenEmptyWorkingDirectory();
-
- ServerState serverState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
- assertThat(serverState.getPid()).isNull();
- assertThat(serverState.getUptime().intValue()).isEqualTo(0);
-
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
- assertThat(serverState.getClasspath()).isNull();
-
assertThat(serverState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
-
assertThat(serverState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
- assertThat(serverState.getLogFile()).isNull();
-
assertThat(serverState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
- assertThat(serverState.getMemberName()).isNull();
- }
-
- @Test
- public void statusWithStalePidFileReturnsNotResponding() {
- givenPidFile(fakePid);
-
- ServerState serverState = new Builder()
- .setWorkingDirectory(getWorkingDirectoryPath())
- .build()
- .status();
-
- assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
- }
-
@Test
public void stopWithPidReturnsStopped() {
givenRunningServer();
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/internal/process/ProcessControllerFactoryIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/internal/process/ProcessControllerFactoryIntegrationTest.java
index 9faf65176a..ff3a2182c8 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/internal/process/ProcessControllerFactoryIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/internal/process/ProcessControllerFactoryIntegrationTest.java
@@ -19,6 +19,7 @@ import static
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import java.io.File;
import java.io.FileNotFoundException;
@@ -83,6 +84,19 @@ public class ProcessControllerFactoryIntegrationTest {
assertThat(controller).isInstanceOf(MBeanOrFileProcessController.class);
}
+ @Test
+ public void createProcessController_returnsFileProcessController() throws
Exception {
+ // arrange
+ when(parameters.getDirectory()).thenReturn(new File("./"));
+
+ // act
+ ProcessController controller =
+ factory.createProcessController(parameters);
+
+ // assert
+ assertThat(controller).isInstanceOf(FileProcessController.class);
+ }
+
@Test
public void
createProcessController_withoutAttachAPI_returnsFileProcessController()
throws Exception {
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
b/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
index 68a22cf882..1a127d54c8 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
@@ -81,6 +81,7 @@ import org.apache.geode.internal.process.ControllableProcess;
import org.apache.geode.internal.process.FileAlreadyExistsException;
import org.apache.geode.internal.process.FileControllableProcess;
import org.apache.geode.internal.process.MBeanInvocationFailedException;
+import org.apache.geode.internal.process.PidFile;
import org.apache.geode.internal.process.PidUnavailableException;
import org.apache.geode.internal.process.ProcessController;
import org.apache.geode.internal.process.ProcessControllerFactory;
@@ -1073,44 +1074,30 @@ public class LocatorLauncher extends
AbstractLauncher<String> {
}
private LocatorState statusWithWorkingDirectory() {
- int parsedPid = 0;
try {
- final ProcessController controller =
- new
ProcessControllerFactory().createProcessController(controllerParameters,
- new File(getWorkingDirectory()),
ProcessType.LOCATOR.getPidFileName());
- parsedPid = controller.getProcessId();
+ new PidFile(new File(getWorkingDirectory()),
ProcessType.LOCATOR.getPidFileName());
- // note: in-process request will go infinite loop unless we do the
following
- if (parsedPid == ProcessUtils.identifyPid()) {
- LocatorLauncher runningLauncher = getInstance();
- if (runningLauncher != null) {
- return runningLauncher.status();
- }
- }
+ final ProcessController controller =
+ new
ProcessControllerFactory().createProcessController(this.controllerParameters);
final String statusJson = controller.status();
return LocatorState.fromJson(statusJson);
- } catch (ConnectionFailedException handled) {
- // failed to attach to locator JVM
- return createNoResponseState(handled,
- "Failed to connect to locator with process id " + parsedPid);
} catch (FileNotFoundException handled) {
// could not find pid file
return createNoResponseState(handled, "Failed to find process file "
+ ProcessType.LOCATOR.getPidFileName() + " in " +
getWorkingDirectory());
- } catch (IOException | MBeanInvocationFailedException |
UnableToControlProcessException
- | TimeoutException handled) {
+ } catch (IOException | UnableToControlProcessException | TimeoutException
handled) {
return createNoResponseState(handled,
- "Failed to communicate with locator with process id " + parsedPid);
- } catch (PidUnavailableException e) {
- // couldn't determine pid from within locator JVM
- return createNoResponseState(e, "Failed to find usable process id within
file "
- + ProcessType.LOCATOR.getPidFileName() + " in " +
getWorkingDirectory());
+ "Failed to communicate with locator");
} catch (InterruptedException handled) {
Thread.currentThread().interrupt();
return createNoResponseState(handled,
- "Interrupted while trying to communicate with locator with process
id " + parsedPid);
+ "Interrupted while trying to communicate with locator");
+ } catch (MBeanInvocationFailedException | ConnectionFailedException e) {
+ // This would never happen as controller will always be
FileProcessController
}
+
+ return null;
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
b/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
index 257ad60953..a541c5df4a 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
@@ -88,6 +88,7 @@ import org.apache.geode.internal.process.ControllableProcess;
import org.apache.geode.internal.process.FileAlreadyExistsException;
import org.apache.geode.internal.process.FileControllableProcess;
import org.apache.geode.internal.process.MBeanInvocationFailedException;
+import org.apache.geode.internal.process.PidFile;
import org.apache.geode.internal.process.PidUnavailableException;
import org.apache.geode.internal.process.ProcessController;
import org.apache.geode.internal.process.ProcessControllerFactory;
@@ -1194,30 +1195,19 @@ public class ServerLauncher extends
AbstractLauncher<String> {
private ServerState statusWithWorkingDirectory() {
int parsedPid = 0;
try {
- final ProcessController controller =
- new
ProcessControllerFactory().createProcessController(controllerParameters,
- new File(getWorkingDirectory()),
ProcessType.SERVER.getPidFileName());
- parsedPid = controller.getProcessId();
+ // This will ensure that if folder is not correct it will fail ASAP
+ new PidFile(new File(getWorkingDirectory()),
ProcessType.SERVER.getPidFileName());
- // note: in-process request will go infinite loop unless we do the
following
- if (parsedPid == identifyPid()) {
- final ServerLauncher runningLauncher = getInstance();
- if (runningLauncher != null) {
- return runningLauncher.statusInProcess();
- }
- }
+ final ProcessController controller =
+ new
ProcessControllerFactory().createProcessController(this.controllerParameters);
final String statusJson = controller.status();
return ServerState.fromJson(statusJson);
- } catch (ConnectionFailedException handled) {
- // failed to attach to server JVM
- return createNoResponseState(handled,
- "Failed to connect to server with process id " + parsedPid);
} catch (FileNotFoundException handled) {
// could not find pid file
return createNoResponseState(handled, "Failed to find process file "
+ ProcessType.SERVER.getPidFileName() + " in " +
getWorkingDirectory());
- } catch (IOException | MBeanInvocationFailedException |
UnableToControlProcessException
+ } catch (IOException | UnableToControlProcessException
| TimeoutException handled) {
return createNoResponseState(handled,
"Failed to communicate with server with process id " + parsedPid);
@@ -1225,11 +1215,11 @@ public class ServerLauncher extends
AbstractLauncher<String> {
Thread.currentThread().interrupt();
return createNoResponseState(handled,
"Interrupted while trying to communicate with server with process id
" + parsedPid);
- } catch (PidUnavailableException handled) {
- // couldn't determine pid from within server JVM
- return createNoResponseState(handled, "Failed to find usable process id
within file "
- + ProcessType.SERVER.getPidFileName() + " in " +
getWorkingDirectory());
+ } catch (MBeanInvocationFailedException | ConnectionFailedException e) {
+ // This would never happen as controller will always be
FileProcessController
}
+
+ return null;
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
b/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
index b0d9769583..9a6bb9cb2a 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
@@ -77,6 +77,33 @@ class FileProcessController implements ProcessController {
statusTimeoutMillis = units.toMillis(timeout);
}
+ /**
+ * Constructs an instance for controlling a local process.
+ *
+ * @param parameters details about the controllable process
+ */
+ FileProcessController(final FileControllerParameters parameters) {
+ this(parameters, DEFAULT_STATUS_TIMEOUT_MILLIS, MILLISECONDS);
+ }
+
+ /**
+ * Constructs an instance for controlling a local process.
+ *
+ * @param parameters details about the controllable process
+ * @param timeout the timeout that operations must complete within
+ * @param units the units of the timeout
+ */
+ FileProcessController(final FileControllerParameters parameters,
+ final long timeout, final TimeUnit units) {
+ notNull(parameters, "Invalid parameters '" + parameters + "' specified");
+ isTrue(timeout >= 0, "Invalid timeout '" + timeout + "' specified");
+ notNull(units, "Invalid units '" + units + "' specified");
+
+ this.pid = 0;
+ this.parameters = parameters;
+ this.statusTimeoutMillis = units.toMillis(timeout);
+ }
+
@Override
public int getProcessId() {
return pid;
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/process/ProcessControllerFactory.java
b/geode-core/src/main/java/org/apache/geode/internal/process/ProcessControllerFactory.java
index f6647abfda..211e333e46 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/process/ProcessControllerFactory.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/process/ProcessControllerFactory.java
@@ -59,6 +59,15 @@ public class ProcessControllerFactory {
return new MBeanOrFileProcessController(parameters, pid);
}
+ public ProcessController createProcessController(final
ProcessControllerParameters parameters)
+ throws IOException, InterruptedException, TimeoutException {
+ notNull(parameters, "Invalid parameters '" + parameters + "' specified");
+ notNull(parameters.getDirectory(),
+ "Invalid workDir '" + parameters.getDirectory() + "' specified");
+
+ return new FileProcessController(parameters);
+ }
+
public ProcessController createProcessController(final
ProcessControllerParameters parameters,
final File directory, final String pidFileName)
throws IOException, InterruptedException, TimeoutException {
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerTest.java
b/geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerTest.java
index adaa02db2a..da7af1960a 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerTest.java
@@ -49,6 +49,15 @@ public class FileProcessControllerTest {
controller = new FileProcessController(mockParameters, pid, timeout,
units);
}
+ @Test
+ public void noParamsSpecified_throwsNullPointerException() throws Exception {
+ pid = 0;
+
+ assertThatThrownBy(() -> new FileProcessController(null))
+ .isInstanceOf(NullPointerException.class)
+ .hasMessage("Invalid parameters 'null' specified");
+ }
+
@Test
public void pidLessThanOne_throwsIllegalArgumentException() throws Exception
{
pid = 0;
