Skip to content

Commit

Permalink
Support "dry run" mode for updating Desired Nodes (#88305)
Browse files Browse the repository at this point in the history
Add the dry_run query parameter to support simulating of updating of desired nodes. The update request will be validated, but no cluster state updates will be performed. In order to indicate that the response was a result of a dry run, we add the dry_run run field to the JSON representation of a response.

See #82975
  • Loading branch information
arteam committed Jul 26, 2022
1 parent f8a8df4 commit 72a6fdc
Show file tree
Hide file tree
Showing 16 changed files with 267 additions and 33 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/88305.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 88305
summary: Support "dry run" mode for updating Desired Nodes
area: Distributed
type: enhancement
issues: []
10 changes: 9 additions & 1 deletion docs/reference/cluster/update-desired-nodes.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ DELETE /_internal/desired_nodes

include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=master-timeout]

`dry_run`::
(Optional, Boolean) If `true`, then the request simulates the update and
returns a response with `dry_run` field set to `true`.

[[update-desired-nodes-desc]]
==== {api-description-title}

Expand All @@ -58,6 +62,9 @@ this API to let Elasticsearch know about the cluster topology, including future
changes such as adding or removing nodes. Using this information, the system is
able to take better decisions.

It's possible to run the update in "dry run" mode by adding the
`?dry_run` query parameter. This will validate the request result, but will not actually perform the update.

[[update-desired-nodes-examples]]
==== {api-examples-title}

Expand Down Expand Up @@ -92,7 +99,8 @@ The API returns the following result:
[source,console-result]
--------------------------------------------------
{
"replaced_existing_history_id": false
"replaced_existing_history_id": false,
"dry_run": false
}
--------------------------------------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@
}
]
},
"params": {
"dry_run": {
"type": "boolean",
"description": "Simulate the update"
}
},
"body":{
"description":"the specification of the desired nodes",
"required":true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
---
setup:
- skip:
version: " - 8.3.99"
reason: "Support for the dry run option was added in in 8.4.0"

---
"Test dry run doesn't update empty desired nodes":
- do:
cluster.state: {}

- set: { master_node: master }

- do:
nodes.info: {}
- set: { nodes.$master.version: es_version }

- do:
_internal.update_desired_nodes:
history_id: "test"
version: 1
dry_run: true
body:
nodes:
- { settings: { "node.name": "instance-000187" }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
- match: { replaced_existing_history_id: false }
- match: { dry_run: true }

- do:
catch: missing
_internal.get_desired_nodes: {}
- match: { status: 404 }

---
"Test dry run doesn't update existing desired nodes":
- do:
cluster.state: {}

- set: { master_node: master }

- do:
nodes.info: {}
- set: { nodes.$master.version: es_version }

- do:
_internal.update_desired_nodes:
history_id: "test"
version: 1
body:
nodes:
- { settings: { "node.name": "instance-000187" }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
- match: { replaced_existing_history_id: false }
- match: { dry_run: false }

- do:
_internal.get_desired_nodes: {}
- match:
$body:
history_id: "test"
version: 1
nodes:
- { settings: { node: { name: "instance-000187" } }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }

- do:
_internal.update_desired_nodes:
history_id: "test"
version: 2
dry_run: "true"
body:
nodes:
- { settings: { "node.name": "instance-000187" }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
- { settings: { "node.name": "instance-000188" }, processors: 16.0, memory: "128gb", storage: "1tb", node_version: $es_version }
- match: { replaced_existing_history_id: false }
- match: { dry_run: true }

- do:
_internal.get_desired_nodes: { }
- match:
$body:
history_id: "test"
version: 1
nodes:
- { settings: { node: { name: "instance-000187" } }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
---
"Test validation works for dry run updates":
- do:
cluster.state: { }

- set: { master_node: master }

- do:
nodes.info: { }
- set: { nodes.$master.version: es_version }

- do:
catch: bad_request
_internal.update_desired_nodes:
history_id: "test"
version: 1
dry_run: "true"
body:
nodes:
- { settings: { "node.external_id": "instance-000245", "random_setting": -42 }, processors: 16.0, memory: "128gb", storage: "1tb", node_version: $es_version }
- match: { status: 400 }
- match: { error.type: illegal_argument_exception }
- match: { error.reason: "Nodes with ids [instance-000245] in positions [0] contain invalid settings" }
- match: { error.suppressed.0.reason: "unknown setting [random_setting] please check that any required plugins are installed, or check the breaking changes documentation for removed settings" }
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,48 @@ public void testUpdateDesiredNodes() {
final var updateDesiredNodesRequest = randomUpdateDesiredNodesRequest();
final var response = updateDesiredNodes(updateDesiredNodesRequest);
assertThat(response.hasReplacedExistingHistoryId(), is(equalTo(false)));
assertThat(response.dryRun(), is(equalTo(false)));

final DesiredNodes latestDesiredNodes = getLatestDesiredNodes();
assertStoredDesiredNodesAreCorrect(updateDesiredNodesRequest, latestDesiredNodes);
}

public void testDryRunUpdateDoesNotUpdateEmptyDesiredNodes() {
UpdateDesiredNodesResponse dryRunResponse = updateDesiredNodes(
randomDryRunUpdateDesiredNodesRequest(Version.CURRENT, Settings.EMPTY)
);
assertThat(dryRunResponse.dryRun(), is(equalTo(true)));

expectThrows(ResourceNotFoundException.class, this::getLatestDesiredNodes);
}

public void testDryRunUpdateDoesNotUpdateExistingDesiredNodes() {
UpdateDesiredNodesResponse response = updateDesiredNodes(randomUpdateDesiredNodesRequest(Version.CURRENT, Settings.EMPTY));
assertThat(response.dryRun(), is(equalTo(false)));

DesiredNodes desiredNodes = getLatestDesiredNodes();

UpdateDesiredNodesResponse dryRunResponse = updateDesiredNodes(
randomDryRunUpdateDesiredNodesRequest(Version.CURRENT, Settings.EMPTY)
);
assertThat(dryRunResponse.dryRun(), is(equalTo(true)));

assertEquals(getLatestDesiredNodes(), desiredNodes);
}

public void testSettingsAreValidatedWithDryRun() {
var exception = expectThrows(
IllegalArgumentException.class,
() -> updateDesiredNodes(
randomDryRunUpdateDesiredNodesRequest(
Version.CURRENT,
Settings.builder().put(SETTING_HTTP_TCP_KEEP_IDLE.getKey(), Integer.MIN_VALUE).build()
)
)
);
assertThat(exception.getMessage(), containsString("contain invalid settings"));
}

public void testUpdateDesiredNodesIsIdempotent() {
final var updateDesiredNodesRequest = randomUpdateDesiredNodesRequest();
updateDesiredNodes(updateDesiredNodesRequest);
Expand All @@ -71,7 +108,8 @@ public void testUpdateDesiredNodesIsIdempotent() {
final var equivalentUpdateRequest = new UpdateDesiredNodesRequest(
updateDesiredNodesRequest.getHistoryID(),
updateDesiredNodesRequest.getVersion(),
desiredNodesList
desiredNodesList,
false
);

updateDesiredNodes(equivalentUpdateRequest);
Expand All @@ -88,7 +126,8 @@ public void testGoingBackwardsWithinTheSameHistoryIsForbidden() {
final var backwardsUpdateDesiredNodesRequest = new UpdateDesiredNodesRequest(
updateDesiredNodesRequest.getHistoryID(),
updateDesiredNodesRequest.getVersion() - 1,
updateDesiredNodesRequest.getNodes()
updateDesiredNodesRequest.getNodes(),
false
);

final VersionConflictException exception = expectThrows(
Expand All @@ -105,7 +144,8 @@ public void testSameVersionWithDifferentContentIsForbidden() {
final var updateDesiredNodesRequestWithSameHistoryIdAndVersionAndDifferentSpecs = new UpdateDesiredNodesRequest(
updateDesiredNodesRequest.getHistoryID(),
updateDesiredNodesRequest.getVersion(),
randomList(1, 10, DesiredNodesTestCase::randomDesiredNode)
randomList(1, 10, DesiredNodesTestCase::randomDesiredNode),
false
);

final IllegalArgumentException exception = expectThrows(
Expand Down Expand Up @@ -228,7 +268,8 @@ public void testNodeProcessorsGetValidatedWithDesiredNodeProcessors() {
Settings.builder().put(NODE_PROCESSORS_SETTING.getKey(), numProcessors + 1).build(),
numProcessors
)
)
),
false
);

final IllegalArgumentException exception = expectThrows(
Expand Down Expand Up @@ -265,7 +306,8 @@ public void testNodeProcessorsGetValidatedWithDesiredNodeProcessors() {
Settings.builder().put(NODE_PROCESSORS_SETTING.getKey(), numProcessors).build(),
numProcessors
)
)
),
false
);

updateDesiredNodes(updateDesiredNodesRequest);
Expand Down Expand Up @@ -377,7 +419,17 @@ private UpdateDesiredNodesRequest randomUpdateDesiredNodesRequest(Version versio
return new UpdateDesiredNodesRequest(
UUIDs.randomBase64UUID(),
randomIntBetween(2, 20),
randomList(2, 10, () -> randomDesiredNode(version, settings))
randomList(2, 10, () -> randomDesiredNode(version, settings)),
false
);
}

private UpdateDesiredNodesRequest randomDryRunUpdateDesiredNodesRequest(Version version, Settings settings) {
return new UpdateDesiredNodesRequest(
UUIDs.randomBase64UUID(),
randomIntBetween(2, 20),
randomList(2, 10, () -> randomDesiredNode(version, settings)),
true
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ private UpdateDesiredNodesRequest randomUpdateDesiredNodesRequest() {
ByteSizeValue.ofGb(randomIntBetween(128, 256)),
Version.CURRENT
)
)
),
false
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public void testDesiredNodesStatusIsTracked() {
final var updateDesiredNodesRequest = new UpdateDesiredNodesRequest(
randomAlphaOfLength(10),
1,
concatLists(actualizedDesiredNodes, pendingDesiredNodes)
concatLists(actualizedDesiredNodes, pendingDesiredNodes),
false
);
updateDesiredNodes(updateDesiredNodesRequest);

Expand All @@ -49,7 +50,8 @@ public void testDesiredNodesStatusIsTracked() {
final var newVersionUpdateDesiredNodesRequest = new UpdateDesiredNodesRequest(
updateDesiredNodesRequest.getHistoryID(),
updateDesiredNodesRequest.getVersion() + 1,
updateDesiredNodesRequest.getNodes()
updateDesiredNodesRequest.getNodes(),
false
);
updateDesiredNodes(newVersionUpdateDesiredNodesRequest);

Expand All @@ -70,7 +72,8 @@ public void testIdempotentUpdateWithUpdatedStatus() {
final var updateDesiredNodesRequest = new UpdateDesiredNodesRequest(
randomAlphaOfLength(10),
1,
concatLists(actualizedDesiredNodes, pendingDesiredNodes)
concatLists(actualizedDesiredNodes, pendingDesiredNodes),
false
);
updateDesiredNodes(updateDesiredNodesRequest);

Expand Down Expand Up @@ -98,7 +101,8 @@ public void testActualizedDesiredNodesAreKeptAsActualizedEvenIfNodesLeavesTempor
final var updateDesiredNodesRequest = new UpdateDesiredNodesRequest(
randomAlphaOfLength(10),
1,
concatLists(actualizedDesiredNodes, pendingDesiredNodes)
concatLists(actualizedDesiredNodes, pendingDesiredNodes),
false
);
updateDesiredNodes(updateDesiredNodesRequest);

Expand Down Expand Up @@ -130,7 +134,8 @@ public void testStatusInformationIsClearedAfterHistoryIdChanges() throws Excepti
final var updateDesiredNodesRequest = new UpdateDesiredNodesRequest(
randomAlphaOfLength(10),
1,
concatLists(actualizedDesiredNodes, pendingDesiredNodes)
concatLists(actualizedDesiredNodes, pendingDesiredNodes),
false
);
updateDesiredNodes(updateDesiredNodesRequest);

Expand All @@ -146,7 +151,8 @@ public void testStatusInformationIsClearedAfterHistoryIdChanges() throws Excepti
final var updateDesiredNodesWithNewHistoryRequest = new UpdateDesiredNodesRequest(
randomAlphaOfLength(10),
1,
updateDesiredNodesRequest.getNodes()
updateDesiredNodesRequest.getNodes(),
false
);
final var response = updateDesiredNodes(updateDesiredNodesWithNewHistoryRequest);
assertThat(response.hasReplacedExistingHistoryId(), is(equalTo(true)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,18 +181,27 @@ public ClusterState execute(ClusterState currentState, List<TaskContext<UpdateDe
final var initialDesiredNodes = DesiredNodesMetadata.fromClusterState(currentState).getLatestDesiredNodes();
var desiredNodes = initialDesiredNodes;
for (final var taskContext : taskContexts) {

final UpdateDesiredNodesRequest request = taskContext.getTask().request();
if (request.isDryRun()) {
try {
updateDesiredNodes(desiredNodes, request);
taskContext.success(() -> taskContext.getTask().listener().onResponse(new UpdateDesiredNodesResponse(false, true)));
} catch (Exception e) {
taskContext.onFailure(e);
}
continue;
}
final var previousDesiredNodes = desiredNodes;
try {
desiredNodes = updateDesiredNodes(desiredNodes, taskContext.getTask().request());
desiredNodes = updateDesiredNodes(desiredNodes, request);
} catch (Exception e) {
taskContext.onFailure(e);
continue;
}
final var replacedExistingHistoryId = previousDesiredNodes != null
&& previousDesiredNodes.hasSameHistoryId(desiredNodes) == false;
taskContext.success(
() -> taskContext.getTask().listener().onResponse(new UpdateDesiredNodesResponse(replacedExistingHistoryId))
() -> taskContext.getTask().listener().onResponse(new UpdateDesiredNodesResponse(replacedExistingHistoryId, false))
);
}

Expand Down

0 comments on commit 72a6fdc

Please sign in to comment.