Skip to content

Commit

Permalink
Simplify LocalExporter cleaner function to fix failing tests (elastic…
Browse files Browse the repository at this point in the history 
…#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.
  • Loading branch information
@jbaiera
jbaiera committed Feb 15, 2022
1 parent 5e44c72 commit 16a46cb
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 47 deletions.
78 changes: 39 additions & 39 deletions ...toring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,64 +598,64 @@ private boolean canUseWatcher() {

@Override
public void onCleanUpIndices(TimeValue retention) {
if (state.get() != State.RUNNING) {
ClusterState clusterState = clusterService.state();
if (clusterService.localNode() == null
|| clusterState == null
|| clusterState.blocks().hasGlobalBlockWithLevel(ClusterBlockLevel.METADATA_WRITE)) {
logger.debug("exporter not ready");
return;
}

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

ClusterState clusterState = clusterService.state();
if (clusterState != null) {
final long expirationTimeMillis = expiration.toInstant().toEpochMilli();
final long currentTimeMillis = System.currentTimeMillis();
final long expirationTimeMillis = expiration.toInstant().toEpochMilli();
final long currentTimeMillis = System.currentTimeMillis();

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

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

// avoid deleting the current alerts index, but feel free to delete older ones
currents.add(MonitoringTemplateRegistry.ALERTS_INDEX_TEMPLATE_NAME);
// avoid deleting the current alerts index, but feel free to delete older ones
currents.add(MonitoringTemplateRegistry.ALERTS_INDEX_TEMPLATE_NAME);

Set<String> indices = new HashSet<>();
for (ObjectObjectCursor<String, IndexMetadata> index : clusterState.getMetadata().indices()) {
String indexName = index.key;
Set<String> indices = new HashSet<>();
for (ObjectObjectCursor<String, IndexMetadata> index : clusterState.getMetadata().indices()) {
String indexName = index.key;

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

long creationDate = index.value.getCreationDate();
if (creationDate <= expirationTimeMillis) {
if (logger.isDebugEnabled()) {
logger.debug(
"detected expired index [name={}, created={}, expired={}]",
indexName,
Instant.ofEpochMilli(creationDate).atZone(ZoneOffset.UTC),
expiration
);
}
indices.add(indexName);
long creationDate = index.value.getCreationDate();
if (creationDate <= expirationTimeMillis) {
if (logger.isDebugEnabled()) {
logger.debug(
"detected expired index [name={}, created={}, expired={}]",
indexName,
Instant.ofEpochMilli(creationDate).atZone(ZoneOffset.UTC),
expiration
);
}
indices.add(indexName);
}
}
}

if (indices.isEmpty() == false) {
logger.info("cleaning up [{}] old indices", indices.size());
deleteIndices(indices);
} else {
logger.debug("no old indices found for clean up");
}
if (indices.isEmpty() == false) {
logger.info("cleaning up [{}] old indices", indices.size());
deleteIndices(indices);
} else {
logger.debug("no old indices found for clean up");
}
}
}
Expand Down
8 changes: 0 additions & 8 deletions .../test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
import org.elasticsearch.xpack.monitoring.exporter.Exporter;
import org.elasticsearch.xpack.monitoring.exporter.Exporters;
import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase;
import org.junit.Before;

Expand All @@ -23,7 +22,6 @@
import java.util.Locale;

import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
import static org.hamcrest.Matchers.is;

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

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

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

Expand Down Expand Up @@ -167,10 +163,6 @@ protected CleanerService.Listener getListener() throws Exception {
Exporters exporters = internalCluster().getInstance(Exporters.class, internalCluster().getMasterName());
for (Exporter exporter : exporters.getEnabledExporters()) {
if (exporter instanceof CleanerService.Listener) {
// Ensure that the exporter is initialized.
if (exporter instanceof LocalExporter) {
assertBusy(() -> assertThat(((LocalExporter) exporter).isExporterReady(), is(true)));
}
return (CleanerService.Listener) exporter;
}
}
Expand Down

0 comments on commit 16a46cb

Please sign in to comment.