Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Getting index statistics could contain failed shards #2210

Merged
merged 1 commit into from May 11, 2016
Merged

Getting index statistics could contain failed shards #2210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 24 additions & 29 deletions graylog2-server/src/main/java/org/graylog2/indexer/Deflector.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -182,29 +180,31 @@ public void cycle() {
}

public int getNewestTargetNumber() throws NoTargetIndexException {
final Map<String, IndexStats> indices = this.indices.getAll();
if (indices.isEmpty()) {
final Set<String> indexNames = indices.getIndexNamesAndAliases(getDeflectorWildcard()).keySet();

if (indexNames.isEmpty()) {
throw new NoTargetIndexException();
}

final List<Integer> 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);
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about replacing this with a streams call like this:

        final Optional<Integer> highestIndexNumber = indexNames.stream()
            .filter(indexName -> !this.isGraylogDeflectorIndex(indexName))
            .map(Deflector::extractIndexNumber)
            .max(Integer::max);

Makes it easier to grasp what is done (at least for me).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory I agree, but in this case we'd need to refactor extractIndexNumber as well and its callers, because it throws an exception, and it already felt I changed a lot of code already :(

How about making those changes on master?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if (indexNumbers.isEmpty()) {
if (highestIndexNumber == -1) {
throw new NoTargetIndexException();
}

return Collections.max(indexNumbers);
return highestIndexNumber;
}

/**
Expand All @@ -213,32 +213,27 @@ public int getNewestTargetNumber() throws NoTargetIndexException {
* @return list of managed indices
*/
public String[] getAllGraylogIndexNames() {
final Map<String, IndexStats> indices = this.indices.getAll();
final List<String> result = Lists.newArrayListWithExpectedSize(indices.size());
for (String indexName : indices.keySet()) {
if (isGraylogIndex(indexName)) {
result.add(indexName);
}
}
final Set<String> indexNames = indices.getIndexNamesAndAliases(getDeflectorWildcard()).keySet();
// also allow restore archives to be returned
final List<String> result = indexNames.stream()
.filter(this::isGraylogIndex)
.collect(Collectors.toList());

return result.toArray(new String[result.size()]);
}

/**
* Returns all Graylog deflector indices.
*
* @return index name and index stats
* @return index name and aliases of that index
*/
public Map<String, IndexStats> getAllGraylogDeflectorIndices() {
final ImmutableMap.Builder<String, IndexStats> result = ImmutableMap.builder();
for (Map.Entry<String, IndexStats> e : indices.getAll().entrySet()) {
final String name = e.getKey();
public Map<String, Set<String>> getAllGraylogDeflectorIndices() {
final Map<String, Set<String>> 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 {
Expand Down
38 changes: 38 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/indexer/indices/Indices.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -177,6 +183,9 @@ public Map<String, IndexStats> 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();
}

Expand Down Expand Up @@ -208,6 +217,35 @@ public boolean aliasExists(String alias) {
return c.admin().indices().aliasesExist(new GetAliasesRequest(alias)).actionGet().exists();
}

@NotNull
public Map<String, Set<String>> 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<String, List<AliasMetaData>> aliases = getIndexResponse.aliases();
final Map<String, Set<String>> indexAliases = Maps.newHashMap();
for (String index : indices) {
final List<AliasMetaData> 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();
Expand Down
8 changes: 4 additions & 4 deletions ...a/org/graylog2/indexer/retention/strategies/AbstractIndexCountBasedRetentionStrategy.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -50,7 +50,7 @@ public AbstractIndexCountBasedRetentionStrategy(Deflector deflector, Indices ind

@Override
public void retain() {
final Map<String, IndexStats> deflectorIndices = deflector.getAllGraylogDeflectorIndices();
final Map<String, Set<String>> deflectorIndices = deflector.getAllGraylogDeflectorIndices();
final int indexCount = deflectorIndices.size();
final Optional<Integer> maxIndices = getMaxNumberOfIndices();

Expand Down Expand Up @@ -80,10 +80,10 @@ public void retain() {
}
}

private void runRetention(Map<String, IndexStats> deflectorIndices, int removeCount) throws NoTargetIndexException {
private void runRetention(Map<String, Set<String>> 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;
}
Expand Down
83 changes: 81 additions & 2 deletions graylog2-server/src/test/java/org/graylog2/indexer/DeflectorTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -90,7 +96,7 @@ public void testBuildDeflectorNameWithCustomIndexPrefix() {

@Test
public void nullIndexerDoesNotThrow() {
final Map<String, IndexStats> deflectorIndices = deflector.getAllGraylogDeflectorIndices();
final Map<String, Set<String>> deflectorIndices = deflector.getAllGraylogDeflectorIndices();
assertNotNull(deflectorIndices);
assertEquals(0, deflectorIndices.size());
}
Expand Down Expand Up @@ -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<String, Set<String>> 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<String, Set<String>> 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<String, Set<String>> 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<String, Set<String>> deflectorIndices = deflector.getAllGraylogDeflectorIndices();

assertThat(deflectorIndices).isNotNull();
assertThat(deflectorIndices).isNotEmpty();
assertThat(deflectorIndices.keySet())
.containsExactlyInAnyOrder("graylog_1", "graylog_2", "graylog_3", "graylog_5");
}



}