Use query param instead of a system property for opting in for new cluster health response code by arteam · Pull Request #79351 · elastic/elasticsearch

arteam · 2021-10-18T11:09:38Z

The original change was implemented in #78940, bu we have decided to move from a system property to an a request parameter, so Cloud users/clients have an easier way to opt-in for the new status code.

References #70849

…uster health response code

arteam · 2021-10-18T13:05:50Z

@elasticmachine update branch

elasticmachine · 2021-10-18T14:45:15Z

Pinging @elastic/clients-team (Team:Clients)

elasticmachine · 2021-10-18T14:45:32Z

Pinging @elastic/es-distributed (Team:Distributed)

DaveCTurner

Looks good, just a few small comments.

server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java

server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequest.java

…alth/ClusterHealthResponse.java Co-authored-by: David Turner <david.turner@elastic.co>

DaveCTurner

I think we should add the following YAML REST test too:

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.health/20_request_timeout.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.health/20_request_timeout.yml
index 66a7cb2b48d..6bd4f5fdb8e 100644
--- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.health/20_request_timeout.yml
+++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.health/20_request_timeout.yml
@@ -35,3 +35,23 @@
   - match:     { initializing_shards:     0 }
   - match:     { unassigned_shards:       0 }
   - gte:       { number_of_pending_tasks: 0 }
+
+
+---
+"cluster health request timeout with 200 response code":
+  - do:
+      cluster.health:
+        timeout: 1ms
+        wait_for_active_shards: 5
+        return_200_for_cluster_health_timeout: true
+
+  - is_true:   cluster_name
+  - is_true:   timed_out
+  - gte:       { number_of_nodes:         1 }
+  - gte:       { number_of_data_nodes:    1 }
+  - match:     { active_primary_shards:   0 }
+  - match:     { active_shards:           0 }
+  - match:     { relocating_shards:       0 }
+  - match:     { initializing_shards:     0 }
+  - match:     { unassigned_shards:       0 }
+  - gte:       { number_of_pending_tasks: 0 }

Other than that, LGTM.

...ck/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterHealthAction.java

…ml/utils/MlIndexAndAliasTests.java Co-authored-by: David Turner <david.turner@elastic.co>

…er/RestClusterHealthAction.java Co-authored-by: David Turner <david.turner@elastic.co>

…eout query parameter

arteam · 2021-10-18T16:50:23Z

Thanks a bunch, David!

…timeout")

elasticsearchmachine · 2021-10-18T20:45:17Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 79351

…new cluster health response code Backport elastic#79351 to 7.x: The original change was implemented in elastic#78940, but we have decided to move from a system property to a request parameter, so Cloud users/clients have an easier way to opt-in for the new status code. Relates elastic#70849

…new cluster health response code (#79397) Backport #79351 to 7.x: The original change was implemented in #78940, but we have decided to move from a system property to a request parameter, so Cloud users/clients have an easier way to opt-in for the new status code. Relates #70849

* upstream/master: (34 commits) Add extensionName() to security extension (elastic#79329) More robust and consistent allowAll indicesAccessControl (elastic#79415) Fix circuit breaker leak in MultiTerms aggregation (elastic#79362) guard geoline aggregation from parents aggegator that emit empty buckets (elastic#79129) Vector tiles: increase the size of the envelope used to clip geometries (elastic#79030) Revert "[ML] Add queue_capacity setting to start deployment API (elastic#79369)" (elastic#79374) Convert token service license object to LicensedFeature (elastic#79284) [TEST] Fix ShardPathTests for MDP (elastic#79393) Fix fleet search API with no checkpints (elastic#79400) Reduce BWC version for transient settings (elastic#79396) EQL: Rename a test class for eclipse (elastic#79254) Use search_coordination threadpool in field caps (elastic#79378) Use query param instead of a system property for opting in for new cluster health response code (elastic#79351) Add new kNN search endpoint (elastic#79013) Disable BWC tests Convert auditing license object to LicensedFeature (elastic#79280) Update BWC versions after backport of elastic#78551 Enable InstantiatingObjectParser to pass context as a first argument (elastic#79206) Move xcontent filtering tests (elastic#79298) Update links to Fleet/Agent docs (elastic#79303) ...

…new cluster health response code (elastic#79397) Backport elastic#79351 to 7.x: The original change was implemented in elastic#78940, but we have decided to move from a system property to a request parameter, so Cloud users/clients have an easier way to opt-in for the new status code. Relates elastic#70849

Documents the deprecation introduced in elastic#78180 and adjusted in elastic#79351.

Trivial follow-up to elastic#79351 that introduced some spots that create fresh empty string arrays.

…new cluster health response code (#79397) (#79435) Backport #79351 to 7.x: The original change was implemented in #78940, but we have decided to move from a system property to a request parameter, so Cloud users/clients have an easier way to opt-in for the new status code. Relates #70849

Trivial follow-up to #79351 that introduced some spots that create fresh empty string arrays.

Documents the deprecation introduced in #78180 and adjusted in #79351.

Trivial follow-up to #79351 that introduced some spots that create fresh empty string arrays.

…r new cluster health response code (elastic#79351)" This reverts commit 8901a99

#78968)" (#80826) * [8.0] Revert "Return 200 OK response code for a cluster health timeout (#78968)" This reverts commit a2c3dae * Revert "Allow deprecation warning for the return_200_for_cluster_health_timeout parameter (#80178) (#80444)" This reverts commit 4102cf7. * Revert "Drop pre-7.2.0 wire format in ClusterHealthRequest (#79551)" This reverts commit b9fbe66. * Revert "Adjust the BWC version for the return200ForClusterHealthTimeout field (#79436)" This reverts commit f60bda5. * Revert "Use query param instead of a system property for opting in for new cluster health response code (#79351)" This reverts commit 8901a99 * Revert "Deprecate returning 408 for a server timeout on `_cluster/health` (#78180)" This reverts commit f266eb3 * Drop pre-7.2.0 wire format in ClusterHealthRequest (#79551) This reverts commit fa4d562. * Revert "[8.0] Disable BWC for #80821 (#80840)"

)" (#80821) * Revert "Return 200 OK response code for a cluster health timeout (#78968)" This reverts commit a2c3dae * Revert "Allow deprecation warning for the return_200_for_cluster_health_timeout parameter (#80178)" This reverts commit 1c711e3. * Revert "Drop pre-7.2.0 wire format in ClusterHealthRequest (#79551)" This reverts commit b9fbe66. * Revert "Adjust the BWC version for the return200ForClusterHealthTimeout field (#79436)" This reverts commit f60bda5. * Revert "Use query param instead of a system property for opting in for new cluster health response code (#79351)" This reverts commit 8901a99 * Revert "Deprecate returning 408 for a server timeout on `_cluster/health` (#78180)" This reverts commit f266eb3 * Drop pre-7.2.0 wire format in ClusterHealthRequest (#79551) This reverts commit fa4d562 * Revert "Disable BWC for #80821 (#80839)" This reverts commit cb0e73e. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

elasticsearchmachine added the v8.0.0 label Oct 18, 2021

sethmlarson added the Team:Clients Meta label for clients team label Oct 18, 2021

Use query param instead of a system property for opting in for new cl…

7bfe441

…uster health response code

arteam force-pushed the query-param-instead-of-setting branch from e43fefc to 7bfe441 Compare October 18, 2021 12:09

elasticmachine and others added 2 commits October 18, 2021 09:05

Merge branch 'master' into query-param-instead-of-setting

661c387

Use version 8

450bfd4

arteam marked this pull request as ready for review October 18, 2021 14:45

arteam added the :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Oct 18, 2021

arteam requested a review from DaveCTurner October 18, 2021 14:45

elasticmachine added the Team:Distributed Meta label for distributed team. label Oct 18, 2021

DaveCTurner reviewed Oct 18, 2021

View reviewed changes

Update server/src/main/java/org/elasticsearch/action/admin/cluster/he…

829835f

…alth/ClusterHealthResponse.java Co-authored-by: David Turner <david.turner@elastic.co>

DaveCTurner added the >bug label Oct 18, 2021

arteam added 3 commits October 18, 2021 17:18

Throw an error if user requests 200 on an older cluster

7bc6f0c

Remove the constructor with a single return200ForServerTimeout

6b672f5

Add query parameter to the docs

95652e4

arteam force-pushed the query-param-instead-of-setting branch from 93e69ba to 95652e4 Compare October 18, 2021 15:53

arteam added 2 commits October 18, 2021 17:55

Don't send return200ForClusterHealthTimeout to older nodes

0180108

Don't follow JavaBeans conventions

6bc8637

DaveCTurner approved these changes Oct 18, 2021

View reviewed changes

...ck/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java Outdated Show resolved Hide resolved

DaveCTurner reviewed Oct 18, 2021

View reviewed changes

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterHealthAction.java Outdated Show resolved Hide resolved

arteam and others added 3 commits October 18, 2021 18:45

Update x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/…

e84e429

…ml/utils/MlIndexAndAliasTests.java Co-authored-by: David Turner <david.turner@elastic.co>

Update server/src/main/java/org/elasticsearch/rest/action/admin/clust…

be5fb14

…er/RestClusterHealthAction.java Co-authored-by: David Turner <david.turner@elastic.co>

Add an integration test for sending return_200_for_cluster_health_tim…

91e5fe7

…eout query parameter

arteam and others added 2 commits October 18, 2021 18:53

Set the correct query parameter name ("return_200_for_cluster_health_…

ef975e6

…timeout")

Merge branch 'master' into query-param-instead-of-setting

421e4e0

Update 20_request_timeout.yml test name with bwc and backport for 7.16

f61c452

arteam added auto-backport Automatically create backport pull requests when merged v7.16.0 labels Oct 18, 2021

arteam merged commit 8901a99 into elastic:master Oct 18, 2021

arteam deleted the query-param-instead-of-setting branch October 18, 2021 20:44

arteam mentioned this pull request Oct 18, 2021

[7.x] Use query param instead of a system property for opting in for new cluster health response code #79397

Merged

This was referenced Oct 19, 2021

[7.x] Use query param instead of a system property for opting in for new cluster health response code (#79397) #79435

Merged

Adjust the BWC version for the return200ForClusterHealthTimeout field #79436

Merged

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 19, 2021

Add deprecation docs for cluster health HTTP code

b123408

Documents the deprecation introduced in elastic#78180 and adjusted in elastic#79351.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 19, 2021

Use Strings#EMPTY_ARRAY

524a66b

Trivial follow-up to elastic#79351 that introduced some spots that create fresh empty string arrays.

This was referenced Oct 19, 2021

Use Strings#EMPTY_ARRAY #79452

Merged

Add deprecation docs for cluster health HTTP code #79451

Merged

arteam pushed a commit that referenced this pull request Oct 19, 2021

Use Strings#EMPTY_ARRAY (#79452)

0a72217

Trivial follow-up to #79351 that introduced some spots that create fresh empty string arrays.

DaveCTurner added a commit that referenced this pull request Oct 19, 2021

Add deprecation docs for cluster health HTTP code (#79451)

9184b4c

Documents the deprecation introduced in #78180 and adjusted in #79351.

DaveCTurner added a commit that referenced this pull request Oct 19, 2021

Use Strings#EMPTY_ARRAY (#79452)

4723419

Trivial follow-up to #79351 that introduced some spots that create fresh empty string arrays.

jakelandis added v8.0.0-beta1 and removed v8.0.0 labels Oct 27, 2021

arteam mentioned this pull request Nov 18, 2021

[8.0] Revert "Return 200 OK response code for a cluster health timeout (#78968)" #80826

Merged

arteam added a commit to arteam/elasticsearch that referenced this pull request Nov 18, 2021

Revert "Use query param instead of a system property for opting in fo…

a544e23

…r new cluster health response code (elastic#79351)" This reverts commit 8901a99

arteam mentioned this pull request Nov 18, 2021

Revert "Return 200 OK response code for a cluster health timeout (#78968)" #80821

Merged

arteam added a commit to arteam/elasticsearch that referenced this pull request Nov 18, 2021

Revert "Use query param instead of a system property for opting in fo…

f14bada

…r new cluster health response code (elastic#79351)" This reverts commit 8901a99

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use query param instead of a system property for opting in for new cluster health response code#79351

Use query param instead of a system property for opting in for new cluster health response code#79351
arteam merged 20 commits intoelastic:masterfrom
arteam:query-param-instead-of-setting

arteam commented Oct 18, 2021 •

edited

Loading

Uh oh!

arteam commented Oct 18, 2021

Uh oh!

elasticmachine commented Oct 18, 2021

Uh oh!

elasticmachine commented Oct 18, 2021

Uh oh!

DaveCTurner left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaveCTurner left a comment

Uh oh!

Uh oh!

Uh oh!

arteam commented Oct 18, 2021

Uh oh!

elasticsearchmachine commented Oct 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

arteam commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arteam commented Oct 18, 2021

Uh oh!

elasticmachine commented Oct 18, 2021

Uh oh!

elasticmachine commented Oct 18, 2021

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arteam commented Oct 18, 2021

Uh oh!

elasticsearchmachine commented Oct 18, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

arteam commented Oct 18, 2021 •

edited

Loading