Skip to content

Conversation

@arteam
Copy link
Contributor

@arteam arteam commented Oct 18, 2021

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

@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label Oct 18, 2021
@arteam arteam force-pushed the query-param-instead-of-setting branch from e43fefc to 7bfe441 Compare October 18, 2021 12:09
@arteam
Copy link
Contributor Author

arteam commented Oct 18, 2021

@elasticmachine update branch

@arteam arteam marked this pull request as ready for review October 18, 2021 14:45
@elasticmachine
Copy link
Collaborator

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

@arteam arteam added the :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Oct 18, 2021
@arteam arteam requested a review from DaveCTurner October 18, 2021 14:45
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Oct 18, 2021
@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few small comments.

…alth/ClusterHealthResponse.java

Co-authored-by: David Turner <[email protected]>
@arteam arteam force-pushed the query-param-instead-of-setting branch from 93e69ba to 95652e4 Compare October 18, 2021 15:53
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

arteam and others added 3 commits October 18, 2021 18:45
@arteam
Copy link
Contributor Author

arteam commented Oct 18, 2021

Thanks a bunch, David!

@arteam arteam added auto-backport Automatically create backport pull requests when merged v7.16.0 labels Oct 18, 2021
@arteam arteam merged commit 8901a99 into elastic:master Oct 18, 2021
@arteam arteam deleted the query-param-instead-of-setting branch October 18, 2021 20:44
@elasticsearchmachine
Copy link
Collaborator

💔 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

arteam added a commit to arteam/elasticsearch that referenced this pull request Oct 18, 2021
…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
arteam added a commit that referenced this pull request Oct 18, 2021
…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
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 19, 2021
* 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)
  ...
arteam added a commit to arteam/elasticsearch that referenced this pull request Oct 19, 2021
…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
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 19, 2021
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
Trivial follow-up to elastic#79351 that introduced some spots that create fresh
empty string arrays.
DaveCTurner pushed a commit that referenced this pull request Oct 19, 2021
…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
arteam pushed a commit that referenced this pull request Oct 19, 2021
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
Documents the deprecation introduced in #78180 and adjusted in #79351.
DaveCTurner added a commit that referenced this pull request Oct 19, 2021
Trivial follow-up to #79351 that introduced some spots that create fresh
empty string arrays.
arteam added a commit to arteam/elasticsearch that referenced this pull request Nov 18, 2021
…r new cluster health response code (elastic#79351)"

This reverts commit 8901a99
arteam added a commit to arteam/elasticsearch that referenced this pull request Nov 18, 2021
…r new cluster health response code (elastic#79351)"

This reverts commit 8901a99
arteam added a commit that referenced this pull request Nov 18, 2021
#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)"
arteam added a commit that referenced this pull request Nov 18, 2021
)" (#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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >bug :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. Team:Clients Meta label for clients team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants