Skip to content

Commit 3ae08c8

Browse files
original-brownbearoriginal-brownbear
authored
Cleanup DataTierAllocationDecider (#83572)
Clean this code up a little more as its one of the remaining rather expensive allocation deciders in profiling: * Make static what can be made static * extract cold paths to maybe make this compile a little better * Use the same logic in `tierNodesPresent` and `allocationAllowed` to have the `DATA_ROLE` shortcut in both.
1 parent facec15 commit 3ae08c8

File tree

10 files changed

+95
-132
lines changed

10 files changed

+95
-132
lines changed

‎x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/storage/ProactiveStorageDeciderService.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderContext;
2222
import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderResult;
2323
import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderService;
24-
import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider;
2524

2625
import java.io.IOException;
2726
import java.util.List;
@@ -33,11 +32,9 @@ public class ProactiveStorageDeciderService implements AutoscalingDeciderService
3332

3433
private final DiskThresholdSettings diskThresholdSettings;
3534
private final AllocationDeciders allocationDeciders;
36-
private final DataTierAllocationDecider dataTierAllocationDecider;
3735

3836
public ProactiveStorageDeciderService(Settings settings, ClusterSettings clusterSettings, AllocationDeciders allocationDeciders) {
3937
this.diskThresholdSettings = new DiskThresholdSettings(settings, clusterSettings);
40-
this.dataTierAllocationDecider = new DataTierAllocationDecider();
4138
this.allocationDeciders = allocationDeciders;
4239
}
4340

@@ -64,8 +61,7 @@ public AutoscalingDeciderResult scale(Settings configuration, AutoscalingDecider
6461
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
6562
context,
6663
diskThresholdSettings,
67-
allocationDeciders,
68-
dataTierAllocationDecider
64+
allocationDeciders
6965
);
7066
long unassignedBytesBeforeForecast = allocationState.storagePreventsAllocation();
7167
assert unassignedBytesBeforeForecast >= 0;

‎x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderService.java

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,10 @@ public class ReactiveStorageDeciderService implements AutoscalingDeciderService
7171
public static final String NAME = "reactive_storage";
7272

7373
private final DiskThresholdSettings diskThresholdSettings;
74-
private final DataTierAllocationDecider dataTierAllocationDecider;
7574
private final AllocationDeciders allocationDeciders;
7675

7776
public ReactiveStorageDeciderService(Settings settings, ClusterSettings clusterSettings, AllocationDeciders allocationDeciders) {
7877
this.diskThresholdSettings = new DiskThresholdSettings(settings, clusterSettings);
79-
this.dataTierAllocationDecider = new DataTierAllocationDecider();
8078
this.allocationDeciders = allocationDeciders;
8179
}
8280

@@ -108,12 +106,7 @@ public AutoscalingDeciderResult scale(Settings configuration, AutoscalingDecider
108106
return new AutoscalingDeciderResult(null, new ReactiveReason("current capacity not available", -1, -1));
109107
}
110108

111-
AllocationState allocationState = new AllocationState(
112-
context,
113-
diskThresholdSettings,
114-
allocationDeciders,
115-
dataTierAllocationDecider
116-
);
109+
AllocationState allocationState = new AllocationState(context, diskThresholdSettings, allocationDeciders);
117110
long unassignedBytes = allocationState.storagePreventsAllocation();
118111
long assignedBytes = allocationState.storagePreventsRemainOrMove();
119112
long maxShardSize = allocationState.maxShardSize();
@@ -177,7 +170,6 @@ static Optional<String> singleNoDecision(Decision decision, Predicate<Decision>
177170
public static class AllocationState {
178171
private final ClusterState state;
179172
private final AllocationDeciders allocationDeciders;
180-
private final DataTierAllocationDecider dataTierAllocationDecider;
181173
private final DiskThresholdSettings diskThresholdSettings;
182174
private final ClusterInfo info;
183175
private final SnapshotShardSizeInfo shardSizeInfo;
@@ -189,13 +181,11 @@ public static class AllocationState {
189181
AllocationState(
190182
AutoscalingDeciderContext context,
191183
DiskThresholdSettings diskThresholdSettings,
192-
AllocationDeciders allocationDeciders,
193-
DataTierAllocationDecider dataTierAllocationDecider
184+
AllocationDeciders allocationDeciders
194185
) {
195186
this(
196187
context.state(),
197188
allocationDeciders,
198-
dataTierAllocationDecider,
199189
diskThresholdSettings,
200190
context.info(),
201191
context.snapshotShardSizeInfo(),
@@ -207,7 +197,6 @@ public static class AllocationState {
207197
AllocationState(
208198
ClusterState state,
209199
AllocationDeciders allocationDeciders,
210-
DataTierAllocationDecider dataTierAllocationDecider,
211200
DiskThresholdSettings diskThresholdSettings,
212201
ClusterInfo info,
213202
SnapshotShardSizeInfo shardSizeInfo,
@@ -216,7 +205,6 @@ public static class AllocationState {
216205
) {
217206
this.state = state;
218207
this.allocationDeciders = allocationDeciders;
219-
this.dataTierAllocationDecider = dataTierAllocationDecider;
220208
this.diskThresholdSettings = diskThresholdSettings;
221209
this.info = info;
222210
this.shardSizeInfo = shardSizeInfo;
@@ -347,7 +335,7 @@ boolean needsThisTier(ShardRouting shard, RoutingAllocation allocation) {
347335
Set<Decision.Type> decisionTypes = allocation.routingNodes()
348336
.stream()
349337
.map(
350-
node -> dataTierAllocationDecider.shouldFilter(
338+
node -> DataTierAllocationDecider.shouldFilter(
351339
indexMetadata,
352340
node.node().getRoles(),
353341
this::highestPreferenceTier,
@@ -380,7 +368,7 @@ boolean needsThisTier(ShardRouting shard, RoutingAllocation allocation) {
380368

381369
private boolean isAssignedToTier(ShardRouting shard, RoutingAllocation allocation) {
382370
IndexMetadata indexMetadata = indexMetadata(shard, allocation);
383-
return dataTierAllocationDecider.shouldFilter(indexMetadata, roles, this::highestPreferenceTier, allocation) != Decision.NO;
371+
return DataTierAllocationDecider.shouldFilter(indexMetadata, roles, this::highestPreferenceTier, allocation) != Decision.NO;
384372
}
385373

386374
private IndexMetadata indexMetadata(ShardRouting shard, RoutingAllocation allocation) {
@@ -504,7 +492,6 @@ public AllocationState forecast(long forecastWindow, long now) {
504492
return new AllocationState(
505493
forecastClusterState,
506494
allocationDeciders,
507-
dataTierAllocationDecider,
508495
diskThresholdSettings,
509496
forecastInfo,
510497
shardSizeInfo,

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/ProactiveStorageDeciderServiceTests.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ public void testForecastNoDates() {
178178
null,
179179
null,
180180
null,
181-
null,
182181
Set.of(),
183182
Set.of()
184183
);
@@ -209,7 +208,6 @@ public void testForecastZero() {
209208
state,
210209
null,
211210
null,
212-
null,
213211
randomClusterInfo(state),
214212
null,
215213
Sets.newHashSet(state.nodes()),
@@ -254,7 +252,6 @@ public void testForecast() {
254252
state,
255253
null,
256254
null,
257-
null,
258255
info,
259256
null,
260257
Sets.newHashSet(state.nodes()),

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderDecisionTests.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,6 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl
9696
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)
9797
);
9898

99-
private static final DataTierAllocationDecider DATA_TIER_ALLOCATION_DECIDER = new DataTierAllocationDecider();
100-
10199
private ClusterState state;
102100
private final int hotNodes = randomIntBetween(1, 8);
103101
private final int warmNodes = randomIntBetween(1, 3);
@@ -366,8 +364,7 @@ private static void verify(
366364
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
367365
createContext(state, Set.of(role)),
368366
DISK_THRESHOLD_SETTINGS,
369-
createAllocationDeciders(allocationDeciders),
370-
DATA_TIER_ALLOCATION_DECIDER
367+
createAllocationDeciders(allocationDeciders)
371368
);
372369
assertThat(subject.invoke(allocationState), equalTo(expected));
373370
}
@@ -429,7 +426,7 @@ private static AllocationDeciders createAllocationDeciders(AllocationDecider...
429426
Collections.emptyList()
430427
);
431428
return new AllocationDeciders(
432-
Stream.of(Stream.of(extraDeciders), Stream.of(new DataTierAllocationDecider()), systemAllocationDeciders.stream())
429+
Stream.of(Stream.of(extraDeciders), Stream.of(DataTierAllocationDecider.INSTANCE), systemAllocationDeciders.stream())
433430
.flatMap(s -> s)
434431
.collect(Collectors.toList())
435432
);

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderServiceTests.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,6 @@ public void validateSizeOf(
280280
clusterState,
281281
null,
282282
null,
283-
null,
284283
info,
285284
null,
286285
Set.of(),
@@ -358,7 +357,6 @@ private void validateSizeOfSnapshotShard(
358357
null,
359358
null,
360359
null,
361-
null,
362360
shardSizeInfo,
363361
Set.of(),
364362
Set.of()
@@ -448,7 +446,6 @@ public void testUnmovableSize() {
448446
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
449447
clusterState,
450448
null,
451-
null,
452449
thresholdSettings,
453450
info,
454451
null,
@@ -521,11 +518,9 @@ public ClusterState addPreference(IndexMetadata indexMetadata, ClusterState clus
521518

522519
public boolean canRemainWithNoNodes(ClusterState clusterState, ShardRouting shardRouting, AllocationDecider... deciders) {
523520
AllocationDeciders allocationDeciders = new AllocationDeciders(Arrays.asList(deciders));
524-
DataTierAllocationDecider dataTierAllocationDecider = new DataTierAllocationDecider();
525521
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
526522
clusterState,
527523
allocationDeciders,
528-
dataTierAllocationDecider,
529524
new DiskThresholdSettings(Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)),
530525
ClusterInfo.EMPTY,
531526
null,
@@ -626,11 +621,9 @@ private void verifyNeedsWarmTier(
626621
AllocationDecider... deciders
627622
) {
628623
AllocationDeciders allocationDeciders = new AllocationDeciders(Arrays.asList(deciders));
629-
DataTierAllocationDecider dataTierAllocationDecider = new DataTierAllocationDecider();
630624
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
631625
clusterState,
632626
allocationDeciders,
633-
dataTierAllocationDecider,
634627
new DiskThresholdSettings(Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)),
635628
ClusterInfo.EMPTY,
636629
null,

0 commit comments

Comments
 (0)