Skip to content

Commit 499bef6

Browse files
committed
Fix PerFieldFormatSupplier bug
Correctly use seqno_field_use_tsdb_doc_values_format feature flag to ensure _seq_no doesn't get excluded.
1 parent ebb94bd commit 499bef6

File tree

3 files changed

+49
-14
lines changed

3 files changed

+49
-14
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,12 +531,6 @@ tests:
531531
- class: org.elasticsearch.xpack.ml.integration.InferenceIT
532532
method: testInferClassificationModel
533533
issue: https://github.com/elastic/elasticsearch/issues/133448
534-
- class: org.elasticsearch.xpack.logsdb.LogsIndexingIT
535-
method: testRouteOnSortFields
536-
issue: https://github.com/elastic/elasticsearch/issues/133993
537-
- class: org.elasticsearch.xpack.logsdb.LogsIndexingIT
538-
method: testShrink
539-
issue: https://github.com/elastic/elasticsearch/issues/133875
540534
- class: org.elasticsearch.xpack.esql.action.CrossClusterQueryWithPartialResultsIT
541535
method: testPartialResults
542536
issue: https://github.com/elastic/elasticsearch/issues/131481

server/src/main/java/org/elasticsearch/index/codec/PerFieldFormatSupplier.java

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,35 @@
2727
import org.elasticsearch.index.mapper.Mapper;
2828
import org.elasticsearch.index.mapper.MapperService;
2929
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
30+
import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper;
31+
import org.elasticsearch.index.mapper.TimeSeriesRoutingHashFieldMapper;
3032
import org.elasticsearch.index.mapper.vectors.DenseVectorFieldMapper;
3133

34+
import java.util.Collections;
35+
import java.util.HashSet;
36+
import java.util.Set;
37+
3238
/**
3339
* Class that encapsulates the logic of figuring out the most appropriate file format for a given field, across postings, doc values and
3440
* vectors.
3541
*/
3642
public class PerFieldFormatSupplier {
3743

38-
private static final FeatureFlag SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT = new FeatureFlag("seqno_field_use_tsdb_doc_values_format");
44+
static final FeatureFlag SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT = new FeatureFlag("seqno_field_use_tsdb_doc_values_format");
45+
private static final Set<String> INCLUDE_META_FIELDS;
46+
47+
static {
48+
// TODO: should we just allow all fields to use tsdb doc values codec?
49+
// Avoid using tsdb codec for fields like _seq_no, _primary_term.
50+
// But _tsid and _ts_routing_hash should always use the tsdb codec.
51+
Set<String> includeMetaField = new HashSet<>(3);
52+
includeMetaField.add(TimeSeriesIdFieldMapper.NAME);
53+
includeMetaField.add(TimeSeriesRoutingHashFieldMapper.NAME);
54+
if (SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT.isEnabled()) {
55+
includeMetaField.add(SeqNoFieldMapper.NAME);
56+
}
57+
INCLUDE_META_FIELDS = Collections.unmodifiableSet(includeMetaField);
58+
}
3959

4060
private static final DocValuesFormat docValuesFormat = new Lucene90DocValuesFormat();
4161
private static final KnnVectorsFormat knnVectorsFormat = new Lucene99HnswVectorsFormat();
@@ -126,13 +146,7 @@ boolean useTSDBDocValuesFormat(final String field) {
126146
}
127147

128148
private boolean excludeFields(String fieldName) {
129-
// TODO: should we just allow all fields to use tsdb doc values codec?
130-
// Avoid using tsdb codec for fields like _seq_no, _primary_term.
131-
// But _tsid and _ts_routing_hash should always use the tsdb codec.
132-
return fieldName.startsWith("_")
133-
&& fieldName.equals("_tsid") == false
134-
&& fieldName.equals("_ts_routing_hash") == false
135-
&& (SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT.isEnabled() && fieldName.equals(SeqNoFieldMapper.NAME) == false);
149+
return fieldName.startsWith("_") && INCLUDE_META_FIELDS.contains(fieldName) == false;
136150
}
137151

138152
private boolean isTimeSeriesModeIndex() {

server/src/test/java/org/elasticsearch/index/codec/PerFieldMapperCodecTests.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
import org.elasticsearch.index.codec.bloomfilter.ES87BloomFilterPostingsFormat;
2222
import org.elasticsearch.index.codec.postings.ES812PostingsFormat;
2323
import org.elasticsearch.index.mapper.MapperService;
24+
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
25+
import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper;
26+
import org.elasticsearch.index.mapper.TimeSeriesRoutingHashFieldMapper;
2427
import org.elasticsearch.test.ESTestCase;
2528

2629
import java.io.IOException;
@@ -201,6 +204,30 @@ public void testLogsIndexMode() throws IOException {
201204
assertThat((perFieldMapperCodec.useTSDBDocValuesFormat("response_size")), is(true));
202205
}
203206

207+
public void testMetaFields() throws IOException {
208+
PerFieldFormatSupplier perFieldMapperCodec = createFormatSupplier(true, IndexMode.LOGSDB, MAPPING_3);
209+
assertThat((perFieldMapperCodec.useTSDBDocValuesFormat(TimeSeriesIdFieldMapper.NAME)), is(true));
210+
assertThat((perFieldMapperCodec.useTSDBDocValuesFormat(TimeSeriesRoutingHashFieldMapper.NAME)), is(true));
211+
}
212+
213+
public void testSeqnoField() throws IOException {
214+
assumeTrue(
215+
"seqno_field_use_tsdb_doc_values_format should be enabled",
216+
PerFieldFormatSupplier.SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT.isEnabled()
217+
);
218+
PerFieldFormatSupplier perFieldMapperCodec = createFormatSupplier(true, IndexMode.LOGSDB, MAPPING_3);
219+
assertThat((perFieldMapperCodec.useTSDBDocValuesFormat(SeqNoFieldMapper.NAME)), is(true));
220+
}
221+
222+
public void testSeqnoFieldFeatureFlagDisabled() throws IOException {
223+
assumeTrue(
224+
"seqno_field_use_tsdb_doc_values_format should be disabled",
225+
PerFieldFormatSupplier.SEQNO_FIELD_USE_TSDB_DOC_VALUES_FORMAT.isEnabled() == false
226+
);
227+
PerFieldFormatSupplier perFieldMapperCodec = createFormatSupplier(true, IndexMode.LOGSDB, MAPPING_3);
228+
assertThat((perFieldMapperCodec.useTSDBDocValuesFormat(SeqNoFieldMapper.NAME)), is(false));
229+
}
230+
204231
private PerFieldFormatSupplier createFormatSupplier(boolean enableES87TSDBCodec, IndexMode mode, String mapping) throws IOException {
205232
Settings.Builder settings = Settings.builder();
206233
settings.put(IndexSettings.MODE.getKey(), mode);

0 commit comments

Comments
 (0)