-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add wait_for_completion parameter to resize, open, and forcemerge APIs (#6228) #6434
Merged
andrross
merged 12 commits into
opensearch-project:main
from
gaobinlong:add_wait_for_completion_parameter
Mar 27, 2023
Merged
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
79e67ef
Add wait_for_completion parameter to resize&open&forcemerge APIs (#6228)
gaobinlong 0ab3886
Modify changelog
gaobinlong 56fa2a1
fix test failure
gaobinlong 5bfd8e5
Fix test failure
gaobinlong eceb40f
Merge remote-tracking branch 'upstream/main' into add_wait_for_comple…
gaobinlong b8fcdd7
Merge remote-tracking branch 'upstream/main' into add_wait_for_comple…
gaobinlong e727d64
Merge remote-tracking branch 'upstream/main' into add_wait_for_comple…
gaobinlong db39bd4
change header of new file
gaobinlong c4c4add
Merge remote-tracking branch 'upstream/main' into add_wait_for_comple…
gaobinlong fbefce6
modify changelog
gaobinlong 286faf2
merge upstream main
gaobinlong c7eab0f
merge upstream main
gaobinlong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
rest-api-spec/src/main/resources/rest-api-spec/test/indices.clone/40_wait_for_completion.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
--- | ||
"Clone index with wait_for_completion": | ||
# clone index with wait_for_completion parameter, when the parameter is set to false, the API | ||
# will return a task immediately and the clone operation will run in background. | ||
|
||
- skip: | ||
version: " - 2.9.99" | ||
reason: "only available in 3.0+" | ||
features: allowed_warnings | ||
|
||
- do: | ||
nodes.info: | ||
node_id: data:true | ||
- set: | ||
nodes._arbitrary_key_: node_id | ||
|
||
- do: | ||
indices.create: | ||
index: source | ||
wait_for_active_shards: 1 | ||
body: | ||
settings: | ||
# ensure everything is allocated on the same data node | ||
index.routing.allocation.include._id: $node_id | ||
index.number_of_shards: 1 | ||
index.number_of_replicas: 0 | ||
- do: | ||
index: | ||
index: source | ||
id: "1" | ||
body: { "foo": "hello world" } | ||
|
||
- do: | ||
get: | ||
index: source | ||
id: "1" | ||
|
||
- match: { _index: source } | ||
- match: { _id: "1" } | ||
- match: { _source: { foo: "hello world" } } | ||
|
||
# make it read-only | ||
- do: | ||
indices.put_settings: | ||
index: source | ||
body: | ||
index.blocks.write: true | ||
index.number_of_replicas: 0 | ||
|
||
- do: | ||
cluster.health: | ||
wait_for_status: green | ||
index: source | ||
|
||
# clone with wait_for_completion | ||
- do: | ||
indices.clone: | ||
index: "source" | ||
target: "new_cloned_index" | ||
wait_for_active_shards: 1 | ||
cluster_manager_timeout: 10s | ||
wait_for_completion: false | ||
task_execution_timeout: 30s | ||
body: | ||
settings: | ||
index.number_of_shards: 1 | ||
"index.number_of_replicas": 0 | ||
|
||
- match: { task: /^.+$/ } | ||
- set: { task: taskId } | ||
|
||
- do: | ||
tasks.get: | ||
wait_for_completion: true | ||
task_id: $taskId | ||
- match: { task.action: "indices:admin/resize" } | ||
- match: { task.description: "clone from [source] to [new_cloned_index]" } | ||
|
||
# .tasks index is created when the clone index operation completes, so we should delete .tasks index finally, | ||
# if not, the .tasks index may introduce unexpected warnings and then cause other test cases to fail. | ||
# Delete the .tasks index directly will also introduce warning, but currently we don't have such APIs which can delete one | ||
# specified task or clear all completed tasks, so we have to do so. Expect we can introduce more tasks related APIs in future | ||
- do: | ||
allowed_warnings: | ||
- "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default" | ||
indices.delete: | ||
index: .tasks | ||
ignore_unavailable: true |
39 changes: 39 additions & 0 deletions
39
...-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/20_wait_for_completion.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
--- | ||
"Force merge index with wait_for_completion": | ||
# force merge index with wait_for_completion parameter, when the parameter is set to false, the API | ||
# will return a task immediately and the merge process will run in background. | ||
|
||
- skip: | ||
version: " - 2.9.99" | ||
reason: "only available in 3.0+" | ||
features: allowed_warnings | ||
|
||
- do: | ||
indices.create: | ||
index: test_index | ||
|
||
- do: | ||
indices.forcemerge: | ||
index: test_index | ||
wait_for_completion: false | ||
max_num_segments: 1 | ||
- match: { task: /^.+$/ } | ||
- set: { task: taskId } | ||
|
||
- do: | ||
tasks.get: | ||
wait_for_completion: true | ||
task_id: $taskId | ||
- match: { task.action: "indices:admin/forcemerge" } | ||
- match: { task.description: "Force-merge indices [test_index], maxSegments[1], onlyExpungeDeletes[false], flush[true]" } | ||
|
||
# .tasks index is created when the force-merge operation completes, so we should delete .tasks index finally, | ||
# if not, the .tasks index may introduce unexpected warnings and then cause other test cases to fail. | ||
# Delete the .tasks index directly will also introduce warning, but currently we don't have such APIs which can delete one | ||
# specified task or clear all completed tasks, so we have to do so. Expect we can introduce more tasks related APIs in future | ||
- do: | ||
allowed_warnings: | ||
- "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default" | ||
indices.delete: | ||
index: .tasks | ||
ignore_unavailable: true |
50 changes: 50 additions & 0 deletions
50
rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/30_wait_for_completion.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
--- | ||
"Open index with wait_for_completion": | ||
# open index with wait_for_completion parameter, when the parameter is set to false, the API | ||
# will return a task immediately and the open operation will run in background. | ||
|
||
- skip: | ||
version: " - 2.9.99" | ||
reason: "only available in 3.0+" | ||
features: allowed_warnings | ||
|
||
- do: | ||
indices.create: | ||
index: test_index | ||
body: | ||
settings: | ||
number_of_replicas: 0 | ||
number_of_shards: 1 | ||
|
||
- do: | ||
indices.close: | ||
index: test_index | ||
- is_true: acknowledged | ||
|
||
- do: | ||
indices.open: | ||
index: test_index | ||
wait_for_active_shards: all | ||
cluster_manager_timeout: 10s | ||
wait_for_completion: false | ||
task_execution_timeout: 30s | ||
- match: { task: /^.+$/ } | ||
- set: { task: taskId } | ||
|
||
- do: | ||
tasks.get: | ||
wait_for_completion: true | ||
task_id: $taskId | ||
- match: { task.action: "indices:admin/open" } | ||
- match: { task.description: "open indices [test_index]" } | ||
|
||
# .tasks index is created when the open index operation completes, so we should delete .tasks index finally, | ||
# if not, the .tasks index may introduce unexpected warnings and then cause other test cases to fail. | ||
# Delete the .tasks index directly will also introduce warning, but currently we don't have such APIs which can delete one | ||
# specified task or clear all completed tasks, so we have to do so. Expect we can introduce more tasks related APIs in future | ||
- do: | ||
allowed_warnings: | ||
- "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default" | ||
indices.delete: | ||
index: .tasks | ||
ignore_unavailable: true |
88 changes: 88 additions & 0 deletions
88
...-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/50_wait_for_completion.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
--- | ||
"Shrink index with wait_for_completion": | ||
# shrink index with wait_for_completion parameter, when the parameter is set to false, the API | ||
# will return a task immediately and the shrink operation will run in background. | ||
|
||
- skip: | ||
version: " - 2.9.99" | ||
reason: "only available in 3.0+" | ||
features: allowed_warnings | ||
|
||
- do: | ||
nodes.info: | ||
node_id: data:true | ||
- set: | ||
nodes._arbitrary_key_: node_id | ||
|
||
- do: | ||
indices.create: | ||
index: source | ||
wait_for_active_shards: 1 | ||
body: | ||
settings: | ||
# ensure everything is allocated on the same data node | ||
index.routing.allocation.include._id: $node_id | ||
index.number_of_shards: 3 | ||
index.number_of_replicas: 0 | ||
- do: | ||
index: | ||
index: source | ||
id: "1" | ||
body: { "foo": "hello world" } | ||
|
||
- do: | ||
get: | ||
index: source | ||
id: "1" | ||
|
||
- match: { _index: source } | ||
- match: { _id: "1" } | ||
- match: { _source: { foo: "hello world" } } | ||
|
||
# make it read-only | ||
- do: | ||
indices.put_settings: | ||
index: source | ||
body: | ||
index.blocks.write: true | ||
index.number_of_replicas: 0 | ||
|
||
- do: | ||
cluster.health: | ||
wait_for_status: green | ||
index: source | ||
|
||
# shrink with wait_for_completion | ||
- do: | ||
indices.shrink: | ||
index: "source" | ||
target: "new_shrunken_index" | ||
wait_for_active_shards: 1 | ||
cluster_manager_timeout: 10s | ||
wait_for_completion: false | ||
task_execution_timeout: 30s | ||
body: | ||
settings: | ||
index.number_of_shards: 1 | ||
"index.number_of_replicas": 0 | ||
|
||
- match: { task: /^.+$/ } | ||
- set: { task: taskId } | ||
|
||
- do: | ||
tasks.get: | ||
wait_for_completion: true | ||
task_id: $taskId | ||
- match: { task.action: "indices:admin/resize" } | ||
- match: { task.description: "shrink from [source] to [new_shrunken_index]" } | ||
|
||
# .tasks index is created when the shrink index operation completes, so we should delete .tasks index finally, | ||
# if not, the .tasks index may introduce unexpected warnings and then cause other test cases to fail. | ||
# Delete the .tasks index directly will also introduce warning, but currently we don't have such APIs which can delete one | ||
# specified task or clear all completed tasks, so we have to do so. Expect we can introduce more tasks related APIs in future | ||
- do: | ||
allowed_warnings: | ||
- "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default" | ||
indices.delete: | ||
index: .tasks | ||
ignore_unavailable: true |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the first time we've encountered this problem in the rest-api-spec tests? Are there no tests here for the other APIs that currently use the .tasks index?
I understand why you're doing this but I don't love having to manually clean up a side effect like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the first time we encounter this problem, currently the
.tasks
index is only used by reindex/update_by_query/delete_by_query, but their code and rest-api-spec tests are in an independent modulereindex
, so they will not affect the test cases inside the core rest-api-spec. The failed yaml tests above contain operations which refresh all index or search all index like 160_extended_stats_metric.yml or 13_fields.yml, so they will also refresh or search the.tasks
index and then fail because of thedirect access to system indices...
warning. I found that there are so many yaml tests which refresh all index or search all index but don't containallowed_warnings
, so maybe it's tricky to modify all of them.