Skip to content

Commit a5f8919

Browse files
authored
Make ILM force merging best effort (#43246)
It's possible for force merges kicked off by ILM to silently stop (due to a node relocating for example). In which case, the segment count may not reach what the user configured. In the subsequent `SegmentCountStep` waiting for the expected segment count may wait indefinitely. Because of this, this commit makes force merges "best effort" and then changes the `SegmentCountStep` to simply report (at INFO level) if the merge was not successful. Relates to #42824 Resolves #43245
1 parent bcb77b4 commit a5f8919

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/SegmentCountStep.java

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@
55
*/
66
package org.elasticsearch.xpack.core.indexlifecycle;
77

8+
import org.apache.logging.log4j.LogManager;
9+
import org.apache.logging.log4j.Logger;
810
import org.elasticsearch.action.ActionListener;
11+
import org.elasticsearch.action.admin.indices.segments.IndexSegments;
912
import org.elasticsearch.action.admin.indices.segments.IndicesSegmentsRequest;
13+
import org.elasticsearch.action.admin.indices.segments.ShardSegments;
1014
import org.elasticsearch.client.Client;
1115
import org.elasticsearch.cluster.metadata.IndexMetaData;
16+
import org.elasticsearch.cluster.routing.ShardRouting;
1217
import org.elasticsearch.common.ParseField;
1318
import org.elasticsearch.common.Strings;
1419
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
@@ -17,13 +22,17 @@
1722

1823
import java.io.IOException;
1924
import java.util.Arrays;
25+
import java.util.List;
26+
import java.util.Map;
2027
import java.util.Objects;
21-
import java.util.stream.StreamSupport;
28+
import java.util.stream.Collectors;
2229

2330
/**
2431
* This {@link Step} evaluates whether force_merge was successful by checking the segment count.
2532
*/
2633
public class SegmentCountStep extends AsyncWaitStep {
34+
35+
private static final Logger logger = LogManager.getLogger(SegmentCountStep.class);
2736
public static final String NAME = "segment-count";
2837

2938
private final int maxNumSegments;
@@ -41,10 +50,19 @@ public int getMaxNumSegments() {
4150
public void evaluateCondition(IndexMetaData indexMetaData, Listener listener) {
4251
getClient().admin().indices().segments(new IndicesSegmentsRequest(indexMetaData.getIndex().getName()),
4352
ActionListener.wrap(response -> {
44-
long numberShardsLeftToMerge =
45-
StreamSupport.stream(response.getIndices().get(indexMetaData.getIndex().getName()).spliterator(), false)
46-
.filter(iss -> Arrays.stream(iss.getShards()).anyMatch(p -> p.getSegments().size() > maxNumSegments)).count();
47-
listener.onResponse(numberShardsLeftToMerge == 0, new Info(numberShardsLeftToMerge));
53+
IndexSegments segments = response.getIndices().get(indexMetaData.getIndex().getName());
54+
List<ShardSegments> unmergedShards = segments.getShards().values().stream()
55+
.flatMap(iss -> Arrays.stream(iss.getShards()))
56+
.filter(shardSegments -> shardSegments.getSegments().size() > maxNumSegments)
57+
.collect(Collectors.toList());
58+
if (unmergedShards.size() > 0) {
59+
Map<ShardRouting, Integer> unmergedShardCounts = unmergedShards.stream()
60+
.collect(Collectors.toMap(ShardSegments::getShardRouting, ss -> ss.getSegments().size()));
61+
logger.info("[{}] best effort force merge to [{}] segments did not succeed for {} shards: {}",
62+
indexMetaData.getIndex().getName(), maxNumSegments, unmergedShards.size(), unmergedShardCounts);
63+
}
64+
// Force merging is best effort, so always return true that the condition has been met.
65+
listener.onResponse(true, new Info(unmergedShards.size()));
4866
}, listener::onFailure));
4967
}
5068

@@ -90,8 +108,12 @@ public long getNumberShardsLeftToMerge() {
90108
@Override
91109
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
92110
builder.startObject();
93-
builder.field(MESSAGE.getPreferredName(),
94-
"Waiting for [" + numberShardsLeftToMerge + "] shards " + "to forcemerge");
111+
if (numberShardsLeftToMerge == 0) {
112+
builder.field(MESSAGE.getPreferredName(), "all shards force merged successfully");
113+
} else {
114+
builder.field(MESSAGE.getPreferredName(),
115+
"[" + numberShardsLeftToMerge + "] shards did not successfully force merge");
116+
}
95117
builder.field(SHARDS_TO_MERGE.getPreferredName(), numberShardsLeftToMerge);
96118
builder.endObject();
97119
return builder;

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SegmentCountStepTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public void onFailure(Exception e) {
138138
assertEquals(new SegmentCountStep.Info(0L), conditionInfo.get());
139139
}
140140

141-
public void testIsConditionFails() {
141+
public void testIsConditionIsTrueEvenWhenMoreSegments() {
142142
int maxNumSegments = randomIntBetween(3, 10);
143143
Index index = new Index(randomAlphaOfLengthBetween(1, 20), randomAlphaOfLengthBetween(1, 20));
144144
Client client = Mockito.mock(Client.class);
@@ -191,8 +191,8 @@ public void onFailure(Exception e) {
191191
}
192192
});
193193

194-
assertFalse(conditionMetResult.get());
195-
assertEquals(new SegmentCountStep.Info(1L), conditionInfo.get());
194+
assertTrue(conditionMetResult.get());
195+
assertEquals(new SegmentCountStep.Info(0L), conditionInfo.get());
196196
}
197197

198198
public void testThrowsException() {

0 commit comments

Comments
 (0)