Browse Source

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.
James Baiera 3 năm trước cách đây
mục cha
commit
d6aba55d3a

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

@@ -598,64 +598,64 @@ public class LocalExporter extends Exporter implements ClusterStateListener, Cle
 
     @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");
             }
         }
     }

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

@@ -14,7 +14,6 @@ import org.elasticsearch.xpack.core.monitoring.MonitoringField;
 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;
 
@@ -23,7 +22,6 @@ import java.time.ZonedDateTime;
 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 {
@@ -40,7 +38,6 @@ public abstract class AbstractIndicesCleanerTestCase extends MonitoringIntegTest
         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));
@@ -107,7 +104,6 @@ public abstract class AbstractIndicesCleanerTestCase extends MonitoringIntegTest
         assertIndicesCount(1);
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/78862")
     public void testDeleteIndices() throws Exception {
         CleanerService.Listener listener = getListener();
 
@@ -167,10 +163,6 @@ public abstract class AbstractIndicesCleanerTestCase extends MonitoringIntegTest
         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;
             }
         }