From c9efc1b66dea2c5f4a04f3c82b566d668239f436 Mon Sep 17 00:00:00 2001 From: Kay Roepke Date: Tue, 10 May 2016 14:34:28 +0200 Subject: [PATCH] Getting index statistics could contain failed shards During rotation and retention we requested all index statistics multiple times, which, in a overloaded cluster, could lead to shard failures due to timeouts. This failure wasn't logged and could lead to using the wrong (older) index to base rotation decisions on, effectively rotating indices too early. This change makes Graylog use a more lightweight API to determine all index names including their aliases, reducing the usage of the expensive Index Statistics to the indices page only. The current rotation and retention strategies do not need to know all index statistics which require to touch every single shard in the cluster. fixes #2194 --- .../java/org/graylog2/indexer/Deflector.java | 53 ++++++------ .../org/graylog2/indexer/indices/Indices.java | 38 +++++++++ ...tractIndexCountBasedRetentionStrategy.java | 8 +- .../org/graylog2/indexer/DeflectorTest.java | 83 ++++++++++++++++++- 4 files changed, 147 insertions(+), 35 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog2/indexer/Deflector.java b/graylog2-server/src/main/java/org/graylog2/indexer/Deflector.java index cae6afe9af5b..107f2e354711 100644 --- a/graylog2-server/src/main/java/org/graylog2/indexer/Deflector.java +++ b/graylog2-server/src/main/java/org/graylog2/indexer/Deflector.java @@ -16,9 +16,6 @@ */ package org.graylog2.indexer; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; -import org.elasticsearch.action.admin.indices.stats.IndexStats; import org.elasticsearch.cluster.health.ClusterHealthStatus; import org.elasticsearch.indices.InvalidAliasNameException; import org.graylog2.indexer.indices.Indices; @@ -34,11 +31,12 @@ import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Named; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static com.google.common.base.Strings.isNullOrEmpty; @@ -182,29 +180,31 @@ public void cycle() { } public int getNewestTargetNumber() throws NoTargetIndexException { - final Map indices = this.indices.getAll(); - if (indices.isEmpty()) { + final Set indexNames = indices.getIndexNamesAndAliases(getDeflectorWildcard()).keySet(); + + if (indexNames.isEmpty()) { throw new NoTargetIndexException(); } - final List indexNumbers = Lists.newArrayListWithExpectedSize(indices.size()); - for (String indexName : indices.keySet()) { + int highestIndexNumber = -1; + for (String indexName : indexNames) { if (!isGraylogDeflectorIndex(indexName)) { continue; } try { - indexNumbers.add(extractIndexNumber(indexName)); + final int indexNumber = extractIndexNumber(indexName); + highestIndexNumber = Math.max(indexNumber, highestIndexNumber); } catch (NumberFormatException ex) { - LOG.debug("Couldn't extract index number from index name " + indexName, ex); + LOG.warn("Couldn't extract index number from index name " + indexName, ex); } } - if (indexNumbers.isEmpty()) { + if (highestIndexNumber == -1) { throw new NoTargetIndexException(); } - return Collections.max(indexNumbers); + return highestIndexNumber; } /** @@ -213,13 +213,11 @@ public int getNewestTargetNumber() throws NoTargetIndexException { * @return list of managed indices */ public String[] getAllGraylogIndexNames() { - final Map indices = this.indices.getAll(); - final List result = Lists.newArrayListWithExpectedSize(indices.size()); - for (String indexName : indices.keySet()) { - if (isGraylogIndex(indexName)) { - result.add(indexName); - } - } + final Set indexNames = indices.getIndexNamesAndAliases(getDeflectorWildcard()).keySet(); + // also allow restore archives to be returned + final List result = indexNames.stream() + .filter(this::isGraylogIndex) + .collect(Collectors.toList()); return result.toArray(new String[result.size()]); } @@ -227,18 +225,15 @@ public String[] getAllGraylogIndexNames() { /** * Returns all Graylog deflector indices. * - * @return index name and index stats + * @return index name and aliases of that index */ - public Map getAllGraylogDeflectorIndices() { - final ImmutableMap.Builder result = ImmutableMap.builder(); - for (Map.Entry e : indices.getAll().entrySet()) { - final String name = e.getKey(); + public Map> getAllGraylogDeflectorIndices() { + final Map> indexNamesAndAliases = indices.getIndexNamesAndAliases(getDeflectorWildcard()); - if (isGraylogDeflectorIndex(name)) { - result.put(name, e.getValue()); - } - } - return result.build(); + // filter out the restored archives from the result set + return indexNamesAndAliases.entrySet().stream() + .filter(e -> isGraylogDeflectorIndex(e.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } public String getNewestTargetName() throws NoTargetIndexException { diff --git a/graylog2-server/src/main/java/org/graylog2/indexer/indices/Indices.java b/graylog2-server/src/main/java/org/graylog2/indexer/indices/Indices.java index b29f9fd5808d..f67510fd3f82 100644 --- a/graylog2-server/src/main/java/org/graylog2/indexer/indices/Indices.java +++ b/graylog2-server/src/main/java/org/graylog2/indexer/indices/Indices.java @@ -19,6 +19,7 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.collect.UnmodifiableIterator; import org.elasticsearch.ElasticsearchException; @@ -38,6 +39,7 @@ import org.elasticsearch.action.admin.indices.flush.FlushRequest; import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.elasticsearch.action.admin.indices.get.GetIndexRequest; +import org.elasticsearch.action.admin.indices.get.GetIndexRequestBuilder; import org.elasticsearch.action.admin.indices.get.GetIndexResponse; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; @@ -55,6 +57,7 @@ import org.elasticsearch.client.IndicesAdminClient; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.health.ClusterHealthStatus; +import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.routing.ShardRouting; @@ -84,13 +87,16 @@ import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Singleton; +import javax.validation.constraints.NotNull; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import static java.util.stream.Collectors.toSet; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; @Singleton @@ -177,6 +183,9 @@ public Map getAll() { final IndicesStatsRequest request = c.admin().indices().prepareStats(allIndicesAlias()).request(); final IndicesStatsResponse response = c.admin().indices().stats(request).actionGet(); + if (response.getFailedShards() > 0) { + LOG.warn("IndexStats response contains failed shards, response is incomplete: {}", (Object) response.getShardFailures()); + } return response.getIndices(); } @@ -208,6 +217,35 @@ public boolean aliasExists(String alias) { return c.admin().indices().aliasesExist(new GetAliasesRequest(alias)).actionGet().exists(); } + @NotNull + public Map> getIndexNamesAndAliases(String indexPattern) { + + // only request indices matching the name or pattern in `indexPattern` and only get the alias names for each index, + // not the settings or mappings + final GetIndexRequestBuilder getIndexRequestBuilder = c.admin().indices().prepareGetIndex(); + getIndexRequestBuilder.addFeatures(GetIndexRequest.Feature.ALIASES); + getIndexRequestBuilder.setIndices(indexPattern); + + final GetIndexResponse getIndexResponse = c.admin().indices().getIndex(getIndexRequestBuilder.request()).actionGet(); + + final String[] indices = getIndexResponse.indices(); + final ImmutableOpenMap> aliases = getIndexResponse.aliases(); + final Map> indexAliases = Maps.newHashMap(); + for (String index : indices) { + final List aliasMetaData = aliases.get(index); + if (aliasMetaData == null) { + indexAliases.put(index, Collections.emptySet()); + } else { + indexAliases.put(index, + aliasMetaData.stream() + .map(AliasMetaData::alias) + .collect(toSet())); + } + } + + return indexAliases; + } + @Nullable public String aliasTarget(String alias) { final IndicesAdminClient indicesAdminClient = c.admin().indices(); diff --git a/graylog2-server/src/main/java/org/graylog2/indexer/retention/strategies/AbstractIndexCountBasedRetentionStrategy.java b/graylog2-server/src/main/java/org/graylog2/indexer/retention/strategies/AbstractIndexCountBasedRetentionStrategy.java index 83d1f9081a5e..39c1c224b52c 100644 --- a/graylog2-server/src/main/java/org/graylog2/indexer/retention/strategies/AbstractIndexCountBasedRetentionStrategy.java +++ b/graylog2-server/src/main/java/org/graylog2/indexer/retention/strategies/AbstractIndexCountBasedRetentionStrategy.java @@ -18,7 +18,6 @@ package org.graylog2.indexer.retention.strategies; import com.google.common.base.Optional; -import org.elasticsearch.action.admin.indices.stats.IndexStats; import org.graylog2.indexer.Deflector; import org.graylog2.indexer.IndexHelper; import org.graylog2.indexer.NoTargetIndexException; @@ -31,6 +30,7 @@ import org.slf4j.LoggerFactory; import java.util.Map; +import java.util.Set; public abstract class AbstractIndexCountBasedRetentionStrategy implements RetentionStrategy { private static final Logger LOG = LoggerFactory.getLogger(AbstractIndexCountBasedRetentionStrategy.class); @@ -50,7 +50,7 @@ public AbstractIndexCountBasedRetentionStrategy(Deflector deflector, Indices ind @Override public void retain() { - final Map deflectorIndices = deflector.getAllGraylogDeflectorIndices(); + final Map> deflectorIndices = deflector.getAllGraylogDeflectorIndices(); final int indexCount = deflectorIndices.size(); final Optional maxIndices = getMaxNumberOfIndices(); @@ -80,10 +80,10 @@ public void retain() { } } - private void runRetention(Map deflectorIndices, int removeCount) throws NoTargetIndexException { + private void runRetention(Map> deflectorIndices, int removeCount) throws NoTargetIndexException { for (String indexName : IndexHelper.getOldestIndices(deflectorIndices.keySet(), removeCount)) { // Never run against the current deflector target. - if (indexName.equals(deflector.getCurrentActualTargetIndex())) { + if (deflectorIndices.get(indexName).contains(deflector.getName())) { LOG.info("Not running retention against current deflector target <{}>.", indexName); continue; } diff --git a/graylog2-server/src/test/java/org/graylog2/indexer/DeflectorTest.java b/graylog2-server/src/test/java/org/graylog2/indexer/DeflectorTest.java index 1cfc0935207a..a0f0695aad9c 100644 --- a/graylog2-server/src/test/java/org/graylog2/indexer/DeflectorTest.java +++ b/graylog2-server/src/test/java/org/graylog2/indexer/DeflectorTest.java @@ -20,7 +20,7 @@ */ package org.graylog2.indexer; -import org.elasticsearch.action.admin.indices.stats.IndexStats; +import com.google.common.collect.Maps; import org.graylog2.indexer.indices.Indices; import org.graylog2.indexer.ranges.CreateNewSingleIndexRangeJob; import org.graylog2.system.activities.SystemMessageActivityWriter; @@ -31,12 +31,18 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import java.util.Collections; import java.util.Map; +import java.util.Set; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class DeflectorTest { @@ -90,7 +96,7 @@ public void testBuildDeflectorNameWithCustomIndexPrefix() { @Test public void nullIndexerDoesNotThrow() { - final Map deflectorIndices = deflector.getAllGraylogDeflectorIndices(); + final Map> deflectorIndices = deflector.getAllGraylogDeflectorIndices(); assertNotNull(deflectorIndices); assertEquals(0, deflectorIndices.size()); } @@ -149,4 +155,77 @@ public void testIsGraylogIndex() { assertFalse(deflector.isGraylogDeflectorIndex("HAHA")); assertFalse(deflector.isGraylogIndex("HAHA")); } + + @Test + public void getNewestTargetNumber() throws NoTargetIndexException { + final Indices indices = mock(Indices.class); + Map> indexNameAliases = Maps.newHashMap(); + indexNameAliases.put("graylog_1", Collections.emptySet()); + indexNameAliases.put("graylog_2", Collections.emptySet()); + indexNameAliases.put("graylog_3", Collections.singleton("graylog_deflector")); + indexNameAliases.put("graylog_4_restored_archive", Collections.emptySet()); + + when(indices.getIndexNamesAndAliases(anyString())).thenReturn(indexNameAliases); + final Deflector deflector = new Deflector(systemJobManager, + "graylog", + activityWriter, + indexReadOnlyJobFactory, + singleIndexRangeJobFactory, + indices); + + final int number = deflector.getNewestTargetNumber(); + assertEquals(3, number); + } + + @Test + public void getAllGraylogIndexNames() { + final Indices indices = mock(Indices.class); + Map> indexNameAliases = Maps.newHashMap(); + indexNameAliases.put("graylog_1", Collections.emptySet()); + indexNameAliases.put("graylog_2", Collections.emptySet()); + indexNameAliases.put("graylog_3", Collections.emptySet()); + indexNameAliases.put("graylog_4_restored_archive", Collections.emptySet()); + indexNameAliases.put("graylog_5", Collections.singleton("graylog_deflector")); + + when(indices.getIndexNamesAndAliases(anyString())).thenReturn(indexNameAliases); + final Deflector deflector = new Deflector(systemJobManager, + "graylog", + activityWriter, + indexReadOnlyJobFactory, + singleIndexRangeJobFactory, + indices); + + final String[] allGraylogIndexNames = deflector.getAllGraylogIndexNames(); + assertThat(allGraylogIndexNames) + .containsExactlyInAnyOrder("graylog_1", "graylog_2", "graylog_3", "graylog_4_restored_archive", "graylog_5"); + } + + @Test + public void getAllGraylogDeflectorIndices() { + final Indices indices = mock(Indices.class); + Map> indexNameAliases = Maps.newHashMap(); + indexNameAliases.put("graylog_1", Collections.emptySet()); + indexNameAliases.put("graylog_2", Collections.emptySet()); + indexNameAliases.put("graylog_3", Collections.emptySet()); + indexNameAliases.put("graylog_4_restored_archive", Collections.emptySet()); + indexNameAliases.put("graylog_5", Collections.singleton("graylog_deflector")); + + when(indices.getIndexNamesAndAliases(anyString())).thenReturn(indexNameAliases); + final Deflector deflector = new Deflector(systemJobManager, + "graylog", + activityWriter, + indexReadOnlyJobFactory, + singleIndexRangeJobFactory, + indices); + + final Map> deflectorIndices = deflector.getAllGraylogDeflectorIndices(); + + assertThat(deflectorIndices).isNotNull(); + assertThat(deflectorIndices).isNotEmpty(); + assertThat(deflectorIndices.keySet()) + .containsExactlyInAnyOrder("graylog_1", "graylog_2", "graylog_3", "graylog_5"); + } + + + }