Skip to content

Commit e9b798b

Browse files
dakronedakrone
authored
[7.10] Make FilterAllocationDecider totally ignore tier-based allocation settings (#67019) (#67034)
Previously we treated attribute filtering for _tier-prefixed attributes a pass-through, meaning that they were essentially always treated as matching in DiscoveryNodeFilters.match, however, for exclude settings, this meant that the node was considered to match the node if a _tier* filter was specified. This commit prunes these attributes from the DiscoveryNodeFilters when considering the filters for FilterAllocationDecider so that they are only considered in DataTierAllocationDecider. Resolves #66679
1 parent 8e294d5 commit e9b798b

File tree

4 files changed

+111
-18
lines changed

4 files changed

+111
-18
lines changed

‎server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.HashMap;
3131
import java.util.Map;
3232
import java.util.function.BiConsumer;
33+
import java.util.stream.Collectors;
3334

3435
public class DiscoveryNodeFilters {
3536

@@ -88,6 +89,32 @@ private boolean matchByIP(String[] values, @Nullable String hostIp, @Nullable St
8889
return false;
8990
}
9091

92+
/**
93+
* Removes any filters that should not be considered, returning a new
94+
* {@link DiscoveryNodeFilters} object. If the filtered object has no
95+
* filters after trimming, {@code null} is returned.
96+
*/
97+
@Nullable
98+
public static DiscoveryNodeFilters trimTier(@Nullable DiscoveryNodeFilters original) {
99+
if (original == null) {
100+
return null;
101+
}
102+
103+
Map<String, String[]> newFilters = original.filters.entrySet().stream()
104+
// Remove all entries that start with "_tier", as these will be handled elsewhere
105+
.filter(entry -> {
106+
String attr = entry.getKey();
107+
return attr != null && attr.startsWith("_tier") == false;
108+
})
109+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
110+
111+
if (newFilters.size() == 0) {
112+
return null;
113+
} else {
114+
return new DiscoveryNodeFilters(original.opType, newFilters);
115+
}
116+
}
117+
91118
public boolean match(DiscoveryNode node) {
92119
for (Map.Entry<String, String[]> entry : filters.entrySet()) {
93120
String attr = entry.getKey();
@@ -181,9 +208,6 @@ public boolean match(DiscoveryNode node) {
181208
}
182209
}
183210
}
184-
} else if (attr != null && attr.startsWith("_tier")) {
185-
// Always allow _tier as an attribute, will be handled elsewhere
186-
return true;
187211
} else {
188212
String nodeAttributeValue = node.getAttributes().get(attr);
189213
if (nodeAttributeValue == null) {

‎server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
101101
// that once it has been allocated post API the replicas can be allocated elsewhere without user interaction
102102
// this is a setting that can only be set within the system!
103103
IndexMetadata indexMd = allocation.metadata().getIndexSafe(shardRouting.index());
104-
DiscoveryNodeFilters initialRecoveryFilters = indexMd.getInitialRecoveryFilters();
104+
DiscoveryNodeFilters initialRecoveryFilters = DiscoveryNodeFilters.trimTier(indexMd.getInitialRecoveryFilters());
105105
if (initialRecoveryFilters != null &&
106106
shardRouting.recoverySource().getType() == RecoverySource.Type.LOCAL_SHARDS &&
107107
initialRecoveryFilters.match(node.node()) == false) {
@@ -155,22 +155,26 @@ private Decision shouldFilter(IndexMetadata indexMd, DiscoveryNode node, Routing
155155
}
156156

157157
private Decision shouldIndexFilter(IndexMetadata indexMd, DiscoveryNode node, RoutingAllocation allocation) {
158-
if (indexMd.requireFilters() != null) {
159-
if (indexMd.requireFilters().match(node) == false) {
158+
DiscoveryNodeFilters indexRequireFilters = DiscoveryNodeFilters.trimTier(indexMd.requireFilters());
159+
DiscoveryNodeFilters indexIncludeFilters = DiscoveryNodeFilters.trimTier(indexMd.includeFilters());
160+
DiscoveryNodeFilters indexExcludeFilters = DiscoveryNodeFilters.trimTier(indexMd.excludeFilters());
161+
162+
if (indexRequireFilters != null) {
163+
if (indexRequireFilters.match(node) == false) {
160164
return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]",
161-
IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_PREFIX, indexMd.requireFilters());
165+
IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_PREFIX, indexRequireFilters);
162166
}
163167
}
164-
if (indexMd.includeFilters() != null) {
165-
if (indexMd.includeFilters().match(node) == false) {
168+
if (indexIncludeFilters != null) {
169+
if (indexIncludeFilters.match(node) == false) {
166170
return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]",
167-
IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_PREFIX, indexMd.includeFilters());
171+
IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_PREFIX, indexIncludeFilters);
168172
}
169173
}
170-
if (indexMd.excludeFilters() != null) {
171-
if (indexMd.excludeFilters().match(node)) {
174+
if (indexExcludeFilters != null) {
175+
if (indexExcludeFilters.match(node)) {
172176
return allocation.decision(Decision.NO, NAME, "node matches index setting [%s] filters [%s]",
173-
IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey(), indexMd.excludeFilters());
177+
IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey(), indexExcludeFilters);
174178
}
175179
}
176180
return null;
@@ -199,12 +203,12 @@ private Decision shouldClusterFilter(DiscoveryNode node, RoutingAllocation alloc
199203
}
200204

201205
private void setClusterRequireFilters(Map<String, String> filters) {
202-
clusterRequireFilters = DiscoveryNodeFilters.buildFromKeyValue(AND, filters);
206+
clusterRequireFilters = DiscoveryNodeFilters.trimTier(DiscoveryNodeFilters.buildFromKeyValue(AND, filters));
203207
}
204208
private void setClusterIncludeFilters(Map<String, String> filters) {
205-
clusterIncludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, filters);
209+
clusterIncludeFilters = DiscoveryNodeFilters.trimTier(DiscoveryNodeFilters.buildFromKeyValue(OR, filters));
206210
}
207211
private void setClusterExcludeFilters(Map<String, String> filters) {
208-
clusterExcludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, filters);
212+
clusterExcludeFilters = DiscoveryNodeFilters.trimTier(DiscoveryNodeFilters.buildFromKeyValue(OR, filters));
209213
}
210214
}

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDeciderTests.java‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,54 @@ public void testFilterInitialRecovery() {
140140
assertEquals("node passes include/exclude/require filters", decision.getExplanation());
141141
}
142142

143-
private ClusterState createInitialClusterState(AllocationService service, Settings settings) {
143+
public void testTierFilterIgnored() {
144+
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
145+
FilterAllocationDecider filterAllocationDecider = new FilterAllocationDecider(Settings.EMPTY, clusterSettings);
146+
AllocationDeciders allocationDeciders = new AllocationDeciders(
147+
Arrays.asList(filterAllocationDecider,
148+
new SameShardAllocationDecider(Settings.EMPTY, clusterSettings),
149+
new ReplicaAfterPrimaryActiveAllocationDecider()));
150+
AllocationService service = new AllocationService(allocationDeciders,
151+
new TestGatewayAllocator(), new BalancedShardsAllocator(Settings.EMPTY), EmptyClusterInfoService.INSTANCE,
152+
EmptySnapshotsInfoService.INSTANCE);
153+
ClusterState state = createInitialClusterState(service, Settings.builder()
154+
.put("index.routing.allocation.require._tier", "data_cold")
155+
.put("index.routing.allocation.include._tier", "data_cold")
156+
.put("index.routing.allocation.include._tier_preference", "data_cold")
157+
.put("index.routing.allocation.exclude._tier", "data_cold")
158+
.build(),
159+
Settings.builder()
160+
.put("cluster.routing.allocation.require._tier", "data_cold")
161+
.put("cluster.routing.allocation.include._tier", "data_cold")
162+
.put("cluster.routing.allocation.exclude._tier", "data_cold")
163+
.build());
164+
RoutingTable routingTable = state.routingTable();
165+
RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, state.getRoutingNodes(), state,
166+
null, null, 0);
167+
allocation.debugDecision(true);
168+
allocation = new RoutingAllocation(allocationDeciders, state.getRoutingNodes(), state,
169+
null, null, 0);
170+
allocation.debugDecision(true);
171+
Decision.Single decision = (Decision.Single) filterAllocationDecider.canAllocate(
172+
routingTable.index("idx").shard(0).shards().get(0),
173+
state.getRoutingNodes().node("node2"), allocation);
174+
assertEquals(decision.toString(), Type.YES, decision.type());
175+
assertEquals("node passes include/exclude/require filters", decision.getExplanation());
176+
decision = (Decision.Single) filterAllocationDecider.canAllocate(
177+
routingTable.index("idx").shard(0).shards().get(0),
178+
state.getRoutingNodes().node("node1"), allocation);
179+
assertEquals(Type.YES, decision.type());
180+
assertEquals("node passes include/exclude/require filters", decision.getExplanation());
181+
}
182+
183+
private ClusterState createInitialClusterState(AllocationService service, Settings indexSettings) {
184+
return createInitialClusterState(service, indexSettings, Settings.EMPTY);
185+
}
186+
187+
private ClusterState createInitialClusterState(AllocationService service, Settings idxSettings, Settings clusterSettings) {
144188
Metadata.Builder metadata = Metadata.builder();
145-
final Settings.Builder indexSettings = settings(Version.CURRENT).put(settings);
189+
metadata.persistentSettings(clusterSettings);
190+
final Settings.Builder indexSettings = settings(Version.CURRENT).put(idxSettings);
146191
final IndexMetadata sourceIndex;
147192
//put a fake closed source index
148193
sourceIndex = IndexMetadata.builder("sourceIndex")

‎x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,26 @@ public void testDataTierTelemetry() {
247247
assertThat(usage.getTierStats().get(DataTier.DATA_HOT).primaryShardBytesMAD, greaterThanOrEqualTo(0L));
248248
}
249249

250+
public void testTierFilteringIgnoredByFilterAllocationDecider() {
251+
startContentOnlyNode();
252+
startHotOnlyNode();
253+
254+
// Exclude all data_cold nodes
255+
client().admin().cluster().prepareUpdateSettings()
256+
.setTransientSettings(Settings.builder()
257+
.put(DataTierAllocationDecider.CLUSTER_ROUTING_EXCLUDE, "data_cold")
258+
.build())
259+
.get();
260+
261+
// Create an index, which should be excluded just fine, ignored by the FilterAllocationDecider
262+
client().admin().indices().prepareCreate(index)
263+
.setSettings(Settings.builder()
264+
.put("index.number_of_shards", 2)
265+
.put("index.number_of_replicas", 0))
266+
.setWaitForActiveShards(0)
267+
.get();
268+
}
269+
250270
private DataTiersFeatureSetUsage getUsage() {
251271
XPackUsageResponse usages = new XPackUsageRequestBuilder(client()).execute().actionGet();
252272
XPackFeatureSet.Usage dtUsage = usages.getUsages().stream()

0 commit comments

Comments
 (0)