From bade2ea7d25326bc9e704afbad633b1c2ee5a90c Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 8 May 2019 15:15:26 -0600 Subject: [PATCH 1/5] Reproduce --- .../org/elasticsearch/index/reindex/ManyDocumentsIT.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java index b86f28452cc96..dc090d6549eec 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.reindex; +import org.apache.http.HttpHost; import org.elasticsearch.client.Request; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.Before; @@ -52,13 +53,19 @@ public void setupTestIndex() throws IOException { public void testReindex() throws IOException { Request request = new Request("POST", "/_reindex"); + HttpHost httpHost = getClusterHosts().stream().filter(h -> "127.0.0.1".equals(h.getHostName())).findFirst().get(); request.setJsonEntity( "{\n" + " \"source\":{\n" + + "\"remote\": {" + + "\"host\": \"" + httpHost.toString() + "\"\n" + + "},\n" + " \"index\":\"test\"\n" + " },\n" + " \"dest\":{\n" + - " \"index\":\"des\"\n" + + " \"index\":\"des\",\n" + + " \"version_type\": \"external\"\n" + +// " \"index\":\"des\"\n" + " }\n" + "}"); Map response = entityAsMap(client().performRequest(request)); From 2498c41b805da967da73d0e3c89094a504865ab6 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 8 May 2019 18:11:27 -0600 Subject: [PATCH 2/5] Changes --- .../index/reindex/remote/RemoteRequestBuilders.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 90332c7d55c9c..8d4296a89d46b 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -75,13 +75,15 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter("scroll", keepAlive.getStringRep()); } request.addParameter("size", Integer.toString(searchRequest.source().size())); - if (searchRequest.source().version() == null || searchRequest.source().version() == true) { + if (searchRequest.source().version() == null || searchRequest.source().version() == false) { /* * Passing `null` here just add the `version` request parameter * without any value. This way of requesting the version works * for all supported versions of Elasticsearch. */ request.addParameter("version", null); + } else { + request.addParameter("version", Boolean.TRUE.toString()); } if (searchRequest.source().sorts() != null) { boolean useScan = false; From ad69803f32b67f776c36e56e4d144ac1156fba88 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 22 May 2019 14:24:40 -0600 Subject: [PATCH 3/5] Changes --- .../reindex/remote/RemoteRequestBuilders.java | 9 ++-- .../remote/ReindexFromOldRemoteIT.java | 43 +++++++++++++------ .../remote/RemoteRequestBuildersTests.java | 23 +++++----- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 8d4296a89d46b..38c5d08651f8f 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -75,16 +75,13 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter("scroll", keepAlive.getStringRep()); } request.addParameter("size", Integer.toString(searchRequest.source().size())); + if (searchRequest.source().version() == null || searchRequest.source().version() == false) { - /* - * Passing `null` here just add the `version` request parameter - * without any value. This way of requesting the version works - * for all supported versions of Elasticsearch. - */ - request.addParameter("version", null); + request.addParameter("version", Boolean.FALSE.toString()); } else { request.addParameter("version", Boolean.TRUE.toString()); } + if (searchRequest.source().sorts() != null) { boolean useScan = false; // Detect if we should use search_type=scan rather than a sort diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/ReindexFromOldRemoteIT.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/ReindexFromOldRemoteIT.java index 9feed83595ff1..27d975c4114f9 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/ReindexFromOldRemoteIT.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/ReindexFromOldRemoteIT.java @@ -56,19 +56,38 @@ private void oldEsTestCase(String portPropertyName, String requestsPerSecond) th } Request reindex = new Request("POST", "/_reindex"); - reindex.setJsonEntity( + if (randomBoolean()) { + // Reindex using the external version_type + reindex.setJsonEntity( "{\n" - + " \"source\":{\n" - + " \"index\": \"test\",\n" - + " \"size\": 1,\n" - + " \"remote\": {\n" - + " \"host\": \"http://127.0.0.1:" + oldEsPort + "\"\n" - + " }\n" - + " },\n" - + " \"dest\": {\n" - + " \"index\": \"test\"\n" - + " }\n" - + "}"); + + " \"source\":{\n" + + " \"index\": \"test\",\n" + + " \"size\": 1,\n" + + " \"remote\": {\n" + + " \"host\": \"http://127.0.0.1:" + oldEsPort + "\"\n" + + " }\n" + + " },\n" + + " \"dest\": {\n" + + " \"index\": \"test\",\n" + + " \"version_type\": \"external\"\n" + + " }\n" + + "}"); + } else { + // Reindex using the default internal version_type + reindex.setJsonEntity( + "{\n" + + " \"source\":{\n" + + " \"index\": \"test\",\n" + + " \"size\": 1,\n" + + " \"remote\": {\n" + + " \"host\": \"http://127.0.0.1:" + oldEsPort + "\"\n" + + " }\n" + + " },\n" + + " \"dest\": {\n" + + " \"index\": \"test\"\n" + + " }\n" + + "}"); + } reindex.addParameter("refresh", "true"); reindex.addParameter("pretty", "true"); if (requestsPerSecond != null) { diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java index bf6856754044d..c09496e9ce564 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java @@ -136,7 +136,7 @@ public void testInitialSearchParamsFields() { // Test request without any fields Version remoteVersion = Version.fromId(between(2000099, Version.CURRENT.id)); assertThat(initialSearch(searchRequest, query, remoteVersion).getParameters(), - not(either(hasKey("stored_fields")).or(hasKey("fields")))); + not(either(hasKey("stored_fields")).or(hasKey("fields")))); // Test stored_fields for versions that support it searchRequest = new SearchRequest().source(new SearchSourceBuilder()); @@ -157,14 +157,14 @@ public void testInitialSearchParamsFields() { searchRequest.source().storedField("_source").storedField("_id"); remoteVersion = Version.fromId(between(0, 2000099 - 1)); assertThat(initialSearch(searchRequest, query, remoteVersion).getParameters(), - hasEntry("fields", "_source,_id,_parent,_routing,_ttl")); + hasEntry("fields", "_source,_id,_parent,_routing,_ttl")); // But only versions before 1.0 force _source to be in the list searchRequest = new SearchRequest().source(new SearchSourceBuilder()); searchRequest.source().storedField("_id"); remoteVersion = Version.fromId(between(1000099, 2000099 - 1)); assertThat(initialSearch(searchRequest, query, remoteVersion).getParameters(), - hasEntry("fields", "_id,_parent,_routing,_ttl")); + hasEntry("fields", "_id,_parent,_routing,_ttl")); } public void testInitialSearchParamsMisc() { @@ -184,7 +184,7 @@ public void testInitialSearchParamsMisc() { fetchVersion = randomBoolean(); searchRequest.source().version(fetchVersion); } - + Map params = initialSearch(searchRequest, query, remoteVersion).getParameters(); if (scroll == null) { @@ -193,7 +193,10 @@ public void testInitialSearchParamsMisc() { assertScroll(remoteVersion, params, scroll); } assertThat(params, hasEntry("size", Integer.toString(size))); - assertThat(params, fetchVersion == null || fetchVersion == true ? hasEntry("version", null) : not(hasEntry("version", null))); + if (fetchVersion != null) { + assertThat(params, fetchVersion ? hasEntry("version", Boolean.TRUE.toString()) : + hasEntry("version", Boolean.FALSE.toString())); + } } private void assertScroll(Version remoteVersion, Map params, TimeValue requested) { @@ -220,22 +223,22 @@ public void testInitialSearchEntity() throws IOException { assertEquals(ContentType.APPLICATION_JSON.toString(), entity.getContentType().getValue()); if (remoteVersion.onOrAfter(Version.fromId(1000099))) { assertEquals("{\"query\":" + query + ",\"_source\":true}", - Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); + Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); } else { assertEquals("{\"query\":" + query + "}", - Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); + Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); } // Source filtering is included if set up - searchRequest.source().fetchSource(new String[] {"in1", "in2"}, new String[] {"out"}); + searchRequest.source().fetchSource(new String[]{"in1", "in2"}, new String[]{"out"}); entity = initialSearch(searchRequest, new BytesArray(query), remoteVersion).getEntity(); assertEquals(ContentType.APPLICATION_JSON.toString(), entity.getContentType().getValue()); assertEquals("{\"query\":" + query + ",\"_source\":{\"includes\":[\"in1\",\"in2\"],\"excludes\":[\"out\"]}}", - Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); + Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8))); // Invalid XContent fails RuntimeException e = expectThrows(RuntimeException.class, - () -> initialSearch(searchRequest, new BytesArray("{}, \"trailing\": {}"), remoteVersion)); + () -> initialSearch(searchRequest, new BytesArray("{}, \"trailing\": {}"), remoteVersion)); assertThat(e.getCause().getMessage(), containsString("Unexpected character (',' (code 44))")); e = expectThrows(RuntimeException.class, () -> initialSearch(searchRequest, new BytesArray("{"), remoteVersion)); assertThat(e.getCause().getMessage(), containsString("Unexpected end-of-input")); From 52b86ff2894db54f719ef38f296948642deb1859 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 22 May 2019 14:57:07 -0600 Subject: [PATCH 4/5] Changes --- modules/reindex/build.gradle | 2 +- .../reindex/remote/RemoteRequestBuilders.java | 2 +- .../index/reindex/ManyDocumentsIT.java | 48 +++++++++++-------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle index da184deedaa11..20c11bd8be6d4 100644 --- a/modules/reindex/build.gradle +++ b/modules/reindex/build.gradle @@ -77,7 +77,7 @@ forbiddenPatterns { exclude '**/*.p12' } -// Support for testing reindex-from-remote against old Elaticsearch versions +// Support for testing reindex-from-remote against old Elasticsearch versions configurations { oldesFixture es2 diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 38c5d08651f8f..6232ead65fb60 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -75,7 +75,7 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter("scroll", keepAlive.getStringRep()); } request.addParameter("size", Integer.toString(searchRequest.source().size())); - + if (searchRequest.source().version() == null || searchRequest.source().version() == false) { request.addParameter("version", Boolean.FALSE.toString()); } else { diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java index dc090d6549eec..fd8cf49cc39ad 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ManyDocumentsIT.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.reindex; -import org.apache.http.HttpHost; import org.elasticsearch.client.Request; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.Before; @@ -53,19 +52,13 @@ public void setupTestIndex() throws IOException { public void testReindex() throws IOException { Request request = new Request("POST", "/_reindex"); - HttpHost httpHost = getClusterHosts().stream().filter(h -> "127.0.0.1".equals(h.getHostName())).findFirst().get(); request.setJsonEntity( "{\n" + " \"source\":{\n" + - "\"remote\": {" + - "\"host\": \"" + httpHost.toString() + "\"\n" + - "},\n" + " \"index\":\"test\"\n" + " },\n" + " \"dest\":{\n" + - " \"index\":\"des\",\n" + - " \"version_type\": \"external\"\n" + -// " \"index\":\"des\"\n" + + " \"index\":\"des\"\n" + " }\n" + "}"); Map response = entityAsMap(client().performRequest(request)); @@ -80,18 +73,35 @@ public void testReindexFromRemote() throws IOException { Map http = (Map) nodeInfo.get("http"); String remote = "http://"+ http.get("publish_address"); Request request = new Request("POST", "/_reindex"); - request.setJsonEntity( + if (randomBoolean()) { + request.setJsonEntity( "{\n" + - " \"source\":{\n" + - " \"index\":\"test\",\n" + - " \"remote\":{\n" + - " \"host\":\"" + remote + "\"\n" + - " }\n" + - " }\n," + - " \"dest\":{\n" + - " \"index\":\"des\"\n" + - " }\n" + - "}"); + " \"source\":{\n" + + " \"index\":\"test\",\n" + + " \"remote\":{\n" + + " \"host\":\"" + remote + "\"\n" + + " }\n" + + " }\n," + + " \"dest\":{\n" + + " \"index\":\"des\"\n" + + " }\n" + + "}"); + } else { + // Test with external version_type + request.setJsonEntity( + "{\n" + + " \"source\":{\n" + + " \"index\":\"test\",\n" + + " \"remote\":{\n" + + " \"host\":\"" + remote + "\"\n" + + " }\n" + + " }\n," + + " \"dest\":{\n" + + " \"index\":\"des\",\n" + + " \"version_type\": \"external\"\n" + + " }\n" + + "}"); + } Map response = entityAsMap(client().performRequest(request)); assertThat(response, hasEntry("total", count)); assertThat(response, hasEntry("created", count)); From 623d415c9530a5842895c9893b508f796266ddbe Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 29 May 2019 09:37:32 -0600 Subject: [PATCH 5/5] Add condition --- .../index/reindex/remote/RemoteRequestBuildersTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java index 627e159155e55..0005fa921b33b 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java @@ -173,6 +173,8 @@ public void testInitialSearchParamsMisc() { if (fetchVersion != null) { assertThat(params, fetchVersion ? hasEntry("version", Boolean.TRUE.toString()) : hasEntry("version", Boolean.FALSE.toString())); + } else { + assertThat(params, hasEntry("version", Boolean.FALSE.toString())); } }