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.

Sign up for GitHub

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

Use pid to identify die-with-dignity process #77385

Closed
wants to merge 2 commits into from
Closed

Use pid to identify die-with-dignity process #77385

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
960d8f9
Use pid identify die-with-dignity process
rjernst Sep 7, 2021
4b6363f
spotless
rjernst Sep 7, 2021
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
6 changes: 6 additions & 0 deletions build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,12 @@ public boolean isProcessAlive() {
return esProcess.isAlive();
}

@Internal
public long getPid() {
requireNonNull(esProcess, "Can't get pid of " + this + " as it's not started. Does the task have `useCluster` ?");
return esProcess.pid();
}

void waitForAllConditions() {
waitForConditions(waitConditions, System.currentTimeMillis(), NODE_UP_TIMEOUT_UNIT.toMillis(NODE_UP_TIMEOUT) +
// Installing plugins at config time and loading them when nods start requires additional time we need to
Expand Down
1 change: 1 addition & 0 deletions test/external-modules/die-with-dignity/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ tasks.named("javaRestTest").configure {
systemProperty 'tests.security.manager', 'false'
systemProperty 'tests.system_call_filter', 'false'
nonInputProperties.systemProperty 'log', "${-> testClusters.javaRestTest.singleNode().getServerLog()}"
nonInputProperties.systemProperty 'es.pid', "${ -> testClusters.javaRestTest.singleNode().getPid() }"
systemProperty 'runtime.java.home', BuildParams.runtimeJavaHome
}

Expand Down
49 changes: 19 additions & 30 deletions ...dignity/src/javaRestTest/java/org/elasticsearch/qa/die_with_dignity/DieWithDignityIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.qa.die_with_dignity;

import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.client.Request;
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.test.rest.ESRestTestCase;
Expand All @@ -22,44 +21,18 @@
import java.util.Iterator;
import java.util.List;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;

@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/77282")
public class DieWithDignityIT extends ESRestTestCase {

public void testDieWithDignity() throws Exception {
// there should be an Elasticsearch process running with the die.with.dignity.test system property
{
final String jpsPath = PathUtils.get(System.getProperty("runtime.java.home"), "bin/jps").toString();
final Process process = new ProcessBuilder().command(jpsPath, "-v").start();

boolean found = false;
try (InputStream is = process.getInputStream(); BufferedReader in = new BufferedReader(new InputStreamReader(is, "UTF-8"))) {
String line;
while ((line = in.readLine()) != null) {
if (line.contains("-Ddie.with.dignity.test=true")) {
found = true;
break;
}
}
}
assertTrue(found);
}
String esPid = System.getProperty("es.pid");
assertTrue(javaPidExists(esPid));

expectThrows(IOException.class, () -> client().performRequest(new Request("GET", "/_die_with_dignity")));

// the Elasticsearch process should die and disappear from the output of jps
assertBusy(() -> {
final String jpsPath = PathUtils.get(System.getProperty("runtime.java.home"), "bin/jps").toString();
final Process process = new ProcessBuilder().command(jpsPath, "-v").start();

try (InputStream is = process.getInputStream(); BufferedReader in = new BufferedReader(new InputStreamReader(is, "UTF-8"))) {
String line;
while ((line = in.readLine()) != null) {
assertThat(line, line, not(containsString("-Ddie.with.dignity.test=true")));
}
}
assertFalse(javaPidExists(esPid));
});

// parse the logs and ensure that Elasticsearch died with the expected cause
Expand Down Expand Up @@ -95,6 +68,22 @@ public void testDieWithDignity() throws Exception {
}
}

private boolean javaPidExists(String expectedPid) throws IOException {
final String jpsPath = PathUtils.get(System.getProperty("runtime.java.home"), "bin/jps").toString();
final Process process = new ProcessBuilder().command(jpsPath, "-v").start();
Copy link
Member

Choose a reason for hiding this comment

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

No need for verbose output if all we're checking for is the PID. You can use -q instead for only PID output. That will simplify the parsing below too.


try (InputStream is = process.getInputStream(); BufferedReader in = new BufferedReader(new InputStreamReader(is, "UTF-8"))) {
String line;
while ((line = in.readLine()) != null) {
String foundPid = line.split(" ")[0];
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified after you switch to using quiet output.

if (expectedPid.equals(foundPid)) {
return true;
}
}
}
return false;
}

private boolean containsAll(String line, String... subStrings) {
for (String subString : subStrings) {
if (line.matches(subString) == false) {
Expand Down