Skip to content

Commit 32933f3

Browse files
alexey-ivanov-esalexey-ivanov-es
authored
GET /_migration/deprecations doesn't check disk watermarks against correct settings values (elastic#138115) (elastic#138818)
This change moves the disk watermark check from all nodes to the master node since the check is performed against information in ClusterInfo. Also, it changes what setting values are used - using values from the cluster state if they are set, or from node settings otherwise. There's a potential edge case: in a mixed cluster (e.g. master 9.2.1 (before the fix), other nodes 9.3 (after the fix)), the check wouldn't be performed. However, due to elastic#137004, the check isn't working in the released versions anyway, so this doesn't make things worse. Closes elastic#137005
1 parent b2dd138 commit 32933f3

File tree

7 files changed

+293
-238
lines changed

7 files changed

+293
-238
lines changed

‎docs/changelog/138115.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 138115
2+
summary: GET /_migration/deprecations doesn't check disk watermarks against correct
3+
settings values
4+
area: Infra/Core
5+
type: bug
6+
issues:
7+
- 137005

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,12 @@ public ByteSizeValue getFreeBytesThresholdLowStage(ByteSizeValue total) {
487487
return getFreeBytesThreshold(total, lowStageWatermark, lowStageMaxHeadroom);
488488
}
489489

490+
public static ByteSizeValue getFreeBytesThresholdLowStage(ByteSizeValue total, ClusterSettings clusterSettings) {
491+
var lowStageWatermark = clusterSettings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
492+
var lowStageMaxHeadroom = clusterSettings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_MAX_HEADROOM_SETTING);
493+
return getFreeBytesThreshold(total, lowStageWatermark, lowStageMaxHeadroom);
494+
}
495+
490496
public ByteSizeValue getFreeBytesThresholdHighStage(ByteSizeValue total) {
491497
return getFreeBytesThreshold(total, highStageWatermark, highStageMaxHeadroom);
492498
}

‎x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportDeprecationInfoAction.java‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
1616
import org.elasticsearch.client.internal.OriginSettingClient;
1717
import org.elasticsearch.client.internal.node.NodeClient;
18+
import org.elasticsearch.cluster.ClusterInfo;
19+
import org.elasticsearch.cluster.ClusterInfoService;
1820
import org.elasticsearch.cluster.ClusterState;
21+
import org.elasticsearch.cluster.DiskUsage;
1922
import org.elasticsearch.cluster.block.ClusterBlockException;
2023
import org.elasticsearch.cluster.block.ClusterBlockLevel;
2124
import org.elasticsearch.cluster.metadata.ComponentTemplate;
@@ -25,11 +28,17 @@
2528
import org.elasticsearch.cluster.metadata.Metadata;
2629
import org.elasticsearch.cluster.metadata.ProjectMetadata;
2730
import org.elasticsearch.cluster.metadata.Template;
31+
import org.elasticsearch.cluster.node.DiscoveryNode;
32+
import org.elasticsearch.cluster.node.DiscoveryNodes;
2833
import org.elasticsearch.cluster.project.ProjectResolver;
34+
import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings;
2935
import org.elasticsearch.cluster.service.ClusterService;
3036
import org.elasticsearch.common.regex.Regex;
37+
import org.elasticsearch.common.settings.ClusterSettings;
3138
import org.elasticsearch.common.settings.Setting;
3239
import org.elasticsearch.common.settings.Settings;
40+
import org.elasticsearch.common.unit.ByteSizeValue;
41+
import org.elasticsearch.common.util.CollectionUtils;
3342
import org.elasticsearch.core.Tuple;
3443
import org.elasticsearch.injection.guice.Inject;
3544
import org.elasticsearch.tasks.Task;
@@ -46,10 +55,14 @@
4655
import java.util.Collections;
4756
import java.util.HashMap;
4857
import java.util.List;
58+
import java.util.Locale;
4959
import java.util.Map;
60+
import java.util.Objects;
5061
import java.util.stream.Collectors;
5162
import java.util.stream.Stream;
5263

64+
import static org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING;
65+
5366
public class TransportDeprecationInfoAction extends TransportMasterNodeReadAction<
5467
DeprecationInfoAction.Request,
5568
DeprecationInfoAction.Response> {
@@ -64,6 +77,7 @@ public class TransportDeprecationInfoAction extends TransportMasterNodeReadActio
6477
private final IndexNameExpressionResolver indexNameExpressionResolver;
6578
private final Settings settings;
6679
private final NamedXContentRegistry xContentRegistry;
80+
private final ClusterInfoService clusterInfoService;
6781
private volatile List<String> skipTheseDeprecations;
6882
private final NodeDeprecationChecker nodeDeprecationChecker;
6983
private final ClusterDeprecationChecker clusterDeprecationChecker;
@@ -80,6 +94,7 @@ public TransportDeprecationInfoAction(
8094
IndexNameExpressionResolver indexNameExpressionResolver,
8195
NodeClient client,
8296
NamedXContentRegistry xContentRegistry,
97+
ClusterInfoService clusterInfoService,
8398
ProjectResolver projectResolver
8499
) {
85100
super(
@@ -96,6 +111,7 @@ public TransportDeprecationInfoAction(
96111
this.indexNameExpressionResolver = indexNameExpressionResolver;
97112
this.settings = settings;
98113
this.xContentRegistry = xContentRegistry;
114+
this.clusterInfoService = clusterInfoService;
99115
skipTheseDeprecations = SKIP_DEPRECATIONS_SETTING.get(settings);
100116
nodeDeprecationChecker = new NodeDeprecationChecker(threadPool);
101117
clusterDeprecationChecker = new ClusterDeprecationChecker(xContentRegistry);
@@ -127,7 +143,13 @@ protected final void masterOperation(
127143
ClusterState state,
128144
final ActionListener<DeprecationInfoAction.Response> listener
129145
) {
130-
PrecomputedData precomputedData = new PrecomputedData();
146+
DeprecationIssue lowWatermarkIssue = checkDiskLowWatermark(
147+
clusterService.getClusterSettings(),
148+
clusterInfoService.getClusterInfo(),
149+
state.nodes()
150+
);
151+
PrecomputedData precomputedData = new PrecomputedData(lowWatermarkIssue);
152+
131153
final var project = projectResolver.getProjectMetadata(state);
132154
try (var refs = new RefCountingListener(checkAndCreateResponse(project, request, precomputedData, listener))) {
133155
nodeDeprecationChecker.check(client, refs.acquire(precomputedData::setOnceNodeSettingsIssues));
@@ -155,7 +177,7 @@ protected final void masterOperation(
155177
* @return The listener that should be executed after all the remote requests have completed and the {@link PrecomputedData}
156178
* is initialised.
157179
*/
158-
public ActionListener<Void> checkAndCreateResponse(
180+
private ActionListener<Void> checkAndCreateResponse(
159181
ProjectMetadata project,
160182
DeprecationInfoAction.Request request,
161183
PrecomputedData precomputedData,
@@ -241,6 +263,11 @@ public static class PrecomputedData {
241263
private final SetOnce<List<DeprecationIssue>> nodeSettingsIssues = new SetOnce<>();
242264
private final SetOnce<Map<String, List<DeprecationIssue>>> pluginIssues = new SetOnce<>();
243265
private final SetOnce<List<TransformConfig>> transformConfigs = new SetOnce<>();
266+
private final DeprecationIssue diskWatermarkIssue;
267+
268+
public PrecomputedData(DeprecationIssue diskWatermarkIssue) {
269+
this.diskWatermarkIssue = diskWatermarkIssue;
270+
}
244271

245272
public void setOnceNodeSettingsIssues(List<DeprecationIssue> nodeSettingsIssues) {
246273
this.nodeSettingsIssues.set(nodeSettingsIssues);
@@ -255,7 +282,12 @@ public void setOnceTransformConfigs(List<TransformConfig> transformConfigs) {
255282
}
256283

257284
public List<DeprecationIssue> nodeSettingsIssues() {
258-
return nodeSettingsIssues.get();
285+
List<DeprecationIssue> deprecationIssues = nodeSettingsIssues.get();
286+
assert deprecationIssues != null : "nodeSettingsIssues must be set before calling this method";
287+
if (diskWatermarkIssue == null) {
288+
return deprecationIssues;
289+
}
290+
return CollectionUtils.appendToCopy(deprecationIssues, diskWatermarkIssue);
259291
}
260292

261293
public Map<String, List<DeprecationIssue>> pluginIssues() {
@@ -396,4 +428,44 @@ private void transformConfigs(PageParams currentPage, ActionListener<Stream<Tran
396428
private <T> ActionListener<T> executeInGenericThreadpool(ActionListener<T> listener) {
397429
return new ThreadedActionListener<>(threadPool.generic(), listener);
398430
}
431+
432+
static DeprecationIssue checkDiskLowWatermark(ClusterSettings clusterSettings, ClusterInfo clusterInfo, DiscoveryNodes discoveryNodes) {
433+
Map<String, DiskUsage> nodeMostAvailableDiskUsages = clusterInfo.getNodeMostAvailableDiskUsages();
434+
435+
List<String> impactedNodeNames = nodeMostAvailableDiskUsages.entrySet()
436+
.stream()
437+
.filter(e -> exceedsLowWatermark(clusterSettings, e.getValue()))
438+
.map(Map.Entry::getKey)
439+
.map(discoveryNodes::get)
440+
.filter(Objects::nonNull)
441+
.map(DiscoveryNode::getName)
442+
.sorted()
443+
.toList();
444+
445+
if (impactedNodeNames.isEmpty()) {
446+
return null;
447+
}
448+
449+
return new DeprecationIssue(
450+
DeprecationIssue.Level.CRITICAL,
451+
"Disk usage exceeds low watermark",
452+
"https://ela.st/es-deprecation-7-disk-watermark-exceeded",
453+
String.format(
454+
Locale.ROOT,
455+
"Disk usage exceeds low watermark, which will prevent reindexing indices during upgrade. Get disk usage on "
456+
+ "all nodes below the value specified in %s (nodes impacted: %s)",
457+
CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(),
458+
impactedNodeNames
459+
),
460+
false,
461+
null
462+
);
463+
}
464+
465+
private static boolean exceedsLowWatermark(ClusterSettings clusterSettings, DiskUsage usage) {
466+
long freeBytes = usage.freeBytes();
467+
long totalBytes = usage.totalBytes();
468+
return freeBytes < DiskThresholdSettings.getFreeBytesThresholdLowStage(ByteSizeValue.ofBytes(totalBytes), clusterSettings)
469+
.getBytes();
470+
}
399471
}

‎x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java‎

Lines changed: 6 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,13 @@
1111
import org.elasticsearch.action.admin.cluster.node.info.PluginsAndModules;
1212
import org.elasticsearch.action.support.ActionFilters;
1313
import org.elasticsearch.action.support.nodes.TransportNodesAction;
14-
import org.elasticsearch.cluster.ClusterInfo;
15-
import org.elasticsearch.cluster.ClusterInfoService;
1614
import org.elasticsearch.cluster.ClusterState;
17-
import org.elasticsearch.cluster.DiskUsage;
1815
import org.elasticsearch.cluster.metadata.Metadata;
1916
import org.elasticsearch.cluster.node.DiscoveryNode;
20-
import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings;
2117
import org.elasticsearch.cluster.service.ClusterService;
2218
import org.elasticsearch.common.io.stream.StreamInput;
2319
import org.elasticsearch.common.regex.Regex;
24-
import org.elasticsearch.common.settings.ClusterSettings;
2520
import org.elasticsearch.common.settings.Settings;
26-
import org.elasticsearch.common.unit.ByteSizeValue;
2721
import org.elasticsearch.injection.guice.Inject;
2822
import org.elasticsearch.license.XPackLicenseState;
2923
import org.elasticsearch.plugins.PluginsService;
@@ -33,12 +27,9 @@
3327
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
3428

3529
import java.io.IOException;
36-
import java.util.ArrayList;
3730
import java.util.Collections;
3831
import java.util.List;
39-
import java.util.Locale;
40-
41-
import static org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING;
32+
import java.util.Objects;
4233

4334
public class TransportNodeDeprecationCheckAction extends TransportNodesAction<
4435
NodesDeprecationCheckRequest,
@@ -50,7 +41,6 @@ public class TransportNodeDeprecationCheckAction extends TransportNodesAction<
5041
private final Settings settings;
5142
private final XPackLicenseState licenseState;
5243
private final PluginsService pluginsService;
53-
private final ClusterInfoService clusterInfoService;
5444
private volatile List<String> skipTheseDeprecations;
5545

5646
@Inject
@@ -61,8 +51,7 @@ public TransportNodeDeprecationCheckAction(
6151
ClusterService clusterService,
6252
TransportService transportService,
6353
PluginsService pluginsService,
64-
ActionFilters actionFilters,
65-
ClusterInfoService clusterInfoService
54+
ActionFilters actionFilters
6655
) {
6756
super(
6857
NodesDeprecationCheckAction.NAME,
@@ -75,7 +64,6 @@ public TransportNodeDeprecationCheckAction(
7564
this.settings = settings;
7665
this.pluginsService = pluginsService;
7766
this.licenseState = licenseState;
78-
this.clusterInfoService = clusterInfoService;
7967
skipTheseDeprecations = TransportDeprecationInfoAction.SKIP_DEPRECATIONS_SETTING.get(settings);
8068
// Safe to register this here because it happens synchronously before the cluster service is started:
8169
clusterService.getClusterSettings()
@@ -130,67 +118,10 @@ NodesDeprecationCheckAction.NodeResponse nodeOperation(
130118
.metadata(Metadata.builder(metadata).transientSettings(transientSettings).persistentSettings(persistentSettings).build())
131119
.build();
132120

133-
List<DeprecationIssue> issues = new ArrayList<>();
134-
for (NodeDeprecationChecks.NodeDeprecationCheck<
135-
Settings,
136-
PluginsAndModules,
137-
ClusterState,
138-
XPackLicenseState,
139-
DeprecationIssue> c : nodeSettingsChecks) {
140-
DeprecationIssue deprecationIssue = c.apply(filteredNodeSettings, pluginsService.info(), filteredClusterState, licenseState);
141-
if (deprecationIssue != null) {
142-
issues.add(deprecationIssue);
143-
}
144-
}
145-
DeprecationIssue watermarkIssue = checkDiskLowWatermark(
146-
filteredNodeSettings,
147-
filteredClusterState.metadata().settings(),
148-
clusterInfoService.getClusterInfo(),
149-
clusterService.getClusterSettings(),
150-
transportService.getLocalNode().getId()
151-
);
152-
if (watermarkIssue != null) {
153-
issues.add(watermarkIssue);
154-
}
121+
List<DeprecationIssue> issues = nodeSettingsChecks.stream()
122+
.map(c -> c.apply(filteredNodeSettings, pluginsService.info(), filteredClusterState, licenseState))
123+
.filter(Objects::nonNull)
124+
.toList();
155125
return new NodesDeprecationCheckAction.NodeResponse(transportService.getLocalNode(), issues);
156126
}
157-
158-
static DeprecationIssue checkDiskLowWatermark(
159-
Settings nodeSettings,
160-
Settings dynamicSettings,
161-
ClusterInfo clusterInfo,
162-
ClusterSettings clusterSettings,
163-
String nodeId
164-
) {
165-
DiskUsage usage = clusterInfo.getNodeMostAvailableDiskUsages().get(nodeId);
166-
if (usage != null) {
167-
long freeBytes = usage.freeBytes();
168-
long totalBytes = usage.totalBytes();
169-
if (exceedsLowWatermark(nodeSettings, clusterSettings, freeBytes, totalBytes)
170-
|| exceedsLowWatermark(dynamicSettings, clusterSettings, freeBytes, totalBytes)) {
171-
return new DeprecationIssue(
172-
DeprecationIssue.Level.CRITICAL,
173-
"Disk usage exceeds low watermark",
174-
"https://ela.st/es-deprecation-7-disk-watermark-exceeded",
175-
String.format(
176-
Locale.ROOT,
177-
"Disk usage exceeds low watermark, which will prevent reindexing indices during upgrade. Get disk usage on "
178-
+ "all nodes below the value specified in %s",
179-
CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey()
180-
),
181-
false,
182-
null
183-
);
184-
}
185-
}
186-
return null;
187-
}
188-
189-
private static boolean exceedsLowWatermark(Settings settingsToCheck, ClusterSettings clusterSettings, long freeBytes, long totalBytes) {
190-
DiskThresholdSettings diskThresholdSettings = new DiskThresholdSettings(settingsToCheck, clusterSettings);
191-
if (freeBytes < diskThresholdSettings.getFreeBytesThresholdLowStage(ByteSizeValue.ofBytes(totalBytes)).getBytes()) {
192-
return true;
193-
}
194-
return false;
195-
}
196127
}

‎x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ public class IndexDeprecationCheckerTests extends ESTestCase {
4949
private static final IndexVersion OLD_VERSION = IndexVersion.fromId(7170099);
5050
private final IndexNameExpressionResolver indexNameExpressionResolver = TestIndexNameExpressionResolver.newInstance();
5151
private final IndexDeprecationChecker checker = new IndexDeprecationChecker(indexNameExpressionResolver);
52-
private final TransportDeprecationInfoAction.PrecomputedData emptyPrecomputedData =
53-
new TransportDeprecationInfoAction.PrecomputedData();
52+
private final TransportDeprecationInfoAction.PrecomputedData emptyPrecomputedData = new TransportDeprecationInfoAction.PrecomputedData(
53+
null
54+
);
5455
private final IndexMetadata.State indexMetdataState;
5556

5657
public IndexDeprecationCheckerTests(@Name("indexMetadataState") IndexMetadata.State indexMetdataState) {
@@ -723,7 +724,7 @@ private TransportDeprecationInfoAction.PrecomputedData createContextWithTransfor
723724
);
724725
}
725726
}
726-
TransportDeprecationInfoAction.PrecomputedData precomputedData = new TransportDeprecationInfoAction.PrecomputedData();
727+
TransportDeprecationInfoAction.PrecomputedData precomputedData = new TransportDeprecationInfoAction.PrecomputedData(null);
727728
precomputedData.setOnceTransformConfigs(transforms);
728729
return precomputedData;
729730
}

0 commit comments

Comments
 (0)