Marvel: Only clean timestamped indices with the current template version

Only current timestamped indices, like .marvel-es-1-* indices should be deleted. Other indices like the ones created by pre v2.3.0 plugin versions should be kept (like .marvel-es-YYYY.MM.dd)

Original commit: elastic/x-pack-elasticsearch@b2aff31875

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/MarvelSettings.java

@@ -24,6 +24,8 @@ public class MarvelSettings extends AbstractComponent {
 
     public static final String MONITORING_INDICES_PREFIX = ".monitoring-es-";
     public static final String MONITORING_DATA_INDEX_PREFIX = ".monitoring-es-data-";
+    public static final String LEGACY_DATA_INDEX_NAME = ".marvel-es-data";
+
     public static final String HISTORY_DURATION_SETTING_NAME = "history.duration";
     public static final TimeValue MAX_LICENSE_GRACE_PERIOD = TimeValue.timeValueHours(7 * 24);
 

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/AbstractCollector.java

@@ -12,7 +12,7 @@ import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.marvel.MarvelSettings;
-import org.elasticsearch.marvel.agent.exporter.IndexNameResolver;
+import org.elasticsearch.marvel.agent.exporter.MonitoringIndexNameResolver;
 import org.elasticsearch.marvel.agent.exporter.MarvelDoc;
 import org.elasticsearch.marvel.agent.exporter.MarvelTemplateUtils;
 import org.elasticsearch.marvel.license.MarvelLicensee;
@@ -27,7 +27,7 @@ public abstract class AbstractCollector<T> extends AbstractLifecycleComponent<T>
     protected final ClusterService clusterService;
     protected final MarvelSettings marvelSettings;
     protected final MarvelLicensee licensee;
-    protected final IndexNameResolver dataIndexNameResolver;
+    private final MonitoringIndexNameResolver dataIndexNameResolver;
 
     @Inject
     public AbstractCollector(Settings settings, String name, ClusterService clusterService,
@@ -116,14 +116,19 @@ public abstract class AbstractCollector<T> extends AbstractLifecycleComponent<T>
         return clusterService.state().metaData().clusterUUID();
     }
 
+
     protected DiscoveryNode localNode() {
         return clusterService.localNode();
     }
 
+    public String resolveDataIndexName(long timestamp) {
+        return dataIndexNameResolver.resolve(timestamp);
+    }
+
     /**
      * Resolves monitoring's data index name
      */
-    public class DataIndexNameResolver implements IndexNameResolver {
+    public class DataIndexNameResolver implements MonitoringIndexNameResolver {
 
         private final String index;
 
@@ -141,5 +146,10 @@ public abstract class AbstractCollector<T> extends AbstractLifecycleComponent<T>
         public String resolve(long timestamp) {
             return index;
         }
+
+        @Override
+        public String indexPattern() {
+            return index;
+        }
     }
 }

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/cluster/ClusterStateCollector.java

@@ -86,7 +86,7 @@ public class ClusterStateCollector extends AbstractCollector<ClusterStateCollect
                 results.add(clusterStateNodeDoc);
 
                 // Adds a document for every node in the monitoring data index (type "node")
-                DiscoveryNodeMarvelDoc discoveryNodeDoc = new DiscoveryNodeMarvelDoc(dataIndexNameResolver.resolve(timestamp), NODE_TYPE,
+                DiscoveryNodeMarvelDoc discoveryNodeDoc = new DiscoveryNodeMarvelDoc(resolveDataIndexName(timestamp), NODE_TYPE,
                         node.getId());
                 discoveryNodeDoc.setClusterUUID(clusterUUID);
                 discoveryNodeDoc.setTimestamp(timestamp);

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/cluster/ClusterStatsCollector.java

@@ -85,7 +85,7 @@ public class ClusterStatsCollector extends AbstractCollector<ClusterStatsCollect
         DiscoveryNode sourceNode = localNode();
 
         // Adds a cluster info document
-        String resolvedIndex = dataIndexNameResolver.resolve(timestamp);
+        String resolvedIndex = resolveDataIndexName(timestamp);
         ClusterInfoMarvelDoc clusterInfoDoc = new ClusterInfoMarvelDoc(resolvedIndex, CLUSTER_INFO_TYPE, clusterUUID);
         clusterInfoDoc.setClusterUUID(clusterUUID);
         clusterInfoDoc.setTimestamp(timestamp);

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/Exporter.java

@@ -27,7 +27,7 @@ public abstract class Exporter  {
     protected final String type;
     protected final Config config;
     protected final ESLogger logger;
-    protected final IndexNameResolver indexNameResolver;
+    protected final MonitoringIndexNameResolver indexNameResolver;
     protected final @Nullable TimeValue bulkTimeout;
 
     public Exporter(String type, Config config) {
@@ -46,7 +46,7 @@ public abstract class Exporter  {
         return config.name;
     }
 
-    public IndexNameResolver indexNameResolver() {
+    public MonitoringIndexNameResolver indexNameResolver() {
         return indexNameResolver;
     }
 
@@ -128,7 +128,7 @@ public abstract class Exporter  {
     /**
      *
      */
-    public class DefaultIndexNameResolver implements IndexNameResolver {
+    public class DefaultIndexNameResolver implements MonitoringIndexNameResolver {
 
         private final DateTimeFormatter indexTimeFormatter;
 
@@ -156,6 +156,11 @@ public abstract class Exporter  {
                     indexTimeFormatter.print(timestamp);
         }
 
+        @Override
+        public String indexPattern() {
+            return MarvelSettings.MONITORING_INDICES_PREFIX + String.valueOf(MarvelTemplateUtils.TEMPLATE_VERSION) + "-*";
+        }
+
         @Override
         public String toString() {
             return indexTimeFormatter.toString();

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/MonitoringIndexNameResolver.java

@@ -8,9 +8,17 @@ package org.elasticsearch.marvel.agent.exporter;
 /**
  *
  */
-public interface IndexNameResolver {
+public interface MonitoringIndexNameResolver {
 
     String resolve(MarvelDoc doc);
 
     String resolve(long timestamp);
+
+    /**
+     * Returns the generic part of the index name (ie without any dynamic part like a timestamp) that can be used to match indices names.
+     *
+     * @return the index pattern
+     */
+    String indexPattern();
+
 }

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/local/LocalBulk.java

@@ -15,7 +15,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.marvel.agent.exporter.ExportBulk;
-import org.elasticsearch.marvel.agent.exporter.IndexNameResolver;
+import org.elasticsearch.marvel.agent.exporter.MonitoringIndexNameResolver;
 import org.elasticsearch.marvel.agent.exporter.MarvelDoc;
 import org.elasticsearch.marvel.agent.renderer.Renderer;
 import org.elasticsearch.marvel.agent.renderer.RendererRegistry;
@@ -31,7 +31,7 @@ public class LocalBulk extends ExportBulk {
 
     private final ESLogger logger;
     private final Client client;
-    private final IndexNameResolver indexNameResolver;
+    private final MonitoringIndexNameResolver indexNameResolver;
     private final RendererRegistry renderers;
 
     private BytesStreamOutput buffer = null;
@@ -39,7 +39,7 @@ public class LocalBulk extends ExportBulk {
 
     AtomicReference<State> state = new AtomicReference<>();
 
-    public LocalBulk(String name, ESLogger logger, Client client, IndexNameResolver indexNameResolver, RendererRegistry renderers) {
+    public LocalBulk(String name, ESLogger logger, Client client, MonitoringIndexNameResolver indexNameResolver, RendererRegistry renderers) {
         super(name);
         this.logger = logger;
         this.client = client;

elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/local/LocalExporter.java

@@ -24,11 +24,11 @@ import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.gateway.GatewayService;
+import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.agent.exporter.ExportBulk;
 import org.elasticsearch.marvel.agent.exporter.Exporter;
 import org.elasticsearch.marvel.agent.exporter.MarvelTemplateUtils;
 import org.elasticsearch.marvel.agent.renderer.RendererRegistry;
-import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.cleaner.CleanerService;
 import org.elasticsearch.shield.InternalClient;
 import org.joda.time.DateTime;
@@ -268,11 +268,15 @@ public class LocalExporter extends Exporter implements ClusterStateListener, Cle
                 long expirationTime = expiration.getMillis();
                 Set<String> indices = new HashSet<>();
 
+                // Only clean up indices that match the timestamped index pattern
+                String pattern = indexNameResolver().indexPattern();
                 for (ObjectObjectCursor<String, IndexMetaData> index : clusterState.getMetaData().indices()) {
                     String indexName =  index.key;
-                    if (Regex.simpleMatch(MarvelSettings.MONITORING_INDICES_PREFIX + "*", indexName)) {
+
-                        // Never delete the data indices
+                    if (Regex.simpleMatch(pattern, indexName)) {
-                        if (indexName.startsWith(MarvelSettings.MONITORING_DATA_INDEX_PREFIX)) {
+                        // Should not happen, but just in case... we never delete the data indices
+                        if (indexName.startsWith(MarvelSettings.MONITORING_DATA_INDEX_PREFIX)
+                                || indexName.equalsIgnoreCase(MarvelSettings.LEGACY_DATA_INDEX_NAME)) {
                             continue;
                         }
 

elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/cleaner/AbstractIndicesCleanerTestCase.java

@@ -10,15 +10,16 @@ import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.license.core.License;
 import org.elasticsearch.license.plugin.core.LicenseState;
 import org.elasticsearch.license.plugin.core.Licensee;
+import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.agent.exporter.Exporter;
 import org.elasticsearch.marvel.agent.exporter.Exporters;
-import org.elasticsearch.marvel.agent.exporter.IndexNameResolver;
+import org.elasticsearch.marvel.agent.exporter.MarvelTemplateUtils;
-import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.license.MarvelLicensee;
 import org.elasticsearch.marvel.test.MarvelIntegTestCase;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
+import org.joda.time.format.DateTimeFormat;
 
 import java.util.Locale;
 
@@ -49,7 +50,7 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
     public void testDeleteIndex() throws Exception {
         internalCluster().startNode();
 
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "test", now().minusDays(10));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now().minusDays(10));
         assertIndicesCount(1);
 
         CleanerService.Listener listener = getListener();
@@ -57,10 +58,10 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
         assertIndicesCount(0);
     }
 
-    public void testIgnoreDataIndex() throws Exception {
+    public void testIgnoreCurrentDataIndex() throws Exception {
         internalCluster().startNode();
 
-        createIndex(MarvelSettings.MONITORING_DATA_INDEX_PREFIX + "test", now().minusDays(10));
+        createDataIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now().minusDays(10));
         assertIndicesCount(1);
 
         CleanerService.Listener listener = getListener();
@@ -68,24 +69,41 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
         assertIndicesCount(1);
     }
 
+    public void testIgnoreDataIndicesInOtherVersions() throws Exception {
+        internalCluster().startNode();
+
+        createIndex(MarvelSettings.LEGACY_DATA_INDEX_NAME, now().minusYears(1));
+        createDataIndex(0, now().minusDays(10));
+        createDataIndex(Integer.MAX_VALUE, now().minusDays(20));
+        assertIndicesCount(3);
+
+        CleanerService.Listener listener = getListener();
+        listener.onCleanUpIndices(days(0));
+        assertIndicesCount(3);
+    }
+
     public void testIgnoreCurrentTimestampedIndex() throws Exception {
         internalCluster().startNode();
 
-        IndexNameResolver indexNameResolver = null;
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now().minusDays(10));
-        for (Exporter exporter : internalCluster().getInstance(Exporters.class)) {
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now());
-            indexNameResolver = exporter.indexNameResolver();
-        }
-        assertNotNull(indexNameResolver);
-
-        DateTime tenDaysAgo = now().minusDays(10);
-        createIndex(indexNameResolver.resolve(tenDaysAgo.getMillis()), tenDaysAgo);
-
-        DateTime today = now();
-        createIndex(indexNameResolver.resolve(today.getMillis()), today);
         assertIndicesCount(2);
 
         CleanerService.Listener listener = getListener();
         listener.onCleanUpIndices(days(0));
+        assertIndicesCount(1);
+    }
+
+    public void testIgnoreTimestampedIndicesInOtherVersions() throws Exception {
+        internalCluster().startNode();
+
+        createTimestampedIndex(0, now().minusDays(10));
+        createTimestampedIndex(Integer.MAX_VALUE, now().minusDays(10));
+        assertIndicesCount(2);
+
+        CleanerService.Listener listener = getListener();
+        listener.onCleanUpIndices(days(0));
+        assertIndicesCount(2);
     }
 
     public void testDeleteIndices() throws Exception {
@@ -94,11 +112,11 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
         CleanerService.Listener listener = getListener();
 
         final DateTime now = now();
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "one-year-ago", now.minusYears(1));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusYears(1));
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "six-months-ago", now.minusMonths(6));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusMonths(6));
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "one-month-ago", now.minusMonths(1));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusMonths(1));
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "ten-days-ago", now.minusDays(10));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusDays(10));
-        createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + "one-day-ago", now.minusDays(1));
+        createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusDays(1));
         assertIndicesCount(5);
 
         // Clean indices that have expired two years ago
@@ -134,7 +152,7 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
 
         final DateTime now = now();
         for (int i = 0; i < max; i++) {
-            createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + String.valueOf(i), now.minusDays(i));
+            createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusDays(i));
         }
         assertIndicesCount(max);
 
@@ -154,7 +172,7 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
 
         final DateTime now = now();
         for (int i = 0; i < max; i++) {
-            createIndex(MarvelSettings.MONITORING_INDICES_PREFIX + String.valueOf(i), now.minusDays(i));
+            createTimestampedIndex(MarvelTemplateUtils.TEMPLATE_VERSION, now.minusDays(i));
         }
         assertIndicesCount(max);
 
@@ -194,6 +212,23 @@ public abstract class AbstractIndicesCleanerTestCase extends MarvelIntegTestCase
         throw new IllegalStateException("unable to find listener");
     }
 
+    /**
+     * Creates a monitoring data index in a given version.
+     */
+    protected void createDataIndex(int version, DateTime creationDate) {
+        String indexName = MarvelSettings.MONITORING_DATA_INDEX_PREFIX + String.valueOf(version);
+        createIndex(indexName, creationDate);
+    }
+
+    /**
+     * Creates a monitoring timestamped index in a given version.
+     */
+    protected void createTimestampedIndex(int version, DateTime creationDate) {
+        String indexName = MarvelSettings.MONITORING_INDICES_PREFIX + String.valueOf(version) + "-"
+                + DateTimeFormat.forPattern(Exporter.DEFAULT_INDEX_NAME_TIME_FORMAT).withZoneUTC().print(creationDate.getMillis());
+        createIndex(indexName, creationDate);
+    }
+
     protected abstract void createIndex(String name, DateTime creationDate);
 
     protected abstract void assertIndicesCount(int count) throws Exception;

elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/cleaner/local/LocalIndicesCleanerTests.java

@@ -9,7 +9,6 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.marvel.agent.exporter.local.LocalExporter;
-import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.cleaner.AbstractIndicesCleanerTestCase;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.InternalSettingsPlugin;
@@ -52,8 +51,7 @@ public class LocalIndicesCleanerTests extends AbstractIndicesCleanerTestCase {
             @Override
             public void run() {
                 try {
-                    assertThat(client().admin().indices().prepareGetSettings(MarvelSettings.MONITORING_INDICES_PREFIX + "*")
+                    assertThat(client().admin().indices().prepareGetSettings().get().getIndexToSettings().size(), equalTo(count));
-                            .get().getIndexToSettings().size(), equalTo(count));
                 } catch (IndexNotFoundException e) {
                     if (shieldEnabled) {
                         assertThat(0, equalTo(count));