Skip to content

Fix leaking tests in HealthPeriodicLoggerTests #134295

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

Merged
merged 2 commits into from
Sep 8, 2025
+23 −23
Merged

Fix leaking tests in HealthPeriodicLoggerTests #134295

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
7b3ba66
Fix leaking tests in `HealthPeriodicLoggerTests`
nielsbauman Sep 6, 2025
623f28a
Remove debug log
nielsbauman Sep 8, 2025
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
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,6 @@ tests:
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
method: test {csv-spec:inlinestats.MultiIndexInlinestatsOfMultiTypedField}
issue: https://github.com/elastic/elasticsearch/issues/133973
- class: org.elasticsearch.health.HealthPeriodicLoggerTests
method: testOutputModeNoLogging
issue: https://github.com/elastic/elasticsearch/issues/134200
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
method: test {csv-spec:spatial_shapes.ConvertFromStringParseError}
issue: https://github.com/elastic/elasticsearch/issues/134254
Expand Down
46 changes: 26 additions & 20 deletions server/src/test/java/org/elasticsearch/health/HealthPeriodicLoggerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,23 +436,24 @@ public void testTryToLogHealthConcurrencyControlWithResults() throws Exception {
SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);

// run it once, verify getHealth is called
{
Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
logHealthThread.start();
// We wait to verify that the triggered even is in progress, then we block, so it will rename in progress
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
// We try to log again while it's in progress, we expect this run to be skipped
assertFalse(testHealthPeriodicLogger.tryToLogHealth());
// Unblock the first execution
waitForSecondRun.countDown();
}
Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
logHealthThread.start();
// We wait to verify that the triggered even is in progress, then we block, so it will rename in progress
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
// We try to log again while it's in progress, we expect this run to be skipped
assertFalse(testHealthPeriodicLogger.tryToLogHealth());
// Unblock the first execution
waitForSecondRun.countDown();

// run it again, verify getHealth is called, because we are calling the results listener
{
waitForRelease.await();
testHealthPeriodicLogger.triggered(event);
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(2)));
}

// Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
logHealthThread.join();
}

public void testTryToLogHealthConcurrencyControl() throws Exception {
Expand Down Expand Up @@ -485,11 +486,9 @@ public void testTryToLogHealthConcurrencyControl() throws Exception {
SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);

// call it and verify that getHealth is called
{
Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
logHealthThread.start();
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
}
Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
logHealthThread.start();
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));

// run it again, verify that it's skipped because the other one is in progress
{
Expand All @@ -504,6 +503,9 @@ public void testTryToLogHealthConcurrencyControl() throws Exception {
testHealthPeriodicLogger.triggered(event);
assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(2)));
}

// Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
logHealthThread.join();
}

public void testTryToLogHealthConcurrencyControlWithException() throws Exception {
Expand Down Expand Up @@ -609,11 +611,12 @@ public void testClosingWhenRunInProgress() throws Exception {
SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);

// call it and verify that getHealth is called
{
Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
logHealthThread.start();
assertBusy(() -> assertTrue(testHealthPeriodicLogger.currentlyRunning()));
}
Thread logHealthThread = new Thread(() -> {
testHealthPeriodicLogger.triggered(event);
logger.info("HALLO");
});
logHealthThread.start();
assertBusy(() -> assertTrue(testHealthPeriodicLogger.currentlyRunning()));

// stop and close it
{
Expand All @@ -626,6 +629,9 @@ public void testClosingWhenRunInProgress() throws Exception {
waitForCloseToBeTriggered.countDown();
assertBusy(() -> assertEquals(Lifecycle.State.CLOSED, testHealthPeriodicLogger.lifecycleState()));
}

// Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
logHealthThread.join();
}
}

Expand Down