-
Notifications
You must be signed in to change notification settings - Fork 24.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better storage of _source
#9034
Comments
As you say, arrays complicate things. One of the problems we with the the |
@jpountz just reread your original description and realised that your first suggestion handles the issue of representing things like I like this idea a lot. Once concern: is there extra overhead if we have thousands of small top-level fields, eg 10,000 ints? If so, could we possibly group these small fields? |
Wasn't the issue with storing everything in different fields that the extra lookups were time consuming? This wouldn't help there unless you used it for field that are large, kind like PostgreSQL's toast mechanism. BTW - I've always thought it'd be nice to be able to load portions of string fields. Something like |
With Lucene 4 and above, this is no longer the case - having 1 large field vs several small fields no longer matters. |
Ah cool. I suspect that instinct is left over from Elasticsearch 0.90 days. Cheers. |
cc @elastic/es-search-aggs |
I wanted to check how much it would save so I played with the following patch, which stores every top-level json field in its own stored field as described in the issue description: diff --git a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java
index a3e86ab..f496275 100644
--- a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java
+++ b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java
@@ -236,9 +236,7 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
return null;
}
final long version = parallelArray.version[docIndex];
- final String sourceField = parallelArray.hasRecoverySource[docIndex] ? SourceFieldMapper.RECOVERY_SOURCE_NAME :
- SourceFieldMapper.NAME;
- final FieldsVisitor fields = new FieldsVisitor(true, sourceField);
+ final FieldsVisitor fields = new FieldsVisitor(true, parallelArray.hasRecoverySource[docIndex]);
leaf.reader().document(segmentDocID, fields);
fields.postProcess(mapperService);
diff --git a/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java b/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java
index 462f8ce..a0eeb69 100644
--- a/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java
+++ b/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java
@@ -49,30 +54,49 @@ import static org.elasticsearch.common.util.set.Sets.newHashSet;
* Base {@link StoredFieldVisitor} that retrieves all non-redundant metadata.
*/
public class FieldsVisitor extends StoredFieldVisitor {
+
private static final Set<String> BASE_REQUIRED_FIELDS = unmodifiableSet(newHashSet(
IdFieldMapper.NAME,
RoutingFieldMapper.NAME));
private final boolean loadSource;
- private final String sourceFieldName;
+ private final boolean useRecoverySource;
private final Set<String> requiredFields;
- protected BytesReference source;
protected String type, id;
protected Map<String, List<Object>> fieldsValues;
+ private BytesStreamOutput sourceBytes;
+ private XContentGenerator sourceGenerator;
+ protected BytesReference source;
+
public FieldsVisitor(boolean loadSource) {
- this(loadSource, SourceFieldMapper.NAME);
+ this(loadSource, false);
}
- public FieldsVisitor(boolean loadSource, String sourceFieldName) {
+ public FieldsVisitor(boolean loadSource, boolean useRecoverySource) {
this.loadSource = loadSource;
- this.sourceFieldName = sourceFieldName;
+ this.useRecoverySource = useRecoverySource;
requiredFields = new HashSet<>();
reset();
}
+ private XContentGenerator getSourceGenerator() throws IOException {
+ if (sourceGenerator == null) {
+ sourceBytes = new BytesStreamOutput();
+ sourceGenerator = JsonXContent.jsonXContent.createGenerator(sourceBytes);
+ sourceGenerator.writeStartObject();
+ }
+ return sourceGenerator;
+ }
+
@Override
public Status needsField(FieldInfo fieldInfo) throws IOException {
+ if (fieldInfo.name.equals(SourceFieldMapper.NAME) || fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX)) {
+ return loadSource && useRecoverySource == false ? Status.YES : Status.NO;
+ } else if (fieldInfo.name.equals(SourceFieldMapper.RECOVERY_SOURCE_NAME)) {
+ return loadSource && useRecoverySource ? Status.YES : Status.NO;
+ }
+
if (requiredFields.remove(fieldInfo.name)) {
return Status.YES;
}
@@ -94,6 +118,11 @@ public class FieldsVisitor extends StoredFieldVisitor {
if (mapper != null) {
type = mapper.type();
}
+ if (loadSource && source == null && sourceGenerator == null &&
+ mapper.metadataMapper(SourceFieldMapper.class).enabled()) {
+ // can happen if the source is split and the document has no fields
+ source = new BytesArray("{}");
+ }
for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
MappedFieldType fieldType = mapperService.fullName(entry.getKey());
if (fieldType == null) {
@@ -109,8 +138,12 @@ public class FieldsVisitor extends StoredFieldVisitor {
@Override
public void binaryField(FieldInfo fieldInfo, byte[] value) throws IOException {
- if (sourceFieldName.equals(fieldInfo.name)) {
+ if (SourceFieldMapper.RECOVERY_SOURCE_NAME.equals(fieldInfo.name)
+ || SourceFieldMapper.NAME.equals(fieldInfo.name)) {
+ } else if (fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX)) {
+ String fieldName = fieldInfo.name.substring(SourceFieldMapper.NAME_PREFIX.length());
+ getSourceGenerator().writeRawField(fieldName, new ByteArrayInputStream(value), XContentType.JSON);
} else if (IdFieldMapper.NAME.equals(fieldInfo.name)) {
id = Uid.decodeId(value);
} else {
@@ -120,31 +153,58 @@ public class FieldsVisitor extends StoredFieldVisitor {
@Override
public void stringField(FieldInfo fieldInfo, byte[] bytes) throws IOException {
+ assert fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX) == false;
final String value = new String(bytes, StandardCharsets.UTF_8);
addValue(fieldInfo.name, value);
}
@Override
public void intField(FieldInfo fieldInfo, int value) throws IOException {
+ assert fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX) == false;
addValue(fieldInfo.name, value);
}
@Override
public void longField(FieldInfo fieldInfo, long value) throws IOException {
+ assert fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX) == false;
addValue(fieldInfo.name, value);
}
@Override
public void floatField(FieldInfo fieldInfo, float value) throws IOException {
+ assert fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX) == false;
addValue(fieldInfo.name, value);
}
@Override
public void doubleField(FieldInfo fieldInfo, double value) throws IOException {
+ assert fieldInfo.name.startsWith(SourceFieldMapper.NAME_PREFIX) == false;
addValue(fieldInfo.name, value);
}
public BytesReference source() {
+ if (type == null) {
+ throw new IllegalStateException("Call postProcess first");
+ }
+ if (source != null && sourceGenerator != null) {
+ throw new IllegalStateException("Documents should have a single source");
+ }
+ if (sourceGenerator != null) {
+ try {
+ sourceGenerator.writeEndObject();
+ sourceGenerator.close();
+ } catch (IOException e) {
+ throw new RuntimeException("cannot happen: in-memory stream", e);
+ }
+ source = sourceBytes.bytes();
+ sourceBytes = null;
+ sourceGenerator = null;
+ }
return source;
}
@@ -180,9 +240,6 @@ public class FieldsVisitor extends StoredFieldVisitor {
id = null;
requiredFields.addAll(BASE_REQUIRED_FIELDS);
- if (loadSource) {
- requiredFields.add(sourceFieldName);
- }
}
void addValue(String name, Object value) {
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 f77fc07..b22a1e0 100644
--- a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java
+++ b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java
@@ -198,6 +198,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
} catch (IOException e) {
throw new ElasticsearchException("Failed to get type [" + type + "] and id [" + id + "]", e);
}
+ fieldVisitor.postProcess(mapperService);
source = fieldVisitor.source();
if (!fieldVisitor.fields().isEmpty()) {
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
index 0242585..62f84a1 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -50,6 +57,7 @@ import java.util.function.Function;
public class SourceFieldMapper extends MetadataFieldMapper {
public static final String NAME = "_source";
+ public static final String NAME_PREFIX = NAME + ".";
public static final String RECOVERY_SOURCE_NAME = "_recovery_source";
public static final String CONTENT_TYPE = "_source";
@@ -241,8 +249,28 @@ public class SourceFieldMapper extends MetadataFieldMapper {
builder.close();
source = bStream.bytes();
}
- BytesRef ref = source.toBytesRef();
- fields.add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
+
+ try (XContentParser sourceParser = XContentFactory.xContent(context.sourceToParse().getXContentType())
+ .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, source.streamInput())) {
+ if (sourceParser.nextToken() != Token.START_OBJECT) {
+ throw new IllegalArgumentException("Documents must start with a START_OBJECT, got " + sourceParser.currentToken());
+ }
+ while (sourceParser.nextToken() == Token.FIELD_NAME) {
+ sourceParser.nextToken();
+ String fieldName = sourceParser.currentName();
+ BytesStreamOutput os = new BytesStreamOutput();
+ try (XContentGenerator generator = JsonXContent.jsonXContent.createGenerator(os)) {
+ generator.copyCurrentStructure(sourceParser);
+ }
+ fields.add(new StoredField(NAME + "." + fieldName, os.bytes().toBytesRef()));
+ }
+ if (sourceParser.currentToken() != Token.END_OBJECT) {
+ throw new IllegalArgumentException("Documents must end with a END_OBJECT, but found a " + sourceParser.currentToken());
+ }
+ if (sourceParser.nextToken() != null) {
+ throw new IllegalArgumentException("Documents must end with a END_OBJECT, but found a " + sourceParser.currentToken() + " after the end");
+ }
+ }
} else {
source = null;
} It doesn't pass all tests but it's enough to experiment. The approach is pretty conservative: the order of fields is preserved, and values are stored exactly as they were provided in the original source, it might just drop extra spaces, line feeds or comments. I indexed the geonames dataset and ran the disk usage tool on it.
Disk usage reduction looks interesting, but I think I am even more interested in how this could help improve the simplicity and efficiency of other APIs:
Furthermore, we could further improve disk usage if the user agreed to apply the same accuracy trade-offs to the
|
@jpountz et al., I know this is a really old issue, but I'm trying to figure out why my customer's fetch phase is slow even when using source filtering or Is If so, I am assuming that when my customer asks his query to get a small field from |
@freakingid You explained it right. We have plans to improve the situation both on the ES side (as described in this issue) and the on the Lucene side. |
@mayya-sharipova I find that update performance is too bad , so I want find some ways to fix it. |
Elasticsearch needs to rebuild the entire _source for updates by design, even partial updates. If this is the bottleneck of your workload, there isn't much that can be done. |
thanks! |
I played around with the idea a bit and had more notes to add.
Separately from performance, I wonder if this strategy would sufficiently address users' concerns around _source loading. It only applies to top-level fields, so it wouldn't cover cases where the stored field was part of an object field. It adds some complexity to the mental model -- to debug an issue around source loading performance, a user should understand that we only split apart top-level fields (which are not given special status in other APIs/ operations). |
A problem with disabling
Agreed this is a tricky trade-off. Making |
I had forgotten about
I ran the metricbeat track with a single field
|
I wonder if we should close this issue in favor of synthetic source (#86603). While synthetic source is a different feature, mappings could be configured to mark every field as stored and enable synthetic source, which would give the ability to load a subset of the field without loading and parsing an entire JSON document in memory? The downside compared to the proposal on this issue is that you would lose the JSON structure, but maybe it's ok? |
I think synthetic source could be a fairly big thing that will eventually morph into covering more cases. I wouldn't be surprised if we ended up in a case more like this one day built on top of the synthetic source infrastructure. We're already talking about support for stored fields in synthetic source. It's not too much further to get here. But in terms of things were doing in the short term for source storage I think synthetic source is it. In that sense I think we can close this, yeah. |
Today we store the
_source
is a single big binary stored field. While this is great for simplicity, this also has the bad side-effect to encourage to store fields individually in order to save some json parsing when there are a couple of large field values and we are only interested in some short values. Maybe we could try to be a bit smarter and store the_source
across several stored fields so that it would not be an issue anymore?Random idea: given a document that looks like:
we could for instance store all the top-level fields into their own stored field
or maybe even each value individually (but it becomes more complicated with arrays of objects):
Then we would have to make
_source
filtering aware of the way fields are stored, and for instance if we store only top-level fields into their own stored field then we could translate an include rule likefoo.*
to "retrieve fieldfoo
", andfoo.bar.*
to "get everything underbar
for fieldfoo
".The text was updated successfully, but these errors were encountered: