From 6577f5b0d1c5297e4948ea1b2c7589789c505777 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Mon, 28 May 2018 11:37:11 +0200 Subject: [PATCH 1/6] silence InstallPluginCommandTests, see https://github.com/elastic/elasticsearch/issues/30900 --- .../org/elasticsearch/plugins/InstallPluginCommandTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 1db551934c768..e9d0974c1438c 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -23,6 +23,7 @@ import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import org.apache.lucene.util.LuceneTestCase; +import org.apache.lucene.util.LuceneTestCase.AwaitsFix; import org.bouncycastle.bcpg.ArmoredOutputStream; import org.bouncycastle.bcpg.BCPGOutputStream; import org.bouncycastle.bcpg.HashAlgorithmTags; @@ -115,6 +116,7 @@ import static org.hamcrest.Matchers.not; @LuceneTestCase.SuppressFileSystems("*") +@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30900") public class InstallPluginCommandTests extends ESTestCase { private InstallPluginCommand skipJarHellCommand; From 6e480663d75aa469ac76efd8553695361941d614 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 29 May 2018 09:26:02 +0200 Subject: [PATCH 2/6] Remove AllocatedPersistentTask.getState() (#30858) This commit removes the method AllocatedPersistentTask.getState() that exposes the internal state of an AllocatedPersistentTask and replaces it with a new isCompleted() method. Related to #29608. --- .../persistent/AllocatedPersistentTask.java | 40 +++++++++---------- .../PersistentTasksNodeService.java | 2 +- .../PersistentTasksNodeServiceTests.java | 14 ++++--- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/persistent/AllocatedPersistentTask.java b/server/src/main/java/org/elasticsearch/persistent/AllocatedPersistentTask.java index b311e559c6e91..e48b6c972cb04 100644 --- a/server/src/main/java/org/elasticsearch/persistent/AllocatedPersistentTask.java +++ b/server/src/main/java/org/elasticsearch/persistent/AllocatedPersistentTask.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; -import org.elasticsearch.tasks.TaskCancelledException; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.tasks.TaskManager; @@ -38,18 +37,16 @@ * Represents a executor node operation that corresponds to a persistent task */ public class AllocatedPersistentTask extends CancellableTask { - private volatile String persistentTaskId; - private volatile long allocationId; private final AtomicReference state; - @Nullable - private volatile Exception failure; + private volatile String persistentTaskId; + private volatile long allocationId; + private volatile @Nullable Exception failure; private volatile PersistentTasksService persistentTasksService; private volatile Logger logger; private volatile TaskManager taskManager; - public AllocatedPersistentTask(long id, String type, String action, String description, TaskId parentTask, Map headers) { super(id, type, action, description, parentTask, headers); @@ -101,24 +98,10 @@ public Exception getFailure() { return failure; } - boolean markAsCancelled() { - return state.compareAndSet(AllocatedPersistentTask.State.STARTED, AllocatedPersistentTask.State.PENDING_CANCEL); - } - - public State getState() { - return state.get(); - } - public long getAllocationId() { return allocationId; } - public enum State { - STARTED, // the task is currently running - PENDING_CANCEL, // the task is cancelled on master, cancelling it locally - COMPLETED // the task is done running and trying to notify caller - } - /** * Waits for this persistent task to have the desired state. */ @@ -128,6 +111,14 @@ public void waitForPersistentTaskStatus(Predicate action = mock(PersistentTasksExecutor.class); when(action.getExecutor()).thenReturn(ThreadPool.Names.SAME); @@ -131,8 +131,8 @@ public void testStartTask() throws Exception { if (added == false) { logger.info("No local node action was added"); - } + MetaData.Builder metaData = MetaData.builder(state.metaData()); metaData.putCustom(PersistentTasksCustomMetaData.TYPE, tasks.build()); ClusterState newClusterState = ClusterState.builder(state).metaData(metaData).build(); @@ -149,6 +149,7 @@ public void testStartTask() throws Exception { // Make sure action wasn't called again assertThat(executor.executions.size(), equalTo(1)); + assertThat(executor.get(0).task.isCompleted(), is(false)); // Start another task on this node state = newClusterState; @@ -157,10 +158,15 @@ public void testStartTask() throws Exception { // Make sure action was called this time assertThat(executor.size(), equalTo(2)); + assertThat(executor.get(1).task.isCompleted(), is(false)); // Finish both tasks executor.get(0).task.markAsFailed(new RuntimeException()); executor.get(1).task.markAsCompleted(); + + assertThat(executor.get(0).task.isCompleted(), is(true)); + assertThat(executor.get(1).task.isCompleted(), is(true)); + String failedTaskId = executor.get(0).task.getPersistentTaskId(); String finishedTaskId = executor.get(1).task.getPersistentTaskId(); executor.clear(); @@ -186,7 +192,6 @@ public void testStartTask() throws Exception { // Make sure action was only allocated on this node once assertThat(executor.size(), equalTo(1)); } - } public void testParamsStatusAndNodeTaskAreDelegated() throws Exception { @@ -300,7 +305,6 @@ public void sendCompletionNotification(String taskId, long allocationId, Excepti // Check the the task is now removed from task manager assertThat(taskManager.getTasks().values(), empty()); - } private ClusterState addTask(ClusterState state, String action, Params params, From 89869a2d0da6af9cfcee9c467cb5041281433fc9 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 29 May 2018 08:34:20 +0100 Subject: [PATCH 3/6] Improve allocation-disabling instructions (#30248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarify the “one minute” in the instructions to disable the shard allocation when doing maintenance to say that it is configurable. --- docs/reference/upgrade/disable-shard-alloc.asciidoc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/reference/upgrade/disable-shard-alloc.asciidoc b/docs/reference/upgrade/disable-shard-alloc.asciidoc index 107d20f1135ce..abd40336e9b08 100644 --- a/docs/reference/upgrade/disable-shard-alloc.asciidoc +++ b/docs/reference/upgrade/disable-shard-alloc.asciidoc @@ -1,8 +1,10 @@ -When you shut down a node, the allocation process waits for one minute -before starting to replicate the shards on that node to other nodes -in the cluster, causing a lot of wasted I/O. You can avoid racing the clock -by disabling allocation before shutting down the node: +When you shut down a node, the allocation process waits for +`index.unassigned.node_left.delayed_timeout` (by default, one minute) before +starting to replicate the shards on that node to other nodes in the cluster, +which can involve a lot of I/O. Since the node is shortly going to be +restarted, this I/O is unnecessary. You can avoid racing the clock by disabling +allocation before shutting down the node: [source,js] -------------------------------------------------- From c137ad0c394484225c6f10c26940ebb8cb9385d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 29 May 2018 10:31:52 +0200 Subject: [PATCH 4/6] Replace several try-finally statements (#30880) This change replaces some existing try-finally statements that close resources in their finally block with the slightly shorter and safer try-with-resources pattern. --- .../common/geo/parsers/GeoWKTParser.java | 10 +--- .../index/analysis/Analysis.java | 17 ++---- .../elasticsearch/index/engine/Engine.java | 4 -- .../index/get/ShardGetService.java | 4 +- .../index/termvectors/TermVectorsService.java | 13 ++-- .../elasticsearch/search/SearchService.java | 17 +++--- .../termvectors/MultiTermVectorsIT.java | 13 ++-- .../index/engine/InternalEngineTests.java | 60 +++++++++---------- .../index/shard/IndexShardTests.java | 20 ++++--- 9 files changed, 71 insertions(+), 87 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java b/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java index 20b159222d251..501f6ed59e687 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java +++ b/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java @@ -18,12 +18,9 @@ */ package org.elasticsearch.common.geo.parsers; -import org.locationtech.jts.geom.Coordinate; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoShapeType; - -import java.io.StringReader; import org.elasticsearch.common.geo.builders.CoordinatesBuilder; import org.elasticsearch.common.geo.builders.EnvelopeBuilder; import org.elasticsearch.common.geo.builders.GeometryCollectionBuilder; @@ -37,9 +34,11 @@ import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.GeoShapeFieldMapper; +import org.locationtech.jts.geom.Coordinate; import java.io.IOException; import java.io.StreamTokenizer; +import java.io.StringReader; import java.util.List; /** @@ -77,8 +76,7 @@ public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoSha public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoShapeType shapeType, final GeoShapeFieldMapper shapeMapper) throws IOException, ElasticsearchParseException { - StringReader reader = new StringReader(parser.text()); - try { + try (StringReader reader = new StringReader(parser.text())) { boolean ignoreZValue = (shapeMapper != null && shapeMapper.ignoreZValue().value() == true); // setup the tokenizer; configured to read words w/o numbers StreamTokenizer tokenizer = new StreamTokenizer(reader); @@ -95,8 +93,6 @@ public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoSha ShapeBuilder builder = parseGeometry(tokenizer, shapeType, ignoreZValue); checkEOF(tokenizer); return builder; - } finally { - reader.close(); } } diff --git a/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java b/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java index d736703f6418e..d56b8820e9b1c 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java @@ -234,8 +234,8 @@ public static List getWordList(Environment env, Settings settings, Strin final Path path = env.configFile().resolve(wordListPath); - try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) { - return loadWordList(reader, "#"); + try { + return loadWordList(path, "#"); } catch (CharacterCodingException ex) { String message = String.format(Locale.ROOT, "Unsupported character encoding detected while reading %s_path: %s - files must be UTF-8 encoded", @@ -247,15 +247,9 @@ public static List getWordList(Environment env, Settings settings, Strin } } - public static List loadWordList(Reader reader, String comment) throws IOException { + private static List loadWordList(Path path, String comment) throws IOException { final List result = new ArrayList<>(); - BufferedReader br = null; - try { - if (reader instanceof BufferedReader) { - br = (BufferedReader) reader; - } else { - br = new BufferedReader(reader); - } + try (BufferedReader br = Files.newBufferedReader(path, StandardCharsets.UTF_8)) { String word; while ((word = br.readLine()) != null) { if (!Strings.hasText(word)) { @@ -265,9 +259,6 @@ public static List loadWordList(Reader reader, String comment) throws IO result.add(word.trim()); } } - } finally { - if (br != null) - br.close(); } return result; } diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index 7faaf51f4de6a..314eeffd7aa6a 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1424,10 +1424,6 @@ public DocIdAndVersion docIdAndVersion() { @Override public void close() { - release(); - } - - public void release() { Releasables.close(searcher); } } diff --git a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java index cb5ff580434f0..13f6e58cd79ec 100644 --- a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java +++ b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java @@ -159,7 +159,7 @@ private GetResult innerGet(String type, String id, String[] gFields, boolean rea get = indexShard.get(new Engine.Get(realtime, readFromTranslog, type, id, uidTerm) .version(version).versionType(versionType)); if (get.exists() == false) { - get.release(); + get.close(); } } } @@ -172,7 +172,7 @@ private GetResult innerGet(String type, String id, String[] gFields, boolean rea // break between having loaded it from translog (so we only have _source), and having a document to load return innerGetLoadFromStoredFields(type, id, gFields, fetchSourceContext, get, mapperService); } finally { - get.release(); + get.close(); } } diff --git a/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java b/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java index 573e75d78060a..0148ab44d1b3e 100644 --- a/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java +++ b/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java @@ -85,8 +85,6 @@ static TermVectorsResponse getTermVectors(IndexShard indexShard, TermVectorsRequ termVectorsResponse.setExists(false); return termVectorsResponse; } - Engine.GetResult get = indexShard.get(new Engine.Get(request.realtime(), false, request.type(), request.id(), uidTerm) - .version(request.version()).versionType(request.versionType())); Fields termVectorsByField = null; AggregatedDfs dfs = null; @@ -97,8 +95,9 @@ static TermVectorsResponse getTermVectors(IndexShard indexShard, TermVectorsRequ handleFieldWildcards(indexShard, request); } - final Engine.Searcher searcher = indexShard.acquireSearcher("term_vector"); - try { + try (Engine.GetResult get = indexShard.get(new Engine.Get(request.realtime(), false, request.type(), request.id(), uidTerm) + .version(request.version()).versionType(request.versionType())); + Engine.Searcher searcher = indexShard.acquireSearcher("term_vector")) { Fields topLevelFields = MultiFields.getFields(get.searcher() != null ? get.searcher().reader() : searcher.reader()); DocIdAndVersion docIdAndVersion = get.docIdAndVersion(); /* from an artificial document */ @@ -143,14 +142,12 @@ else if (docIdAndVersion != null) { } } // write term vectors - termVectorsResponse.setFields(termVectorsByField, request.selectedFields(), request.getFlags(), topLevelFields, dfs, termVectorsFilter); + termVectorsResponse.setFields(termVectorsByField, request.selectedFields(), request.getFlags(), topLevelFields, dfs, + termVectorsFilter); } termVectorsResponse.setTookInMillis(TimeUnit.NANOSECONDS.toMillis(nanoTimeSupplier.getAsLong() - startTime)); } catch (Exception ex) { throw new ElasticsearchException("failed to execute term vector request", ex); - } finally { - searcher.close(); - get.release(); } return termVectorsResponse; } diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 8b4a3795c07dd..59af043e0cf7a 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -21,7 +21,6 @@ import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.TopDocs; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; @@ -39,6 +38,7 @@ import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.ConcurrentMapLong; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexSettings; @@ -92,8 +92,8 @@ import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.search.suggest.Suggest; import org.elasticsearch.search.suggest.completion.CompletionSuggestion; -import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.Scheduler.Cancellable; +import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.transport.TransportRequest; @@ -646,20 +646,17 @@ private void freeAllContextForIndex(Index index) { public boolean freeContext(long id) { - final SearchContext context = removeContext(id); - if (context != null) { - assert context.refCount() > 0 : " refCount must be > 0: " + context.refCount(); - try { + try (SearchContext context = removeContext(id)) { + if (context != null) { + assert context.refCount() > 0 : " refCount must be > 0: " + context.refCount(); context.indexShard().getSearchOperationListener().onFreeContext(context); if (context.scrollContext() != null) { context.indexShard().getSearchOperationListener().onFreeScrollContext(context); } - } finally { - context.close(); + return true; } - return true; + return false; } - return false; } public void freeAllScrollContexts() { diff --git a/server/src/test/java/org/elasticsearch/action/termvectors/MultiTermVectorsIT.java b/server/src/test/java/org/elasticsearch/action/termvectors/MultiTermVectorsIT.java index 5ed4f3252d57c..10a4c9f3e1d7a 100644 --- a/server/src/test/java/org/elasticsearch/action/termvectors/MultiTermVectorsIT.java +++ b/server/src/test/java/org/elasticsearch/action/termvectors/MultiTermVectorsIT.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.Fields; import org.apache.lucene.index.Terms; import org.apache.lucene.index.TermsEnum; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.common.settings.Settings; @@ -111,7 +112,8 @@ public void testMultiTermVectorsWithVersion() throws Exception { checkTermTexts(response.getResponses()[1].getResponse().getFields().terms("field"), new String[]{"value1"}); assertThat(response.getResponses()[2].getFailure(), notNullValue()); assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1")); - assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(VersionConflictEngineException.class)); + assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(ElasticsearchException.class)); + assertThat(response.getResponses()[2].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class)); //Version from Lucene index refresh(); @@ -132,7 +134,8 @@ public void testMultiTermVectorsWithVersion() throws Exception { checkTermTexts(response.getResponses()[1].getResponse().getFields().terms("field"), new String[]{"value1"}); assertThat(response.getResponses()[2].getFailure(), notNullValue()); assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1")); - assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(VersionConflictEngineException.class)); + assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(ElasticsearchException.class)); + assertThat(response.getResponses()[2].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class)); for (int i = 0; i < 3; i++) { @@ -155,7 +158,8 @@ public void testMultiTermVectorsWithVersion() throws Exception { assertThat(response.getResponses()[1].getFailure(), notNullValue()); assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2")); assertThat(response.getResponses()[1].getIndex(), equalTo("test")); - assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(VersionConflictEngineException.class)); + assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(ElasticsearchException.class)); + assertThat(response.getResponses()[1].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class)); assertThat(response.getResponses()[2].getId(), equalTo("2")); assertThat(response.getResponses()[2].getIndex(), equalTo("test")); assertThat(response.getResponses()[2].getFailure(), nullValue()); @@ -180,7 +184,8 @@ public void testMultiTermVectorsWithVersion() throws Exception { assertThat(response.getResponses()[1].getFailure(), notNullValue()); assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2")); assertThat(response.getResponses()[1].getIndex(), equalTo("test")); - assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(VersionConflictEngineException.class)); + assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(ElasticsearchException.class)); + assertThat(response.getResponses()[1].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class)); assertThat(response.getResponses()[2].getId(), equalTo("2")); assertThat(response.getResponses()[2].getIndex(), equalTo("test")); assertThat(response.getResponses()[2].getFailure(), nullValue()); diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 979c44dd5fc8d..26da424460ef2 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -21,6 +21,7 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import com.carrotsearch.randomizedtesting.generators.RandomNumbers; + import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -793,7 +794,7 @@ public void testConcurrentGetAndFlush() throws Exception { while (flushFinished.get() == false) { Engine.GetResult previousGetResult = latestGetResult.get(); if (previousGetResult != null) { - previousGetResult.release(); + previousGetResult.close(); } latestGetResult.set(engine.get(newGet(true, doc), searcherFactory)); if (latestGetResult.get().exists() == false) { @@ -807,7 +808,7 @@ public void testConcurrentGetAndFlush() throws Exception { flushFinished.set(true); getThread.join(); assertTrue(latestGetResult.get().exists()); - latestGetResult.get().release(); + latestGetResult.get().close(); } public void testSimpleOperations() throws Exception { @@ -830,21 +831,20 @@ public void testSimpleOperations() throws Exception { searchResult.close(); // but, not there non realtime - Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(false)); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(false)); + } // but, we can still get it (in realtime) - getResult = engine.get(newGet(true, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(true)); - assertThat(getResult.docIdAndVersion(), notNullValue()); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(true)); + assertThat(getResult.docIdAndVersion(), notNullValue()); + } // but not real time is not yet visible - getResult = engine.get(newGet(false, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(false)); - getResult.release(); - + try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(false)); + } // refresh and it should be there engine.refresh("test"); @@ -856,10 +856,10 @@ public void testSimpleOperations() throws Exception { searchResult.close(); // also in non realtime - getResult = engine.get(newGet(false, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(true)); - assertThat(getResult.docIdAndVersion(), notNullValue()); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(true)); + assertThat(getResult.docIdAndVersion(), notNullValue()); + } // now do an update document = testDocument(); @@ -876,10 +876,10 @@ public void testSimpleOperations() throws Exception { searchResult.close(); // but, we can still get it (in realtime) - getResult = engine.get(newGet(true, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(true)); - assertThat(getResult.docIdAndVersion(), notNullValue()); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(true)); + assertThat(getResult.docIdAndVersion(), notNullValue()); + } // refresh and it should be updated engine.refresh("test"); @@ -901,9 +901,9 @@ public void testSimpleOperations() throws Exception { searchResult.close(); // but, get should not see it (in realtime) - getResult = engine.get(newGet(true, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(false)); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(false)); + } // refresh and it should be deleted engine.refresh("test"); @@ -941,10 +941,10 @@ public void testSimpleOperations() throws Exception { engine.flush(); // and, verify get (in real time) - getResult = engine.get(newGet(true, doc), searcherFactory); - assertThat(getResult.exists(), equalTo(true)); - assertThat(getResult.docIdAndVersion(), notNullValue()); - getResult.release(); + try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) { + assertThat(getResult.exists(), equalTo(true)); + assertThat(getResult.docIdAndVersion(), notNullValue()); + } // make sure we can still work with the engine // now do an update @@ -4156,7 +4156,7 @@ public void testSeqNoGenerator() throws IOException { new Term("_id", parsedDocument.id()), parsedDocument, SequenceNumbers.UNASSIGNED_SEQ_NO, - (long) randomIntBetween(1, 8), + randomIntBetween(1, 8), Versions.MATCH_ANY, VersionType.INTERNAL, Engine.Operation.Origin.PRIMARY, @@ -4172,7 +4172,7 @@ public void testSeqNoGenerator() throws IOException { id, new Term("_id", parsedDocument.id()), SequenceNumbers.UNASSIGNED_SEQ_NO, - (long) randomIntBetween(1, 8), + randomIntBetween(1, 8), Versions.MATCH_ANY, VersionType.INTERNAL, Engine.Operation.Origin.PRIMARY, diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 31e51ed43d40e..027b595ee761f 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -1861,10 +1861,11 @@ public void testSearcherWrapperIsUsed() throws IOException { indexDoc(shard, "_doc", "1", "{\"foobar\" : \"bar\"}"); shard.refresh("test"); - Engine.GetResult getResult = shard.get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1")))); - assertTrue(getResult.exists()); - assertNotNull(getResult.searcher()); - getResult.release(); + try (Engine.GetResult getResult = shard + .get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))))) { + assertTrue(getResult.exists()); + assertNotNull(getResult.searcher()); + } try (Engine.Searcher searcher = shard.acquireSearcher("test")) { TopDocs search = searcher.searcher().search(new TermQuery(new Term("foo", "bar")), 10); assertEquals(search.totalHits, 1); @@ -1895,11 +1896,12 @@ public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { search = searcher.searcher().search(new TermQuery(new Term("foobar", "bar")), 10); assertEquals(search.totalHits, 1); } - getResult = newShard.get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1")))); - assertTrue(getResult.exists()); - assertNotNull(getResult.searcher()); // make sure get uses the wrapped reader - assertTrue(getResult.searcher().reader() instanceof FieldMaskingReader); - getResult.release(); + try (Engine.GetResult getResult = newShard + .get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))))) { + assertTrue(getResult.exists()); + assertNotNull(getResult.searcher()); // make sure get uses the wrapped reader + assertTrue(getResult.searcher().reader() instanceof FieldMaskingReader); + } closeShards(newShard); } From eaee530778bc5f71cd1fe05335dae21d2907fbbf Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 29 May 2018 10:54:41 +0200 Subject: [PATCH 5/6] Move list tasks under Tasks namespace (#30906) Our API spec define the tasks API as e.g. tasks.list, meaning that they belong to their own namespace. This commit moves them from the cluster namespace to their own namespace. Relates to #29546 --- .../elasticsearch/client/ClusterClient.java | 24 --- .../client/RestHighLevelClient.java | 10 ++ .../org/elasticsearch/client/TasksClient.java | 64 ++++++++ .../elasticsearch/client/ClusterClientIT.java | 31 ---- .../org/elasticsearch/client/TasksIT.java | 61 ++++++++ .../ClusterClientDocumentationIT.java | 94 ----------- .../SnapshotClientDocumentationIT.java | 2 +- .../TasksClientDocumentationIT.java | 148 ++++++++++++++++++ .../high-level/supported-apis.asciidoc | 10 +- .../{cluster => tasks}/list_tasks.asciidoc | 22 +-- 10 files changed, 303 insertions(+), 163 deletions(-) create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/TasksClient.java create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/TasksIT.java create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TasksClientDocumentationIT.java rename docs/java-rest/high-level/{cluster => tasks}/list_tasks.asciidoc (79%) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java index 0f9e9e582263c..846f29bfb6ef9 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java @@ -21,8 +21,6 @@ import org.apache.http.Header; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse; import org.elasticsearch.action.ingest.PutPipelineRequest; @@ -68,28 +66,6 @@ public void putSettingsAsync(ClusterUpdateSettingsRequest clusterUpdateSettingsR ClusterUpdateSettingsResponse::fromXContent, listener, emptySet(), headers); } - /** - * Get current tasks using the Task Management API - *

- * See - * Task Management API on elastic.co - */ - public ListTasksResponse listTasks(ListTasksRequest request, Header... headers) throws IOException { - return restHighLevelClient.performRequestAndParseEntity(request, RequestConverters::listTasks, ListTasksResponse::fromXContent, - emptySet(), headers); - } - - /** - * Asynchronously get current tasks using the Task Management API - *

- * See - * Task Management API on elastic.co - */ - public void listTasksAsync(ListTasksRequest request, ActionListener listener, Header... headers) { - restHighLevelClient.performRequestAsyncAndParseEntity(request, RequestConverters::listTasks, ListTasksResponse::fromXContent, - listener, emptySet(), headers); - } - /** * Add a pipeline or update an existing pipeline in the cluster *

diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index 68e32abb69dc0..8537bf3b45068 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -192,6 +192,7 @@ public class RestHighLevelClient implements Closeable { private final IndicesClient indicesClient = new IndicesClient(this); private final ClusterClient clusterClient = new ClusterClient(this); private final SnapshotClient snapshotClient = new SnapshotClient(this); + private final TasksClient tasksClient = new TasksClient(this); /** * Creates a {@link RestHighLevelClient} given the low level {@link RestClientBuilder} that allows to build the @@ -264,6 +265,15 @@ public final SnapshotClient snapshot() { return snapshotClient; } + /** + * Provides a {@link TasksClient} which can be used to access the Tasks API. + * + * See Task Management API on elastic.co + */ + public final TasksClient tasks() { + return tasksClient; + } + /** * Executes a bulk request using the Bulk API * diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/TasksClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/TasksClient.java new file mode 100644 index 0000000000000..214f1e7884a2a --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/TasksClient.java @@ -0,0 +1,64 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client; + +import org.apache.http.Header; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; + +import java.io.IOException; + +import static java.util.Collections.emptySet; + +/** + * A wrapper for the {@link RestHighLevelClient} that provides methods for accessing the Tasks API. + *

+ * See Task Management API on elastic.co + */ +public class TasksClient { + private final RestHighLevelClient restHighLevelClient; + + TasksClient(RestHighLevelClient restHighLevelClient) { + this.restHighLevelClient = restHighLevelClient; + } + + /** + * Get current tasks using the Task Management API + *

+ * See + * Task Management API on elastic.co + */ + public ListTasksResponse list(ListTasksRequest request, Header... headers) throws IOException { + return restHighLevelClient.performRequestAndParseEntity(request, RequestConverters::listTasks, ListTasksResponse::fromXContent, + emptySet(), headers); + } + + /** + * Asynchronously get current tasks using the Task Management API + *

+ * See + * Task Management API on elastic.co + */ + public void listAsync(ListTasksRequest request, ActionListener listener, Header... headers) { + restHighLevelClient.performRequestAsyncAndParseEntity(request, RequestConverters::listTasks, ListTasksResponse::fromXContent, + listener, emptySet(), headers); + } +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java index d41117ceb6dd6..44332b058bc15 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java @@ -20,9 +20,6 @@ package org.elasticsearch.client; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; -import org.elasticsearch.action.admin.cluster.node.tasks.list.TaskGroup; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse; import org.elasticsearch.action.ingest.PutPipelineRequest; @@ -37,16 +34,13 @@ import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.ingest.Pipeline; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.tasks.TaskInfo; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import static java.util.Collections.emptyList; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -117,31 +111,6 @@ public void testClusterUpdateSettingNonExistent() { "Elasticsearch exception [type=illegal_argument_exception, reason=transient setting [" + setting + "], not recognized]")); } - public void testListTasks() throws IOException { - ListTasksRequest request = new ListTasksRequest(); - ListTasksResponse response = execute(request, highLevelClient().cluster()::listTasks, highLevelClient().cluster()::listTasksAsync); - - assertThat(response, notNullValue()); - assertThat(response.getNodeFailures(), equalTo(emptyList())); - assertThat(response.getTaskFailures(), equalTo(emptyList())); - // It's possible that there are other tasks except 'cluster:monitor/tasks/lists[n]' and 'action":"cluster:monitor/tasks/lists' - assertThat(response.getTasks().size(), greaterThanOrEqualTo(2)); - boolean listTasksFound = false; - for (TaskGroup taskGroup : response.getTaskGroups()) { - TaskInfo parent = taskGroup.getTaskInfo(); - if ("cluster:monitor/tasks/lists".equals(parent.getAction())) { - assertThat(taskGroup.getChildTasks().size(), equalTo(1)); - TaskGroup childGroup = taskGroup.getChildTasks().iterator().next(); - assertThat(childGroup.getChildTasks().isEmpty(), equalTo(true)); - TaskInfo child = childGroup.getTaskInfo(); - assertThat(child.getAction(), equalTo("cluster:monitor/tasks/lists[n]")); - assertThat(child.getParentTaskId(), equalTo(parent.getTaskId())); - listTasksFound = true; - } - } - assertTrue("List tasks were not found", listTasksFound); - } - public void testPutPipeline() throws IOException { String id = "some_pipeline_id"; XContentType xContentType = randomFrom(XContentType.values()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/TasksIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/TasksIT.java new file mode 100644 index 0000000000000..fc7d70a36e10e --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/TasksIT.java @@ -0,0 +1,61 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client; + +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; +import org.elasticsearch.action.admin.cluster.node.tasks.list.TaskGroup; +import org.elasticsearch.tasks.TaskInfo; + +import java.io.IOException; + +import static java.util.Collections.emptyList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; + +public class TasksIT extends ESRestHighLevelClientTestCase { + + public void testListTasks() throws IOException { + ListTasksRequest request = new ListTasksRequest(); + ListTasksResponse response = execute(request, highLevelClient().tasks()::list, highLevelClient().tasks()::listAsync); + + assertThat(response, notNullValue()); + assertThat(response.getNodeFailures(), equalTo(emptyList())); + assertThat(response.getTaskFailures(), equalTo(emptyList())); + // It's possible that there are other tasks except 'cluster:monitor/tasks/lists[n]' and 'action":"cluster:monitor/tasks/lists' + assertThat(response.getTasks().size(), greaterThanOrEqualTo(2)); + boolean listTasksFound = false; + for (TaskGroup taskGroup : response.getTaskGroups()) { + TaskInfo parent = taskGroup.getTaskInfo(); + if ("cluster:monitor/tasks/lists".equals(parent.getAction())) { + assertThat(taskGroup.getChildTasks().size(), equalTo(1)); + TaskGroup childGroup = taskGroup.getChildTasks().iterator().next(); + assertThat(childGroup.getChildTasks().isEmpty(), equalTo(true)); + TaskInfo child = childGroup.getTaskInfo(); + assertThat(child.getAction(), equalTo("cluster:monitor/tasks/lists[n]")); + assertThat(child.getParentTaskId(), equalTo(parent.getTaskId())); + listTasksFound = true; + } + } + assertTrue("List tasks were not found", listTasksFound); + } + +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java index b9329f99a3cde..29bb2d05afcc7 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java @@ -19,13 +19,8 @@ package org.elasticsearch.client.documentation; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.LatchedActionListener; -import org.elasticsearch.action.TaskOperationFailure; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; -import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; -import org.elasticsearch.action.admin.cluster.node.tasks.list.TaskGroup; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse; import org.elasticsearch.action.ingest.PutPipelineRequest; @@ -39,21 +34,15 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.indices.recovery.RecoverySettings; -import org.elasticsearch.tasks.TaskId; -import org.elasticsearch.tasks.TaskInfo; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import static java.util.Collections.emptyList; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.notNullValue; /** * This class is used to generate the Java Cluster API documentation. @@ -193,89 +182,6 @@ public void onFailure(Exception e) { } } - public void testListTasks() throws IOException { - RestHighLevelClient client = highLevelClient(); - { - // tag::list-tasks-request - ListTasksRequest request = new ListTasksRequest(); - // end::list-tasks-request - - // tag::list-tasks-request-filter - request.setActions("cluster:*"); // <1> - request.setNodes("nodeId1", "nodeId2"); // <2> - request.setParentTaskId(new TaskId("parentTaskId", 42)); // <3> - // end::list-tasks-request-filter - - // tag::list-tasks-request-detailed - request.setDetailed(true); // <1> - // end::list-tasks-request-detailed - - // tag::list-tasks-request-wait-completion - request.setWaitForCompletion(true); // <1> - request.setTimeout(TimeValue.timeValueSeconds(50)); // <2> - request.setTimeout("50s"); // <3> - // end::list-tasks-request-wait-completion - } - - ListTasksRequest request = new ListTasksRequest(); - - // tag::list-tasks-execute - ListTasksResponse response = client.cluster().listTasks(request); - // end::list-tasks-execute - - assertThat(response, notNullValue()); - - // tag::list-tasks-response-tasks - List tasks = response.getTasks(); // <1> - // end::list-tasks-response-tasks - - // tag::list-tasks-response-calc - Map> perNodeTasks = response.getPerNodeTasks(); // <1> - List groups = response.getTaskGroups(); // <2> - // end::list-tasks-response-calc - - // tag::list-tasks-response-failures - List nodeFailures = response.getNodeFailures(); // <1> - List taskFailures = response.getTaskFailures(); // <2> - // end::list-tasks-response-failures - - assertThat(response.getNodeFailures(), equalTo(emptyList())); - assertThat(response.getTaskFailures(), equalTo(emptyList())); - assertThat(response.getTasks().size(), greaterThanOrEqualTo(2)); - } - - public void testListTasksAsync() throws Exception { - RestHighLevelClient client = highLevelClient(); - { - ListTasksRequest request = new ListTasksRequest(); - - // tag::list-tasks-execute-listener - ActionListener listener = - new ActionListener() { - @Override - public void onResponse(ListTasksResponse response) { - // <1> - } - - @Override - public void onFailure(Exception e) { - // <2> - } - }; - // end::list-tasks-execute-listener - - // Replace the empty listener by a blocking listener in test - final CountDownLatch latch = new CountDownLatch(1); - listener = new LatchedActionListener<>(listener, latch); - - // tag::list-tasks-execute-async - client.cluster().listTasksAsync(request, listener); // <1> - // end::list-tasks-execute-async - - assertTrue(latch.await(30L, TimeUnit.SECONDS)); - } - } - public void testPutPipeline() throws IOException { RestHighLevelClient client = highLevelClient(); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java index 0a57fafe5be59..dc749a96bb85c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java @@ -45,7 +45,7 @@ import static org.hamcrest.Matchers.equalTo; /** - * This class is used to generate the Java Cluster API documentation. + * This class is used to generate the Java Snapshot API documentation. * You need to wrap your code between two tags like: * // tag::example * // end::example diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TasksClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TasksClientDocumentationIT.java new file mode 100644 index 0000000000000..faf447a4143b1 --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TasksClientDocumentationIT.java @@ -0,0 +1,148 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client.documentation; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.LatchedActionListener; +import org.elasticsearch.action.TaskOperationFailure; +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; +import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; +import org.elasticsearch.action.admin.cluster.node.tasks.list.TaskGroup; +import org.elasticsearch.client.ESRestHighLevelClientTestCase; +import org.elasticsearch.client.RestHighLevelClient; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.tasks.TaskId; +import org.elasticsearch.tasks.TaskInfo; + +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static java.util.Collections.emptyList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; + +/** + * This class is used to generate the Java Tasks API documentation. + * You need to wrap your code between two tags like: + * // tag::example + * // end::example + * + * Where example is your tag name. + * + * Then in the documentation, you can extract what is between tag and end tags with + * ["source","java",subs="attributes,callouts,macros"] + * -------------------------------------------------- + * include-tagged::{doc-tests}/{@link TasksClientDocumentationIT}.java[example] + * -------------------------------------------------- + * + * The column width of the code block is 84. If the code contains a line longer + * than 84, the line will be cut and a horizontal scroll bar will be displayed. + * (the code indentation of the tag is not included in the width) + */ +public class TasksClientDocumentationIT extends ESRestHighLevelClientTestCase { + + public void testListTasks() throws IOException { + RestHighLevelClient client = highLevelClient(); + { + // tag::list-tasks-request + ListTasksRequest request = new ListTasksRequest(); + // end::list-tasks-request + + // tag::list-tasks-request-filter + request.setActions("cluster:*"); // <1> + request.setNodes("nodeId1", "nodeId2"); // <2> + request.setParentTaskId(new TaskId("parentTaskId", 42)); // <3> + // end::list-tasks-request-filter + + // tag::list-tasks-request-detailed + request.setDetailed(true); // <1> + // end::list-tasks-request-detailed + + // tag::list-tasks-request-wait-completion + request.setWaitForCompletion(true); // <1> + request.setTimeout(TimeValue.timeValueSeconds(50)); // <2> + request.setTimeout("50s"); // <3> + // end::list-tasks-request-wait-completion + } + + ListTasksRequest request = new ListTasksRequest(); + + // tag::list-tasks-execute + ListTasksResponse response = client.tasks().list(request); + // end::list-tasks-execute + + assertThat(response, notNullValue()); + + // tag::list-tasks-response-tasks + List tasks = response.getTasks(); // <1> + // end::list-tasks-response-tasks + + // tag::list-tasks-response-calc + Map> perNodeTasks = response.getPerNodeTasks(); // <1> + List groups = response.getTaskGroups(); // <2> + // end::list-tasks-response-calc + + // tag::list-tasks-response-failures + List nodeFailures = response.getNodeFailures(); // <1> + List taskFailures = response.getTaskFailures(); // <2> + // end::list-tasks-response-failures + + assertThat(response.getNodeFailures(), equalTo(emptyList())); + assertThat(response.getTaskFailures(), equalTo(emptyList())); + assertThat(response.getTasks().size(), greaterThanOrEqualTo(2)); + } + + public void testListTasksAsync() throws Exception { + RestHighLevelClient client = highLevelClient(); + { + ListTasksRequest request = new ListTasksRequest(); + + // tag::list-tasks-execute-listener + ActionListener listener = + new ActionListener() { + @Override + public void onResponse(ListTasksResponse response) { + // <1> + } + + @Override + public void onFailure(Exception e) { + // <2> + } + }; + // end::list-tasks-execute-listener + + // Replace the empty listener by a blocking listener in test + final CountDownLatch latch = new CountDownLatch(1); + listener = new LatchedActionListener<>(listener, latch); + + // tag::list-tasks-execute-async + client.tasks().listAsync(request, listener); // <1> + // end::list-tasks-execute-async + + assertTrue(latch.await(30L, TimeUnit.SECONDS)); + } + } +} diff --git a/docs/java-rest/high-level/supported-apis.asciidoc b/docs/java-rest/high-level/supported-apis.asciidoc index b04cbb8df79b7..981c2caa543ac 100644 --- a/docs/java-rest/high-level/supported-apis.asciidoc +++ b/docs/java-rest/high-level/supported-apis.asciidoc @@ -104,11 +104,9 @@ include::indices/put_template.asciidoc[] The Java High Level REST Client supports the following Cluster APIs: * <> -* <> * <> include::cluster/put_settings.asciidoc[] -include::cluster/list_tasks.asciidoc[] include::cluster/put_pipeline.asciidoc[] == Snapshot APIs @@ -122,3 +120,11 @@ The Java High Level REST Client supports the following Snapshot APIs: include::snapshot/get_repository.asciidoc[] include::snapshot/create_repository.asciidoc[] include::snapshot/delete_repository.asciidoc[] + +== Tasks APIs + +The Java High Level REST Client supports the following Tasks APIs: + +* <> + +include::tasks/list_tasks.asciidoc[] diff --git a/docs/java-rest/high-level/cluster/list_tasks.asciidoc b/docs/java-rest/high-level/tasks/list_tasks.asciidoc similarity index 79% rename from docs/java-rest/high-level/cluster/list_tasks.asciidoc rename to docs/java-rest/high-level/tasks/list_tasks.asciidoc index 1a2117b2e66e6..e60ca61247e74 100644 --- a/docs/java-rest/high-level/cluster/list_tasks.asciidoc +++ b/docs/java-rest/high-level/tasks/list_tasks.asciidoc @@ -1,4 +1,4 @@ -[[java-rest-high-cluster-list-tasks]] +[[java-rest-high-tasks-list]] === List Tasks API The List Tasks API allows to get information about the tasks currently executing in the cluster. @@ -10,7 +10,7 @@ A `ListTasksRequest`: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-request] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-request] -------------------------------------------------- There is no required parameters. By default the client will list all tasks and will not wait for task completion. @@ -19,7 +19,7 @@ for task completion. ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-request-filter] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-request-filter] -------------------------------------------------- <1> Request only cluster-related tasks <2> Request all tasks running on nodes nodeId1 and nodeId2 @@ -27,13 +27,13 @@ include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-request ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-request-detailed] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-request-detailed] -------------------------------------------------- <1> Should the information include detailed, potentially slow to generate data. Defaults to `false` ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-request-wait-completion] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-request-wait-completion] -------------------------------------------------- <1> Should this request wait for all found tasks to complete. Defaults to `false` <2> Timeout for the request as a `TimeValue`. Applicable only if `setWaitForCompletion` is `true`. @@ -45,7 +45,7 @@ Defaults to 30 seconds ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-execute] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-execute] -------------------------------------------------- [[java-rest-high-cluster-list-tasks-async]] @@ -57,7 +57,7 @@ passed to the asynchronous method: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-execute-async] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-execute-async] -------------------------------------------------- <1> The `ListTasksRequest` to execute and the `ActionListener` to use when the execution completes @@ -71,7 +71,7 @@ A typical listener for `ListTasksResponse` looks like: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-execute-listener] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-execute-listener] -------------------------------------------------- <1> Called when the execution is successfully completed. The response is provided as an argument @@ -82,20 +82,20 @@ provided as an argument ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-response-tasks] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-response-tasks] -------------------------------------------------- <1> List of currently running tasks ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-response-calc] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-response-calc] -------------------------------------------------- <1> List of tasks grouped by a node <2> List of tasks grouped by a parent task ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/ClusterClientDocumentationIT.java[list-tasks-response-failures] +include-tagged::{doc-tests}/TasksClientDocumentationIT.java[list-tasks-response-failures] -------------------------------------------------- <1> List of node failures <2> List of tasks failures From 3c918d799c0003dbc93012a535aaff7701ee1db6 Mon Sep 17 00:00:00 2001 From: Sohaib Iftikhar Date: Tue, 29 May 2018 15:45:53 +0200 Subject: [PATCH 6/6] Deprecate accepting malformed requests in stored script API (#28939) The stored scripts API today accepts malformed requests instead of throwing an exception. This PR deprecates accepting malformed put stored script requests (requests not using the official script format). Relates to #27612 --- .../script/mustache/SearchTemplateIT.java | 4 ++ .../script/StoredScriptSource.java | 61 ++++++++++++------- .../script/ScriptMetaDataTests.java | 7 +++ .../script/StoredScriptSourceTests.java | 4 +- .../script/StoredScriptTests.java | 8 ++- 5 files changed, 59 insertions(+), 25 deletions(-) diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java index fe2fedf62b559..884e26e7df855 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java @@ -198,6 +198,7 @@ public void testIndexedTemplateClient() throws Exception { getResponse = client().admin().cluster().prepareGetStoredScript("testTemplate").get(); assertNull(getResponse.getSource()); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } public void testIndexedTemplate() throws Exception { @@ -267,6 +268,7 @@ public void testIndexedTemplate() throws Exception { .setScript("2").setScriptType(ScriptType.STORED).setScriptParams(templateParams) .get(); assertHitCount(searchResponse.getResponse(), 1); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } // Relates to #10397 @@ -311,6 +313,7 @@ public void testIndexedTemplateOverwrite() throws Exception { .get(); assertHitCount(searchResponse.getResponse(), 1); } + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } public void testIndexedTemplateWithArray() throws Exception { @@ -339,6 +342,7 @@ public void testIndexedTemplateWithArray() throws Exception { .setScript("4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams) .get(); assertHitCount(searchResponse.getResponse(), 5); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } } diff --git a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java index da6dad1dff384..11f8769c86b1f 100644 --- a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java +++ b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -74,6 +74,11 @@ public class StoredScriptSource extends AbstractDiffable imp */ public static final ParseField TEMPLATE_PARSE_FIELD = new ParseField("template"); + /** + * Standard {@link ParseField} for query on the inner field. + */ + public static final ParseField TEMPLATE_NO_WRAPPER_PARSE_FIELD = new ParseField("query"); + /** * Standard {@link ParseField} for lang on the inner level. */ @@ -189,6 +194,26 @@ private StoredScriptSource build(boolean ignoreEmpty) { PARSER.declareField(Builder::setOptions, XContentParser::mapStrings, OPTIONS_PARSE_FIELD, ValueType.OBJECT); } + private static StoredScriptSource parseRemaining(Token token, XContentParser parser) throws IOException { + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { + if (token != Token.START_OBJECT) { + builder.startObject(); + builder.copyCurrentStructure(parser); + builder.endObject(); + } else { + builder.copyCurrentStructure(parser); + } + + String source = Strings.toString(builder); + + if (source == null || source.isEmpty()) { + DEPRECATION_LOGGER.deprecated("empty templates should no longer be used"); + } + + return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap()); + } + } + /** * This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed: * @@ -304,38 +329,28 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon } else { throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, ]"); } - } else { - if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) { - token = parser.nextToken(); - - if (token == Token.VALUE_STRING) { - String source = parser.text(); - - if (source == null || source.isEmpty()) { - DEPRECATION_LOGGER.deprecated("empty templates should no longer be used"); - } - - return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap()); - } - } + } else if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) { - try (XContentBuilder builder = XContentFactory.jsonBuilder()) { - if (token != Token.START_OBJECT) { - builder.startObject(); - builder.copyCurrentStructure(parser); - builder.endObject(); - } else { - builder.copyCurrentStructure(parser); - } + DEPRECATION_LOGGER.deprecated("the template context is now deprecated. Specify templates in a \"script\" element."); - String source = Strings.toString(builder); + token = parser.nextToken(); + if (token == Token.VALUE_STRING) { + String source = parser.text(); if (source == null || source.isEmpty()) { DEPRECATION_LOGGER.deprecated("empty templates should no longer be used"); } return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap()); + } else { + return parseRemaining(token, parser); } + } else if (TEMPLATE_NO_WRAPPER_PARSE_FIELD.getPreferredName().equals(name)) { + DEPRECATION_LOGGER.deprecated("the template context is now deprecated. Specify templates in a \"script\" element."); + return parseRemaining(token, parser); + } else { + DEPRECATION_LOGGER.deprecated("scripts should not be stored without a context. Specify them in a \"script\" element."); + return parseRemaining(token, parser); } } catch (IOException ioe) { throw new UncheckedIOException(ioe); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java index 32d4d48a44810..bef20190acf87 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java @@ -81,10 +81,12 @@ public void testGetScript() throws Exception { XContentBuilder sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().startObject("template").field("field", "value").endObject().endObject(); builder.storeScript("template", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType())); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().field("template", "value").endObject(); builder.storeScript("template_field", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType())); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().startObject("script").field("lang", "_lang").field("source", "_source").endObject().endObject(); @@ -99,14 +101,19 @@ public void testGetScript() throws Exception { public void testDiff() throws Exception { ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null); builder.storeScript("1", StoredScriptSource.parse(new BytesArray("{\"foo\":\"abc\"}"), XContentType.JSON)); + assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element."); builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"def\"}"), XContentType.JSON)); + assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element."); builder.storeScript("3", StoredScriptSource.parse(new BytesArray("{\"foo\":\"ghi\"}"), XContentType.JSON)); + assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element."); ScriptMetaData scriptMetaData1 = builder.build(); builder = new ScriptMetaData.Builder(scriptMetaData1); builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"changed\"}"), XContentType.JSON)); + assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element."); builder.deleteScript("3"); builder.storeScript("4", StoredScriptSource.parse(new BytesArray("{\"foo\":\"jkl\"}"), XContentType.JSON)); + assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element."); ScriptMetaData scriptMetaData2 = builder.build(); ScriptMetaData.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1); diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java index 8aa4ca57acfed..49e2623626895 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java @@ -50,7 +50,9 @@ protected StoredScriptSource createTestInstance() { if (randomBoolean()) { options.put(Script.CONTENT_TYPE_OPTION, xContentType.mediaType()); } - return StoredScriptSource.parse(BytesReference.bytes(template), xContentType); + StoredScriptSource source = StoredScriptSource.parse(BytesReference.bytes(template), xContentType); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); + return source; } catch (IOException e) { throw new AssertionError("Failed to create test instance", e); } diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java index 79e3195f3d923..04483c869d9b3 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -74,6 +74,7 @@ public void testSourceParsing() throws Exception { StoredScriptSource source = new StoredScriptSource("mustache", "code", Collections.emptyMap()); assertThat(parsed, equalTo(source)); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } // complex template with wrapper template object @@ -89,6 +90,7 @@ public void testSourceParsing() throws Exception { StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap()); assertThat(parsed, equalTo(source)); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } // complex template with no wrapper object @@ -104,6 +106,7 @@ public void testSourceParsing() throws Exception { StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap()); assertThat(parsed, equalTo(source)); + assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element."); } // complex template using script as the field name @@ -223,7 +226,10 @@ public void testEmptyTemplateDeprecations() throws IOException { StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap()); assertThat(parsed, equalTo(source)); - assertWarnings("empty templates should no longer be used"); + assertWarnings( + "the template context is now deprecated. Specify templates in a \"script\" element.", + "empty templates should no longer be used" + ); } try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {