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 20b159222d2..501f6ed59e6 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.geo.builders.ShapeBuilder; 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 class GeoWKTParser { 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 class GeoWKTParser { 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 d736703f641..d56b8820e9b 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 class Analysis { 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 class Analysis { } } - 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 class Analysis { 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 7faaf51f4de..314eeffd7aa 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 abstract class Engine implements Closeable { @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 cb5ff580434..13f6e58cd79 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 @@ public final class ShardGetService extends AbstractIndexShardComponent { 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 @@ public final class ShardGetService extends AbstractIndexShardComponent { // 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 573e75d7806..0148ab44d1b 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 @@ public class TermVectorsService { 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 @@ public class TermVectorsService { 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 @@ public class TermVectorsService { } } // 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 8b4a3795c07..59af043e0cf 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 @@ package org.elasticsearch.search; 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.BigArrays; 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.SortAndFormats; 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 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv 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 5ed4f3252d5..10a4c9f3e1d 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.DirectoryReader; 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 class MultiTermVectorsIT extends AbstractTermVectorsTestCase { 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 class MultiTermVectorsIT extends AbstractTermVectorsTestCase { 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 class MultiTermVectorsIT extends AbstractTermVectorsTestCase { 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 class MultiTermVectorsIT extends AbstractTermVectorsTestCase { 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 979c44dd5fc..26da424460e 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 @@ package org.elasticsearch.index.engine; 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 class InternalEngineTests extends EngineTestCase { 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 31e51ed43d4..027b595ee76 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 class IndexShardTests extends IndexShardTestCase { 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 class IndexShardTests extends IndexShardTestCase { 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); }