Skip to content

Commit d6aba55

Browse files
authored
Simplify LocalExporter cleaner function to fix failing tests (#83812)
LocalExporter must be initialized fully before it can be used in the CleanerService to clean up indices. Nothing about its local state is needed for cleaning indices, and I don't think anything about its initialization of monitoring resources is needed in order to delete old indices either. Waiting for initialization can be time consuming, and thus causes some test failures in the cleaner service. By slimming down the required state of the cleaner listener this should clear up some of the test failures surrounding it.
1 parent 2bcc03d commit d6aba55

File tree

2 files changed

+39
-47
lines changed

2 files changed

+39
-47
lines changed

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -598,64 +598,64 @@ private boolean canUseWatcher() {
598598

599599
@Override
600600
public void onCleanUpIndices(TimeValue retention) {
601-
if (state.get() != State.RUNNING) {
601+
ClusterState clusterState = clusterService.state();
602+
if (clusterService.localNode() == null
603+
|| clusterState == null
604+
|| clusterState.blocks().hasGlobalBlockWithLevel(ClusterBlockLevel.METADATA_WRITE)) {
602605
logger.debug("exporter not ready");
603606
return;
604607
}
605608

606-
if (clusterService.state().nodes().isLocalNodeElectedMaster()) {
609+
if (clusterState.nodes().isLocalNodeElectedMaster()) {
607610
// Reference date time will be compared to index.creation_date settings,
608611
// that's why it must be in UTC
609612
ZonedDateTime expiration = ZonedDateTime.now(ZoneOffset.UTC).minus(retention.millis(), ChronoUnit.MILLIS);
610613
logger.debug("cleaning indices [expiration={}, retention={}]", expiration, retention);
611614

612-
ClusterState clusterState = clusterService.state();
613-
if (clusterState != null) {
614-
final long expirationTimeMillis = expiration.toInstant().toEpochMilli();
615-
final long currentTimeMillis = System.currentTimeMillis();
615+
final long expirationTimeMillis = expiration.toInstant().toEpochMilli();
616+
final long currentTimeMillis = System.currentTimeMillis();
616617

617-
// list of index patterns that we clean up
618-
final String[] indexPatterns = new String[] { ".monitoring-*" };
618+
// list of index patterns that we clean up
619+
final String[] indexPatterns = new String[] { ".monitoring-*" };
619620

620-
// Get the names of the current monitoring indices
621-
final Set<String> currents = MonitoredSystem.allSystems()
622-
.map(s -> MonitoringTemplateUtils.indexName(dateTimeFormatter, s, currentTimeMillis))
623-
.collect(Collectors.toSet());
621+
// Get the names of the current monitoring indices
622+
final Set<String> currents = MonitoredSystem.allSystems()
623+
.map(s -> MonitoringTemplateUtils.indexName(dateTimeFormatter, s, currentTimeMillis))
624+
.collect(Collectors.toSet());
624625

625-
// avoid deleting the current alerts index, but feel free to delete older ones
626-
currents.add(MonitoringTemplateRegistry.ALERTS_INDEX_TEMPLATE_NAME);
626+
// avoid deleting the current alerts index, but feel free to delete older ones
627+
currents.add(MonitoringTemplateRegistry.ALERTS_INDEX_TEMPLATE_NAME);
627628

628-
Set<String> indices = new HashSet<>();
629-
for (ObjectObjectCursor<String, IndexMetadata> index : clusterState.getMetadata().indices()) {
630-
String indexName = index.key;
629+
Set<String> indices = new HashSet<>();
630+
for (ObjectObjectCursor<String, IndexMetadata> index : clusterState.getMetadata().indices()) {
631+
String indexName = index.key;
631632

632-
if (Regex.simpleMatch(indexPatterns, indexName)) {
633-
// Never delete any "current" index (e.g., today's index or the most recent version no timestamp, like alerts)
634-
if (currents.contains(indexName)) {
635-
continue;
636-
}
633+
if (Regex.simpleMatch(indexPatterns, indexName)) {
634+
// Never delete any "current" index (e.g., today's index or the most recent version no timestamp, like alerts)
635+
if (currents.contains(indexName)) {
636+
continue;
637+
}
637638

638-
long creationDate = index.value.getCreationDate();
639-
if (creationDate <= expirationTimeMillis) {
640-
if (logger.isDebugEnabled()) {
641-
logger.debug(
642-
"detected expired index [name={}, created={}, expired={}]",
643-
indexName,
644-
Instant.ofEpochMilli(creationDate).atZone(ZoneOffset.UTC),
645-
expiration
646-
);
647-
}
648-
indices.add(indexName);
639+
long creationDate = index.value.getCreationDate();
640+
if (creationDate <= expirationTimeMillis) {
641+
if (logger.isDebugEnabled()) {
642+
logger.debug(
643+
"detected expired index [name={}, created={}, expired={}]",
644+
indexName,
645+
Instant.ofEpochMilli(creationDate).atZone(ZoneOffset.UTC),
646+
expiration
647+
);
649648
}
649+
indices.add(indexName);
650650
}
651651
}
652+
}
652653

653-
if (indices.isEmpty() == false) {
654-
logger.info("cleaning up [{}] old indices", indices.size());
655-
deleteIndices(indices);
656-
} else {
657-
logger.debug("no old indices found for clean up");
658-
}
654+
if (indices.isEmpty() == false) {
655+
logger.info("cleaning up [{}] old indices", indices.size());
656+
deleteIndices(indices);
657+
} else {
658+
logger.debug("no old indices found for clean up");
659659
}
660660
}
661661
}

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
1515
import org.elasticsearch.xpack.monitoring.exporter.Exporter;
1616
import org.elasticsearch.xpack.monitoring.exporter.Exporters;
17-
import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
1817
import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase;
1918
import org.junit.Before;
2019

@@ -23,7 +22,6 @@
2322
import java.util.Locale;
2423

2524
import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
26-
import static org.hamcrest.Matchers.is;
2725

2826
@ClusterScope(scope = TEST, numDataNodes = 0, numClientNodes = 0)
2927
public abstract class AbstractIndicesCleanerTestCase extends MonitoringIntegTestCase {
@@ -40,7 +38,6 @@ public void setup() {
4038
cleanerService.setGlobalRetention(TimeValue.MAX_VALUE);
4139
}
4240

43-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/78737")
4441
public void testNothingToDelete() throws Exception {
4542
CleanerService.Listener listener = getListener();
4643
listener.onCleanUpIndices(days(0));
@@ -107,7 +104,6 @@ public void testIgnoreCurrentTimestampedIndex() throws Exception {
107104
assertIndicesCount(1);
108105
}
109106

110-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/78862")
111107
public void testDeleteIndices() throws Exception {
112108
CleanerService.Listener listener = getListener();
113109

@@ -167,10 +163,6 @@ protected CleanerService.Listener getListener() throws Exception {
167163
Exporters exporters = internalCluster().getInstance(Exporters.class, internalCluster().getMasterName());
168164
for (Exporter exporter : exporters.getEnabledExporters()) {
169165
if (exporter instanceof CleanerService.Listener) {
170-
// Ensure that the exporter is initialized.
171-
if (exporter instanceof LocalExporter) {
172-
assertBusy(() -> assertThat(((LocalExporter) exporter).isExporterReady(), is(true)));
173-
}
174166
return (CleanerService.Listener) exporter;
175167
}
176168
}

0 commit comments

Comments
 (0)