REST high-level client: add clear cache API by javanna · Pull Request #28866 · elastic/elasticsearch

javanna · 2018-03-01T11:13:34Z

Relates to #27205

olcbean

LGTM
I just left a few minor nits.

olcbean · 2018-03-02T10:32:17Z

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java

+     * Clears the cache of one or more indices using the Clear Cache API
+     * <p>
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-clearcache.html">
+     *     Clear Cache API on elastic.co</a>


nit : do we need this \t here? ( we did not use it in the rest of the javadocs )

we need it here otherwise the line is too long

I think @olcbean talks about the indent of the new line here (line 268), to remove it and align with other lines :) like in other comments.

oh right the tab :) that we don't need

olcbean · 2018-03-02T10:41:34Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

    }

+    static Request clearCache(ClearIndicesCacheRequest clearIndicesCacheRequest) {
+        String endpoint = endpoint(clearIndicesCacheRequest.indices(), "_cache", "clear");


Could this result a NPE if there are no indices specified ?

great catch, this is true for every broadcast response

I fixed this

olcbean · 2018-03-02T10:46:12Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

+        Params parameters = Params.builder();
+        parameters.withIndicesOptions(clearIndicesCacheRequest.indicesOptions());
+        parameters.putParam("query", Boolean.toString(clearIndicesCacheRequest.queryCache()));
+        parameters.putParam("fielddata", Boolean.toString(clearIndicesCacheRequest.fieldDataCache()));


nit : isn't field_data the preferred name ?

Can you update the \rest-api-spec\src\main\resources\rest-api-spec\api\indices.clear_cache.json as well? ( do we need to specify all supported params, or only the preferred ones. There is also a recycler flag in the rest specs which I do not see in the code )

I think the fielddata thing is a mistake in the way we use ParseField to read parameters. Looking at the history, they are just synonyms, we never deprecated one in favour of the other. We use fielddata in our docs though. You are welcome to send a PR to remove support for the other deprecated params against master though ;)

Sorry, I only went with the code ( without going into details ).

However, I just called GET _cache/clear?fielddata in Kibana and got

#! Deprecation: Deprecated field [fielddata] used, expected [field_data] instead { "_shards": { "total": 2, "successful": 1, "failed": 0 } }

The deprecation seems to be wrapped in the ParseField#match

We use fielddata in our docs though.

Good catch! Maybe it would be better to have a separate PR for the ref docs ( so that we can track when the preferred name has been changed and since when a deprecation msg is logged ) ( I can take this one over if you want )

You are welcome to send a PR to remove support for the other deprecated params against master though ;)

Which other params do you mean? And remove them from the the rest api specs and the ref docs ?

Which other params do you mean? And remove them from the the rest api specs and the ref docs ?

I meant filter, filter_cache, and request_cache. I think we can remove them now in master.

fielddata is the preferred name as of my merging #28943 today.

olcbean · 2018-03-02T10:59:44Z

...level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java

+            int failedShards = clearCacheResponse.getFailedShards(); // <3>
+            DefaultShardOperationFailedException[] failures = clearCacheResponse.getShardFailures(); // <4>
+            // end::clear-cache-response
+


maybe check the returned values ?

olcbean · 2018-03-02T11:05:42Z

docs/java-rest/high-level/indices/clear_cache.asciidoc

+--------------------------------------------------
+include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[clear-cache-request-fielddata]
+--------------------------------------------------
+<1> Set the `fielddata` flag to `true`


nit : s/fielddata/field_data/

fielddata is correct now!

olcbean · 2018-03-08T17:39:14Z

@javanna I just opened #28943 to deprecate "back" field_data in favor of fielddata ( then this PR does not need to change the fielddata )

olcbean · 2018-03-09T10:35:03Z

rest-api-spec/src/main/resources/rest-api-spec/api/indices.clear_cache.json

-        "recycler": {
-          "type" : "boolean",
-          "description" : "Clear the recycler cache"
-        },


Relates to #27158

tlrx

LGTM, sorry for the delay it took me to review this.

nik9000 · 2018-03-13T20:25:26Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

+        Params parameters = Params.builder();
+        parameters.withIndicesOptions(clearIndicesCacheRequest.indicesOptions());
+        parameters.putParam("query", Boolean.toString(clearIndicesCacheRequest.queryCache()));
+        parameters.putParam("fielddata", Boolean.toString(clearIndicesCacheRequest.fieldDataCache()));


fielddata is the preferred name as of my merging #28943 today.

nik9000 · 2018-03-13T20:26:07Z

docs/java-rest/high-level/indices/clear_cache.asciidoc

+--------------------------------------------------
+include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[clear-cache-request-fielddata]
+--------------------------------------------------
+<1> Set the `fielddata` flag to `true`


fielddata is correct now!

…n endpoints

Relates to #27205 Also Closes #26947 (rest-spec were outdated)

* es/master: (50 commits) Reject updates to the `_default_` mapping. (#29165) Improve similarity docs. (#29089) [Docs] Update api.asciidoc (#29166) Docs: Add note about missing mapping for doc values field (#29036) Fix BWC issue for PreSyncedFlushResponse Remove BytesArray and BytesReference usage from XContentFactory (#29151) Add pluggable XContentBuilder writers and human readable writers (#29120) Add unreleased version 6.2.4 (#29171) Add unreleased version 6.1.5 (#29168) Add a note about using the `retry_failed` flag before accepting data loss (#29160) Fix typo in percolate-query.asciidoc (#29155) Require HTTP::Tiny 0.070 for release notes script Set Java 9 checkstyle to depend on checkstyle conf (#28383) REST high-level client: add clear cache API (#28866) Docs: Add example of resetting index setting (#29048) Plugins: Fix module name conflict check for meta plugins (#29146) Build: Fix meta plugin bundled plugin names (#29147) Build: Simplify rest spec hack configuration (#29149) Build: Fix meta modules to not install as plugin in tests (#29150) Fix javadoc warning in Strings for missing parameter description ...

* es/6.x: (46 commits) Docs: Add note about missing mapping for doc values field (#29036) [DOCS] Removed 6.1.4, 6.2.2, and 6.2.3 coming tags Remove BytesArray and BytesReference usage from XContentFactory (#29151) Fix BWC issue for PreSyncedFlushResponse Add pluggable XContentBuilder writers and human readable writers (#29120) Add unreleased version 6.2.4 (#29171) Add unreleased version 6.1.5 (#29168) Add a note about using the `retry_failed` flag before accepting data loss (#29160) Fix typo in percolate-query.asciidoc (#29155) Add release notes for 6.1.4 and 6.2.3 Require HTTP::Tiny 0.070 for release notes script REST high-level client: add clear cache API (#28866) Relax remote check for bwc project checkouts (#28666) Set Java 9 checkstyle to depend on checkstyle conf (#28383) Docs: Add example of resetting index setting (#29048) Plugins: Fix module name conflict check for meta plugins (#29146) Build: Fix meta plugin bundled plugin names (#29147) Build: Simplify rest spec hack configuration (#29149) CLI: Close subcommands in MultiCommand (#28954) Build: Fix meta modules to not install as plugin in tests (#29150) ...

REST high-level client: add clear cache API

271da22

Relates to elastic#27205

javanna added >enhancement v7.0.0 :Core/Java High Level REST Client v6.3.0 labels Mar 1, 2018

javanna requested a review from tlrx March 1, 2018 11:13

javanna mentioned this pull request Mar 1, 2018

Java high-level REST client completeness #27205

Closed

80 tasks

olcbean approved these changes Mar 2, 2018

View reviewed changes

javanna added 3 commits March 2, 2018 17:15

Merge branch 'master' into enhancement/rest_hl_client_clear_cache

c0dd793

Merge branch 'master' into enhancement/rest_hl_client_clear_cache

22cb53e

address some of the review comments

cddc2e0

olcbean reviewed Mar 9, 2018

View reviewed changes

tlrx approved these changes Mar 12, 2018

View reviewed changes

javanna mentioned this pull request Mar 13, 2018

Clear Cache API rest spec not up to date #26947

Closed

nik9000 approved these changes Mar 13, 2018

View reviewed changes

olcbean mentioned this pull request Mar 14, 2018

REST : Clear Indices Cache API remove deprecated url params #29068

Merged

javanna added 6 commits March 16, 2018 17:47

Merge branch 'master' into enhancement/rest_hl_client_clear_cache

6814616

prevent NPEs if indices are null, add tests and skip encoding of know…

8acef57

…n endpoints

Merge branch 'master' into enhancement/rest_hl_client_clear_cache

f848991

remove unnecessary null check

1611f65

iter

0ee3c95

fix get index issue

ee5d389

javanna merged commit ff09c82 into elastic:master Mar 20, 2018

javanna added the backport pending label Mar 20, 2018

javanna mentioned this pull request Mar 20, 2018

REST high-level client: add support for Indices Update Settings API #28892

Merged

javanna added a commit that referenced this pull request Mar 20, 2018

REST high-level client: add clear cache API (#28866)

9f65252

Relates to #27205 Also Closes #26947 (rest-spec were outdated)

javanna removed the backport pending label Mar 20, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Conversation

javanna commented Mar 1, 2018

Uh oh!

olcbean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olcbean commented Mar 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlrx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants