-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
REST high-level client: add Cluster Health API #29331
REST high-level client: add Cluster Health API #29331
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
Pinging @elastic/es-core-infra |
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.
hi @Van0SS thanks for your PR. I did a quick first round just to give you some early feedback, I will do another in-depth review once you have addressed these first comments and you are satisfied with the overall quality of your PR (removed TODOs etc.).
NOTICE.txt
Outdated
@@ -1,5 +1,5 @@ | |||
Elasticsearch | |||
Copyright 2009-2017 Elasticsearch | |||
Copyright 2009-2018 Elasticsearch |
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.
please could you revert this? This change should not be made as part of this PR, regardless of whether it's needed or not.
assertThat(response.getDelayedUnassignedShards(), is(0)); | ||
assertThat(response.getInitializingShards(), is(0)); | ||
assertThat(response.getUnassignedShards(), is(0)); | ||
assertThat(response.getActiveShardsPercent(), is(100d)); |
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.
we should probably add a test that provides indices and the level parameter
expectedParams.put("level", "shards"); | ||
// Default value in ClusterHealthRequest is NONE but in Request.Params::withWaitForActiveShards is DEFAULT | ||
expectedParams.put("wait_for_active_shards", "0"); | ||
//TODO add random filling for other properties |
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.
++
setRandomTimeout(healthRequest::timeout, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams); | ||
setRandomMasterTimeout(healthRequest, expectedParams); | ||
expectedParams.put("level", "shards"); | ||
// Default value in ClusterHealthRequest is NONE but in Request.Params::withWaitForActiveShards is DEFAULT |
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 see, you may have to make changes here like we already did in the 6.x branch where we have a different default for some API.
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.
Can you please clarify it? If I set in ClusterHealthRequest field waitForActiveShards = ActiveShardCount.DEFAULT
then it will fail in TransportClusterHealthAction::prepareResponse
assert waitForActiveShards.equals(ActiveShardCount.DEFAULT) == false :
"waitForActiveShards must not be DEFAULT on the request object, instead it should be NONE";
Probably it can instead of failing set it to 0.
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 mean to adapt the test, doing something along these lines (from the 6.x branch): https://github.com/elastic/elasticsearch/blob/6.x/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java#L1649 .
return (int) ((float) expectedSize / 0.75F + 1.0F); | ||
} | ||
return Integer.MAX_VALUE; // any large value | ||
} |
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 would refrain from adding these to be honest, especially if only used in our client/parsing code.
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.
++
For the most part we're ok with new HashMap<>(expectedSize);
@Van0SS also, answers to your questions:
Yes we do. If we don't allow the high-level client to provide this parameter, it will only ever be possible to retrieve cluster level view (default) using the high-level REST client. We need to add it as a field to
We probably forgot, could you also add that parameter to the SPEC? You can do it either as part of this PR or as a separate PR that could go in first if you prefer. |
numberOfInFlightFetch == that.numberOfInFlightFetch && | ||
delayedUnassignedShards == that.delayedUnassignedShards && | ||
timedOut == that.timedOut && | ||
Objects.equals(clusterName, that.clusterName) && |
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.
Can you put these in the same order as hashCode
?
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
ClusterIndexHealth that = (ClusterIndexHealth) o; | ||
return numberOfShards == that.numberOfShards && |
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.
Can you put these in the same order as hashcode?
}); | ||
|
||
public static final ObjectParser.NamedObjectParser<ClusterShardHealth, Void> SHARD_PARSER = | ||
(XContentParser p, Void c, String nameIgnored) -> ClusterShardHealth.fromXContent(p); |
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.
Thanks for calling it nameIgnored
.
* @param coll the collection to check, may be null | ||
* @return true if empty or null | ||
*/ | ||
public static boolean isEmpty(final Collection<?> coll) { |
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'd prefer not do have this and to do the null check at the call site to honest.
return (int) ((float) expectedSize / 0.75F + 1.0F); | ||
} | ||
return Integer.MAX_VALUE; // any large value | ||
} |
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.
++
For the most part we're ok with new HashMap<>(expectedSize);
params.withTimeout(healthRequest.timeout()); | ||
params.withMasterTimeout(healthRequest.masterNodeTimeout()); | ||
params.withLocal(healthRequest.local()); | ||
params.putParam("level", "shards"); |
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 see. Right now you request the whole thing because it is simpler that way. I think this is fine. I think if we decide we'd like to support level
as something else then we can do it in a followup. What do you think, @javanna?
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 had missed this line, so my comment on this is a bit off, sorry about that :) this is actually perfectly backwards compatible with the transport client behaviour, as it gets exactly everything. I think that we could add the level parameter though so that we can properly filter what gets returned at REST like every client works.
Thank you guys for the review!
I agree that better do it on the transport level.
I also have a question about integration tests - do we run tests only on 1 node cluster, so I can check that numberOfNodes == 1 or it not always the case? |
@javanna @nik9000 |
@Van0SS
I think so, yes. Do try running tests though and see what happens ;)
True, it's probably undocumented for a reason, the important bit is that it's in the SPEC and properly parsed in the REST action. We can see whether it needs to be documented in the Elasticsearch docs later, as a followup. Thank you!! |
@javanna Fixes are here! |
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.
thanks a lot for your PR @Van0SS it looks pretty good already. I left quite some comments but it's mostly minor stuff. I have just tried to build the docs but I got some errors, could you please check that out?
You need to check out the https://github.com/elastic/docs project, and use the build_docs.pl
script to build the docs. I use it with the following arguments (I run it from the root of the elasticsearch project): --doc docs/java-rest/index.asciidoc --chunk 1 --out ~/temp/asciidoc --open
@@ -643,6 +663,10 @@ static String endpoint(String[] indices, String endpoint, String[] suffixes) { | |||
.addCommaSeparatedPathParts(suffixes).build(); | |||
} | |||
|
|||
static String endpoint(String endpoint, String[] suffixes) { |
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 am not loving that we have a similar endpoint method that takes Strings[] indices, String endpoint
as arguments. Can we use directly EndpointBuilder
here like we do in a few other places rather than adding this new method?
String level = randomFrom("default-shards", "cluster", "indices", "shards"); | ||
if (!"default-shards".equals(level)) { | ||
request.level(level); | ||
} |
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 would prefer that we don't want randomize anything in this integration test. These are quite hard to debug if they fail, I would prefer to have different test methods here for the main code paths.
ClusterHealthRequest healthRequest = new ClusterHealthRequest(); | ||
Map<String, String> expectedParams = new HashMap<>(); | ||
setRandomLocal(healthRequest, expectedParams); | ||
if (randomBoolean()) { |
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.
do we also test the case where we set masterNodeTimeout
but not timeout
?
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.
Maybe a switch which covers all the 4 permutations (none set, both set, only timeout set, only masterNodeTimeout set) would be easier to read?
expectedParams.put("timeout", "30s"); | ||
expectedParams.put("master_timeout", "30s"); | ||
} | ||
setRandomWaitForActiveShards(healthRequest::waitForActiveShards, expectedParams); |
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.
should we add the default value as argument here instead?
expectedParams.put("wait_for_no_relocating_shards", Boolean.TRUE.toString()); | ||
} | ||
} | ||
String[] indices = randomIndicesNames(0, 5); |
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.
like we do in other places, could you also the case where indices is assigned to null? e.g. String[] indices = randomBoolean() ? null : randomIndicesNames(0, 5);
|
||
protected ToXContent.Params getToXContentParams() { | ||
Map<String, String> map = new HashMap<>(); | ||
map.put("level", "shards"); |
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.
Collections.singletonMap("level", "shards")
?
return ClusterIndexHealth.fromXContent(parser); | ||
} | ||
|
||
protected ToXContent.Params getToXContentParams() { |
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.
add @Override
here too?
Map<String, String> map = new HashMap<>(); | ||
map.put("level", "shards"); | ||
return new ToXContent.MapParams(map); | ||
} |
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 that we should add some test of the output with different level (cluster or indices). I can imagine that we can only use shards in the standard tests otherwise all the comparisons between what gets set and what gets parsed/serialized back will fail. Or maybe we can decide this upfront in a randomized fashion and not even generate indices and shards values according to what we are going to output/test ? What do you think?
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.
Agree, will try to implement the upfront decision.
@@ -65,7 +62,8 @@ | |||
T parsed = parseFunction.apply(parser); | |||
assertEqualsConsumer.accept(testInstance, parsed); | |||
if (assertToXContentEquivalence) { | |||
assertToXContentEquivalent(shuffled, XContentHelper.toXContent(parsed, xContentType, false), xContentType); | |||
assertToXContentEquivalent(shuffled, XContentHelper.toXContent(parsed, xContentType, toXContentParams,false), |
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.
missing space in toXContentParams,false
between the two arguments, here and anywhere else.
@@ -127,4 +125,8 @@ protected boolean assertToXContentEquivalence() { | |||
protected String[] getShuffleFieldsExceptions() { | |||
return Strings.EMPTY_ARRAY; | |||
} | |||
|
|||
protected ToXContent.Params getToXContentParams() { |
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.
missing override annotation.
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.
Sorry, I miserably failed with it... Added inspection in IDE.
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.
no worries, small thing.
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.
and this one too?
…_client_cluster_health
@javanna Round 3 changes were committed:
|
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.
hi @Van0SS sorry for keeping you on hold. I did another round of review, left some nits, but also noticed that due to some collateral changes that I've asked you to make, the PR has become quite big and harder to review. Would it be possible to move the changes to the constructors of the response objects etc. to another PR please? Hopefully it's not a lot of work but I would really like to review those two changes separately. Let me know what you think!
emptyClusterAssertion(response); | ||
} | ||
|
||
public static void emptyClusterAssertion(ClusterHealthResponse response) { |
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.
maybe call it assertNoIndices
? that is the empty part right?
} | ||
} | ||
|
||
private void shardAssertion(int shardId, ClusterShardHealth shardHealth) { |
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.
maybe call it assertYellowShard
?
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.
also, all these assert methods could be static?
indexAssertion(index.getKey(), index.getValue(), false); | ||
} | ||
|
||
private void indexAssertion(String indexName, ClusterIndexHealth indexHealth, boolean emptyShards) { |
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.
maybe call it assertYellowIndex
?
} | ||
} | ||
|
||
private void yellowTenShardsClusterAssertion(ClusterHealthResponse response) { |
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.
call it assertTenYellowShards
?
break; | ||
case "masterTimeout": | ||
expectedParams.put("timeout", "30s"); | ||
healthRequest.timeout(masterTimeout); |
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.
shouldn't you set masterTimeout here rather than timeout ?
A time based parameter controlling how long to wait if the master has not been | ||
discovered yet or disconnected. | ||
If not provided, uses the same value as `timeout`. | ||
|
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.
while at it, let's also document wait_for_events
?
", timedOut=" + timedOut + | ||
", clusterStateHealth=" + clusterStateHealth + | ||
", clusterHealthStatus=" + clusterHealthStatus + | ||
'}'; |
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.
do we need to change this? not a huge deal, but given that we don't have clear guidelines on whether we want to print out json or not, I don't know if it's worth changing toString impl from one way to another.
import org.apache.lucene.util.BytesRefBuilder; | ||
import org.apache.lucene.util.InPlaceMergeSorter; | ||
import org.apache.lucene.util.IntroSorter; | ||
|
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.
can you revert these? :)
clusterHealth = ClusterHealthResponse.readResponseFrom(in); | ||
} catch (IOException e) { | ||
throw new IllegalStateException(e); | ||
} |
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 change necessary? I tend to think that we should revert it, this PR is already getting quite big and I would like to reduce the noise introduced by unnecessary changes.
@@ -127,4 +125,8 @@ protected boolean assertToXContentEquivalence() { | |||
protected String[] getShuffleFieldsExceptions() { | |||
return Strings.EMPTY_ARRAY; | |||
} | |||
|
|||
protected ToXContent.Params getToXContentParams() { |
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.
and this one too?
heya @Van0SS I haven't heard back after my last review, would you have a chance to address the comments? Otherwise please let me know what I can do to move this forward. Thanks! |
…o enhancement/rest_hl_client_cluster_health
@javanna Hey, sorry, being busy, hope will update PR soon. |
no worries @Van0SS thank you! let me know if I can help. |
retest this please |
add to whitelist |
retest this please |
-------------------------------------------------- | ||
include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[health-request-local] | ||
-------------------------------------------------- | ||
<1> Non-master node can be used for this request. Defaults to `false` |
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.
Do you want to do wait_for_events
here?
@@ -55,16 +151,28 @@ public ClusterHealthResponse(String clusterName, String[] concreteIndices, Clust | |||
} | |||
|
|||
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState, int numberOfPendingTasks, | |||
int numberOfInFlightFetch, int delayedUnassignedShards, TimeValue taskMaxWaitingTime) { | |||
int numberOfInFlightFetch, int delayedUnassignedShards, TimeValue taskMaxWaitingTime) { |
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.
Can you add the indent back? It is much easier to read if the parameters don't line up with the code.
* For XContent Parser and serialization tests | ||
*/ | ||
ClusterHealthResponse(String clusterName, int numberOfPendingTasks, int numberOfInFlightFetch, int delayedUnassignedShards, | ||
TimeValue taskMaxWaitingTime, boolean timedOut, ClusterStateHealth clusterStateHealth) { |
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.
Same here.
thanks a lot @Van0SS ! |
The default wait_for_active_shards is NONE for cluster health, which differs from all the other API in master, hence we need to make sure to set the parameter whenever it differs from NONE (0). The test around this also had a bug, which is why this was not originally uncovered. Relates to elastic#29331
With elastic#29331 we added support for the cluster health API to the high-level REST client. The transport client does not support the level parameter, and it always returns all the info needed for shards level rendering. We have maintained that behaviour when adding support for cluster health to the high-level REST client, to ease migration, but the correct thing to do is to default the high-level REST client to `cluster` level, which is the same default as when going through the Elasticsearch REST layer.
…#31266) The default wait_for_active_shards is NONE for cluster health, which differs from all the other API in master, hence we need to make sure to set the parameter whenever it differs from NONE (0). The test around this also had a bug, which is why this was not originally uncovered. Relates to #29331
Thank you @javanna for finishing it! |
With #29331 we added support for the cluster health API to the high-level REST client. The transport client does not support the level parameter, and it always returns all the info needed for shards level rendering. We have maintained that behaviour when adding support for cluster health to the high-level REST client, to ease migration, but the correct thing to do is to default the high-level REST client to `cluster` level, which is the same default as when going through the Elasticsearch REST layer.
* master: Remove RestGetAllAliasesAction (#31308) Temporary fix for broken build Reenable Checkstyle's unused import rule (#31270) Remove remaining unused imports before merging #31270 Fix non-REST doc snippet [DOC] Extend SQL docs Immediately flush channel after writing to buffer (#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) CCS: don't proxy requests for already connected node (#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (#31298) [Rollup] Metric config parser must use builder so validation runs (#31159) [ML] Check licence when datafeeds use cross cluster search (#31247) Add notion of internal index settings (#31286) Test: Remove broken yml test feature (#31255) REST hl client: cluster health to default to cluster level (#31268) [ML] Update test thresholds to account for changes to memory control (#31289) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) Ignore numeric shard count if waiting for ALL (#31265) [ML] Implement new rules design (#31110) index_prefixes back-compat should test 6.3 (#30951) Core: Remove plain execute method on TransportAction (#30998) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Modify pipelining handlers to require full requests (#31280) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) Fix Netty 4 Server Transport tests. Again. REST hl client: adjust wait_for_active_shards param in cluster health (#31266) REST high-level Client: remove deprecated API methods (#31200) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) Delete typos in SAML docs (#31199) REST high-level client: add Cluster Health API (#29331) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) Remove some line length supressions (#31209) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) LLClient: Support host selection (#30523) Upgrade to Netty 4.1.25.Final (#31232) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) HLRest: Add get index templates API (#31161) Remove all unused imports and fix CRLF (#31207) [Tests] Fix self-referencing tests [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Limit the number of concurrent requests per node (#31206) Call ensureNoSelfReferences() on _agg state variable after scripted metric agg script executions (#31044) Move java version checker back to its own jar (#30708) [test] add fix for rare virtualbox error (#31212)
* 6.x: SQL: Fix build on Java 10 [Tests] Mutualize fixtures code in BaseHttpFixture (#31210) [TEST] Fix RemoteClusterClientTests#testEnsureWeReconnect [ML] Update test thresholds to account for changes to memory control (#31289) Reenable Checkstyle's unused import rule (#31270) [ML] Check licence when datafeeds use cross cluster search (#31247) Fix non-REST doc snippet [DOC] Extend SQL docs [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) SQL: Whitelist SQL utility class for better scripting (#30681) Add 5.6.11 version constant. Fix version detection. [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) Add missing release notes. Security: fix token bwc with pre 6.0.0-beta2 (#31254) Fix compilation error in UpdateSettingsIT (#31304) Test: Remove broken yml test feature (#31255) Add unreleased version 6.3.1 [Rollup] Metric config parser must use builder so validation runs (#31159) Removes experimental tag from scripted_metric aggregation (#31298) [DOCS] Removes coming tag from 6.3.0 release notes 6.3 release notes. Add notion of internal index settings (#31286) REST high-level client: add Cluster Health API (#29331) Remove leftover usage of deprecated client API SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) HLRest: Add get index templates API (#31161) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) [TEST] Mute RecoveryIT.testHistoryUUIDIsGenerated Ignore numeric shard count if waiting for ALL (#31265) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160) Fix Netty 4 Server Transport tests. Again. [DOCS] Fixed typo. [DOCS] Added release highlights for 6.3 (#31256) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Upgrade to Netty 4.1.25.Final (#31232) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) [DOCS] Adds machine learning 6.3.0 release notes (#31217) Remove all unused imports and fix CRLF (#31207) [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Compliant SAML Response destination check (#31175) Move java version checker back to its own jar (#30708) TEST: Retry synced-flush if ongoing ops on primary (#30978) [test] add fix for rare virtualbox error (#31212)
Relates to #27205
It's not a final PR, there are things to discuss and tasks to finish. Want to be sure that I am on a right track.
Questions:
There is formatting output parameter 'level'. Currently, it is used only in REST API. Do we need it in High level client? UPD: added
Found out that API parameter 'master_timeout' isn't documented but present in ClusterHealthRequest and parsed in REST Controller. Is it as it should be? UPD: added
For discussion in this PR:
Suggestions to implement:
clientCONTROLLER level, not like now - in toXContent UPD: better do it on transport level but not nowThanks in advance for reviewing, I understand that there are some changes in the existing code.