Skip to content

Commit 6c33c94

Browse files
authored
HasFrozenCacheAllocationDecider returns No decision for unknown node (#137232)
Since SearchableSnapshotAllocator updates frozen cache service before allocation, the unknown state for a node can only happen when the node leaves the cluster. In this case, it makes more sense to reject the allocation instead of throttling since the node may not come back. This is also more consistent with the behaviour of SearchableSnapshotAllocator which does not consider any node that is not currently in the cluster (effectively a No). If the node does come back, it will go through again the cache info fetching steps. Relates: #136794
1 parent c81eee5 commit 6c33c94

File tree

2 files changed

+73
-2
lines changed

2 files changed

+73
-2
lines changed

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/decider/HasFrozenCacheAllocationDecider.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ public class HasFrozenCacheAllocationDecider extends AllocationDecider {
4848
"there was an error fetching the searchable snapshot shared cache state from this node"
4949
);
5050

51+
private static final Decision NO_UNKNOWN_NODE = Decision.single(
52+
Decision.Type.NO,
53+
NAME,
54+
"this node is unknown to the searchable snapshot shared cache state"
55+
);
56+
5157
private final FrozenCacheInfoService frozenCacheService;
5258

5359
public HasFrozenCacheAllocationDecider(FrozenCacheInfoService frozenCacheService) {
@@ -74,7 +80,8 @@ public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNod
7480
return canAllocateToNode(indexMetadata, node);
7581
}
7682

77-
private Decision canAllocateToNode(IndexMetadata indexMetadata, DiscoveryNode discoveryNode) {
83+
// Package private for tests
84+
Decision canAllocateToNode(IndexMetadata indexMetadata, DiscoveryNode discoveryNode) {
7885
if (indexMetadata.isPartialSearchableSnapshot() == false) {
7986
return Decision.ALWAYS;
8087
}
@@ -83,7 +90,8 @@ private Decision canAllocateToNode(IndexMetadata indexMetadata, DiscoveryNode di
8390
case HAS_CACHE -> HAS_FROZEN_CACHE;
8491
case NO_CACHE -> NO_FROZEN_CACHE;
8592
case FAILED -> UNKNOWN_FROZEN_CACHE;
86-
default -> STILL_FETCHING;
93+
case FETCHING -> STILL_FETCHING;
94+
case UNKNOWN -> NO_UNKNOWN_NODE;
8795
};
8896
}
8997

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.searchablesnapshots.allocation.decider;
9+
10+
import org.elasticsearch.cluster.metadata.IndexMetadata;
11+
import org.elasticsearch.cluster.node.DiscoveryNode;
12+
import org.elasticsearch.cluster.routing.allocation.decider.Decision;
13+
import org.elasticsearch.index.IndexVersion;
14+
import org.elasticsearch.test.ESTestCase;
15+
import org.elasticsearch.xpack.searchablesnapshots.cache.shared.FrozenCacheInfoService;
16+
import org.elasticsearch.xpack.searchablesnapshots.cache.shared.FrozenCacheInfoService.NodeState;
17+
18+
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
19+
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY;
20+
import static org.hamcrest.Matchers.equalTo;
21+
import static org.hamcrest.Matchers.is;
22+
import static org.mockito.ArgumentMatchers.any;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
25+
26+
public class HasFrozenCacheAllocationDeciderTests extends ESTestCase {
27+
28+
public void testCanAllocateToNode() {
29+
final var frozenCacheService = mock(FrozenCacheInfoService.class);
30+
final var decider = new HasFrozenCacheAllocationDecider(frozenCacheService);
31+
32+
final var indexMetadata = IndexMetadata.builder(randomIdentifier())
33+
.settings(indexSettings(IndexVersion.current(), between(1, 10), between(0, 2)))
34+
.build();
35+
36+
for (var nodeState : NodeState.values()) {
37+
when(frozenCacheService.getNodeState(any(DiscoveryNode.class))).thenReturn(nodeState);
38+
assertThat(decider.canAllocateToNode(indexMetadata, mock(DiscoveryNode.class)), equalTo(Decision.ALWAYS));
39+
}
40+
41+
final var partialSearchableSnapshotIndexMetadata = IndexMetadata.builder(randomIdentifier())
42+
.settings(
43+
indexSettings(IndexVersion.current(), between(1, 10), between(0, 2)).put(INDEX_STORE_TYPE_SETTING.getKey(), "snapshot")
44+
.put(SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY, true)
45+
)
46+
.build();
47+
48+
for (var nodeState : NodeState.values()) {
49+
when(frozenCacheService.getNodeState(any(DiscoveryNode.class))).thenReturn(nodeState);
50+
final Decision decision = decider.canAllocateToNode(partialSearchableSnapshotIndexMetadata, mock(DiscoveryNode.class));
51+
final Decision.Type expectedType;
52+
if (nodeState == NodeState.HAS_CACHE) {
53+
expectedType = Decision.Type.YES;
54+
} else if (nodeState == NodeState.FETCHING) {
55+
expectedType = Decision.Type.THROTTLE;
56+
} else {
57+
expectedType = Decision.Type.NO;
58+
}
59+
assertThat("incorrect decision for " + nodeState, decision.type(), is(expectedType));
60+
}
61+
}
62+
63+
}

0 commit comments

Comments
 (0)