honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Internal: Remove threadlocal from document parser 

The doc parser uses a context object to store the state of parsing,
namely the existing mappings, new mappings, and the parsed document.
Currently this uses a threadlocal which is "reset" for each doc parsed.
However, the thread local doesn't actually save anything, since
resetting is constructing new objects. This change removes the thread
local, which also simplifies the mapper service as it now does not need
to be closeable.
This commit is contained in:
Ryan Ernst 2016-04-14 11:55:17 -07:00
parent f35cfc3715
commit c6dd17a2c6
6 changed files with 50 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java
View File

@ -127,11 +127,10 @@ public class MetaDataIndexUpgradeService extends AbstractComponent {
SimilarityService similarityService = new SimilarityService(indexSettings, Collections.emptyMap()); SimilarityService similarityService = new SimilarityService(indexSettings, Collections.emptyMap());
try (AnalysisService analysisService = new FakeAnalysisService(indexSettings)) { try (AnalysisService analysisService = new FakeAnalysisService(indexSettings)) {
try (MapperService mapperService = new MapperService(indexSettings, analysisService, similarityService, mapperRegistry, () -> null)) { MapperService mapperService = new MapperService(indexSettings, analysisService, similarityService, mapperRegistry, () -> null);
for (ObjectCursor<MappingMetaData> cursor : indexMetaData.getMappings().values()) { for (ObjectCursor<MappingMetaData> cursor : indexMetaData.getMappings().values()) {
MappingMetaData mappingMetaData = cursor.value; MappingMetaData mappingMetaData = cursor.value;
mapperService.merge(mappingMetaData.type(), mappingMetaData.source(), MapperService.MergeReason.MAPPING_RECOVERY, false); mapperService.merge(mappingMetaData.type(), mappingMetaData.source(), MapperService.MergeReason.MAPPING_RECOVERY, false);
}
} }
} }
} catch (Exception ex) { } catch (Exception ex) {

2
core/src/main/java/org/elasticsearch/index/IndexService.java
View File

@ -239,7 +239,7 @@ public final class IndexService extends AbstractIndexComponent implements IndexC
} }
} }
} finally { } finally {
IOUtils.close(bitsetFilterCache, indexCache, mapperService, indexFieldData, analysisService, refreshTask, fsyncTask, IOUtils.close(bitsetFilterCache, indexCache, indexFieldData, analysisService, refreshTask, fsyncTask,
cache().getPercolatorQueryCache()); cache().getPercolatorQueryCache());
} }
} }

4
core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View File

@ -353,10 +353,6 @@ public class DocumentMapper implements ToXContent {
return new DocumentMapper(mapperService, updated); return new DocumentMapper(mapperService, updated);
} }
public void close() {
documentParser.close();
}
@Override @Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return mapping.toXContent(builder, params); return mapping.toXContent(builder, params);

24
core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View File

@ -19,7 +19,6 @@
package org.elasticsearch.index.mapper; package org.elasticsearch.index.mapper;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
@ -28,7 +27,6 @@ import java.util.List;
import org.apache.lucene.document.Field; import org.apache.lucene.document.Field;
import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.IndexableField;
import org.apache.lucene.util.CloseableThreadLocal;
import org.elasticsearch.Version; import org.elasticsearch.Version;
import org.elasticsearch.common.Strings; import org.elasticsearch.common.Strings;
import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.joda.FormatDateTimeFormatter;
@ -38,12 +36,12 @@ import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.core.BinaryFieldMapper; import org.elasticsearch.index.mapper.core.BinaryFieldMapper;
import org.elasticsearch.index.mapper.core.BooleanFieldMapper; import org.elasticsearch.index.mapper.core.BooleanFieldMapper;
import org.elasticsearch.index.mapper.core.DateFieldMapper; import org.elasticsearch.index.mapper.core.DateFieldMapper;
import org.elasticsearch.index.mapper.core.KeywordFieldMapper;
import org.elasticsearch.index.mapper.core.KeywordFieldMapper.KeywordFieldType;
import org.elasticsearch.index.mapper.core.LegacyDateFieldMapper; import org.elasticsearch.index.mapper.core.LegacyDateFieldMapper;
import org.elasticsearch.index.mapper.core.LegacyDoubleFieldMapper; import org.elasticsearch.index.mapper.core.LegacyDoubleFieldMapper;
import org.elasticsearch.index.mapper.core.LegacyFloatFieldMapper; import org.elasticsearch.index.mapper.core.LegacyFloatFieldMapper;
import org.elasticsearch.index.mapper.core.LegacyIntegerFieldMapper; import org.elasticsearch.index.mapper.core.LegacyIntegerFieldMapper;
import org.elasticsearch.index.mapper.core.KeywordFieldMapper;
import org.elasticsearch.index.mapper.core.KeywordFieldMapper.KeywordFieldType;
import org.elasticsearch.index.mapper.core.LegacyLongFieldMapper; import org.elasticsearch.index.mapper.core.LegacyLongFieldMapper;
import org.elasticsearch.index.mapper.core.NumberFieldMapper; import org.elasticsearch.index.mapper.core.NumberFieldMapper;
import org.elasticsearch.index.mapper.core.StringFieldMapper; import org.elasticsearch.index.mapper.core.StringFieldMapper;
@ -57,14 +55,7 @@ import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper; import org.elasticsearch.index.mapper.object.RootObjectMapper;
/** A parser for documents, given mappings from a DocumentMapper */ /** A parser for documents, given mappings from a DocumentMapper */
final class DocumentParser implements Closeable { final class DocumentParser {
private CloseableThreadLocal<ParseContext.InternalParseContext> cache = new CloseableThreadLocal<ParseContext.InternalParseContext>() {
@Override
protected ParseContext.InternalParseContext initialValue() {
return new ParseContext.InternalParseContext(indexSettings.getSettings(), docMapperParser, docMapper, new ContentPath(0));
}
};
private final IndexSettings indexSettings; private final IndexSettings indexSettings;
private final DocumentMapperParser docMapperParser; private final DocumentMapperParser docMapperParser;
@ -81,7 +72,7 @@ final class DocumentParser implements Closeable {
source.type(docMapper.type()); source.type(docMapper.type());
final Mapping mapping = docMapper.mapping(); final Mapping mapping = docMapper.mapping();
final ParseContext.InternalParseContext context = cache.get(); final ParseContext.InternalParseContext context = new ParseContext.InternalParseContext(indexSettings.getSettings(), docMapperParser, docMapper, new ContentPath(0));
XContentParser parser = null; XContentParser parser = null;
try { try {
parser = parser(source); parser = parser(source);
@ -101,8 +92,6 @@ final class DocumentParser implements Closeable {
reverseOrder(context); reverseOrder(context);
ParsedDocument doc = parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers())); ParsedDocument doc = parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
// reset the context to free up memory
context.reset(null, null, null);
return doc; return doc;
} }
@ -932,9 +921,4 @@ final class DocumentParser implements Closeable {
private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper.Dynamic dynamic) { private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper.Dynamic dynamic) {
return dynamic == null ? ObjectMapper.Dynamic.TRUE : dynamic; return dynamic == null ? ObjectMapper.Dynamic.TRUE : dynamic;
} }
@Override
public void close() {
cache.close();
}
} }

9
core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View File

@ -63,7 +63,7 @@ import static org.elasticsearch.common.collect.MapBuilder.newMapBuilder;
/** /**
* *
*/ */
public class MapperService extends AbstractIndexComponent implements Closeable { public class MapperService extends AbstractIndexComponent {
/** /**
* The reason why a mapping is being merged. * The reason why a mapping is being merged.
@ -166,13 +166,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
} }
} }
@Override
public void close() {
for (DocumentMapper documentMapper : mappers.values()) {
documentMapper.close();
}
}
public boolean hasNested() { public boolean hasNested() {
return this.hasNested; return this.hasNested;
} }

107
core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
View File

@ -19,11 +19,16 @@
package org.elasticsearch.index.mapper; package org.elasticsearch.index.mapper;
import org.elasticsearch.ExceptionsHelper; import java.io.IOException;
import org.elasticsearch.Version; import java.io.UncheckedIOException;
import org.elasticsearch.cluster.metadata.IndexMetaData; import java.util.Arrays;
import org.elasticsearch.common.compress.CompressedXContent; import java.util.Collections;
import java.util.HashSet;
import java.util.concurrent.ExecutionException;
import java.util.function.Function;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexService;
@ -31,40 +36,23 @@ import org.elasticsearch.index.mapper.MapperService.MergeReason;
import org.elasticsearch.index.mapper.core.KeywordFieldMapper.KeywordFieldType; import org.elasticsearch.index.mapper.core.KeywordFieldMapper.KeywordFieldType;
import org.elasticsearch.index.mapper.core.NumberFieldMapper.NumberFieldType; import org.elasticsearch.index.mapper.core.NumberFieldMapper.NumberFieldType;
import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.Rule;
import org.junit.rules.ExpectedException;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.function.Function;
import java.util.concurrent.ExecutionException;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.instanceOf;
public class MapperServiceTests extends ESSingleNodeTestCase { public class MapperServiceTests extends ESSingleNodeTestCase {
@Rule
public ExpectedException expectedException = ExpectedException.none();
public void testTypeNameStartsWithIllegalDot() { public void testTypeNameStartsWithIllegalDot() {
expectedException.expect(MapperParsingException.class);
expectedException.expect(hasToString(containsString("mapping type name [.test-type] must not start with a '.'")));
String index = "test-index"; String index = "test-index";
String type = ".test-type"; String type = ".test-type";
String field = "field"; String field = "field";
client() MapperParsingException e = expectThrows(MapperParsingException.class, () -> {
.admin() client().admin().indices().prepareCreate(index)
.indices() .addMapping(type, field, "type=text")
.prepareCreate(index) .execute().actionGet();
.addMapping(type, field, "type=text") });
.execute() assertTrue(e.getMessage(), e.getMessage().contains("mapping type name [.test-type] must not start with a '.'"));
.actionGet();
} }
public void testTypeNameTooLong() { public void testTypeNameTooLong() {
@ -72,15 +60,12 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
String field = "field"; String field = "field";
String type = new String(new char[256]).replace("\0", "a"); String type = new String(new char[256]).replace("\0", "a");
expectedException.expect(MapperParsingException.class); MapperParsingException e = expectThrows(MapperParsingException.class, () -> {
expectedException.expect(hasToString(containsString("mapping type name [" + type + "] is too long; limit is length 255 but was [256]"))); client().admin().indices().prepareCreate(index)
client() .addMapping(type, field, "type=text")
.admin() .execute().actionGet();
.indices() });
.prepareCreate(index) assertTrue(e.getMessage(), e.getMessage().contains("mapping type name [" + type + "] is too long; limit is length 255 but was [256]"));
.addMapping(type, field, "type=text")
.execute()
.actionGet();
} }
public void testTypes() throws Exception { public void testTypes() throws Exception {
@ -103,36 +88,26 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
public void testIndexIntoDefaultMapping() throws Throwable { public void testIndexIntoDefaultMapping() throws Throwable {
// 1. test implicit index creation // 1. test implicit index creation
try { ExecutionException e = expectThrows(ExecutionException.class, () -> {
client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "1").setSource("{").execute().get(); client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "1").setSource("{}").execute().get();
fail(); });
} catch (Throwable t) { Throwable throwable = ExceptionsHelper.unwrapCause(e.getCause());
if (t instanceof ExecutionException) { if (throwable instanceof IllegalArgumentException) {
t = t.getCause(); assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage());
} } else {
final Throwable throwable = ExceptionsHelper.unwrapCause(t); throw e;
if (throwable instanceof IllegalArgumentException) {
assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage());
} else {
throw t;
}
} }
// 2. already existing index // 2. already existing index
IndexService indexService = createIndex("index2"); IndexService indexService = createIndex("index2");
try { expectThrows(ExecutionException.class, () -> {
client().prepareIndex("index2", MapperService.DEFAULT_MAPPING, "2").setSource().execute().get(); client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "2").setSource().execute().get();
fail(); });
} catch (Throwable t) { throwable = ExceptionsHelper.unwrapCause(e.getCause());
if (t instanceof ExecutionException) { if (throwable instanceof IllegalArgumentException) {
t = t.getCause(); assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage());
} } else {
final Throwable throwable = ExceptionsHelper.unwrapCause(t); throw e;
if (throwable instanceof IllegalArgumentException) {
assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage());
} else {
throw t;
}
} }
assertFalse(indexService.mapperService().hasMapping(MapperService.DEFAULT_MAPPING)); assertFalse(indexService.mapperService().hasMapping(MapperService.DEFAULT_MAPPING));
} }
@ -149,13 +124,11 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
}; };
createIndex("test1").mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE, false); createIndex("test1").mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE, false);
//set total number of fields to 1 to trigger an exception //set total number of fields to 1 to trigger an exception
try { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
createIndex("test2", Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 1).build()) createIndex("test2", Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 1).build())
.mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE, false); .mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE, false);
fail("Expected IllegalArgumentException"); });
} catch (IllegalArgumentException e) { assertTrue(e.getMessage(), e.getMessage().contains("Limit of total fields [1] in index [test2] has been exceeded"));
assertThat(e.getMessage(), containsString("Limit of total fields [1] in index [test2] has been exceeded"));
}
} }
public void testMappingDepthExceedsLimit() throws Throwable { public void testMappingDepthExceedsLimit() throws Throwable {