Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2b73b64
Enable skippers
romseygeek Aug 21, 2025
ed6a938
Merge branch 'main' into benchmark/main-enabled-skippers
romseygeek Sep 5, 2025
16b8100
Update docs/changelog/134221.yaml
romseygeek Sep 5, 2025
2d8ab3f
Delete docs/changelog/134221.yaml
romseygeek Sep 5, 2025
05c7e22
Extra load step in TimeSeriesIT
romseygeek Sep 8, 2025
451fb81
Merge remote-tracking branch 'romseygeek/benchmark/main-enabled-skipp…
romseygeek Sep 8, 2025
d3f3bd8
fix index version gating on host.name skipper
romseygeek Sep 8, 2025
f6caa0f
Index versions @timestamp gate
romseygeek Sep 9, 2025
5fe8969
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Sep 9, 2025
62d2505
... and tsid too
romseygeek Sep 10, 2025
b7dc09f
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Sep 10, 2025
3f9ad9d
[CI] Auto commit changes from spotless
Sep 10, 2025
09a195a
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Sep 24, 2025
612c1bc
Add ability to merge SortedNumericDocValuesRangeQueries
romseygeek Sep 19, 2025
b82f902
Fix versions
romseygeek Sep 24, 2025
447aa39
Merge remote-tracking branch 'romseygeek/benchmark/main-enabled-skipp…
romseygeek Sep 24, 2025
9e7e95f
[CI] Auto commit changes from spotless
Sep 24, 2025
129b84c
Rounding can use DocValuesSkipper
romseygeek Sep 25, 2025
d88afa5
Merge remote-tracking branch 'romseygeek/benchmark/main-enabled-skipp…
romseygeek Sep 25, 2025
7151761
Hack: be aware of doc values skippers in VSC.alignsWithSearchIndex()
romseygeek Sep 29, 2025
479c5b0
Use DocValuesSkippers in SearchContextStats
romseygeek Sep 30, 2025
ee9864b
[CI] Auto commit changes from spotless
Sep 30, 2025
b72a89b
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Oct 8, 2025
0e75c81
precommit
romseygeek Oct 8, 2025
9928e55
Merge remote-tracking branch 'romseygeek/benchmark/main-enabled-skipp…
romseygeek Oct 8, 2025
e49189f
version
romseygeek Oct 8, 2025
3ee948f
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Oct 9, 2025
125638e
[CI] Auto commit changes from spotless
Oct 9, 2025
ee0219d
tests
romseygeek Oct 9, 2025
a335ad3
Merge remote-tracking branch 'origin/main' into benchmark/main-enable…
romseygeek Oct 9, 2025
6c05d0f
Merge remote-tracking branch 'romseygeek/benchmark/main-enabled-skipp…
romseygeek Oct 9, 2025
7f64beb
Merge branch 'main' into benchmark/main-enabled-skippers
romseygeek Oct 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ public boolean isES87TSDBCodecEnabled() {
public static final boolean DOC_VALUES_SKIPPER = new FeatureFlag("doc_values_skipper").isEnabled();
public static final Setting<Boolean> USE_DOC_VALUES_SKIPPER = Setting.boolSetting(
"index.mapping.use_doc_values_skipper",
false,
true,
Property.IndexScope,
Property.Final
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,15 +586,16 @@ public void testProfile() {
List<DriverProfile> dataProfiles = profile.drivers().stream().filter(d -> d.description().equals("data")).toList();
assertThat(dataProfiles, hasSize(1));
List<OperatorStatus> ops = dataProfiles.get(0).operators();
assertThat(ops, hasSize(5));
assertThat(ops, hasSize(6));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds an extra ValuesSourceReaderOperator step into this ESQL query - does that sound right to you @martijnvg ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know for what field(s) both value source reader operations are?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new one is for @timestamp:
{"operator":"ValuesSourceReaderOperator[fields = [@timestamp]]","status":{"readers_built":{"@timestamp:column_at_a_time:BlockDocValuesReader.SingletonLongs":3},"values_loaded":98,"process_nanos":887208,"pages_received":3,"pages_emitted":3,"rows_received":98,"rows_emitted":98}}

The other is for _tsid, cpu and cluster and is unchanged from before:
{"operator":"ValuesSourceReaderOperator[fields = [_tsid, cpu, cluster]]","status":{"readers_built":{"_tsid:column_at_a_time:BlockDocValuesReader.SingletonOrdinals":3,"cluster:column_at_a_time:BlockDocValuesReader.SingletonOrdinals":3,"cpu:column_at_a_time:BlockDocValuesReader.SingletonDoubles":3},"values_loaded":294,"process_nanos":1226959,"pages_received":3,"pages_emitted":3,"rows_received":98,"rows_emitted":98}}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check whether the LuceneSourceOperator pushes down the buckets as filters? For example do see IndexOrDocValuesQuery(indexQuery=@timestamp:[1713139320000 TO 1713139379999], dvQuery=@timestamp:[1713139320000 TO 1713139379999]) in processedQueries of the status of LuceneSourceOperator?

Maybe this change results in compute engine no longer pushing down filters?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, the field caps api for @timestamp field will no longer return that it is indexed and that is why I think the query plan is different, with this as a result.

I don't think this blocks merging this pr. Maybe we need to think about marking a field as indexed if it has doc value skippers and is part of index sorting before we release the feature flag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked LuceneSourceOperator with this change and there aren't any IndexOrDocValuesQuery instances in processed query.

So I think this is good. Let's merge it when CI is green.

assertThat(ops.get(0).operator(), containsString("LuceneSourceOperator"));
assertThat(ops.get(0).status(), Matchers.instanceOf(LuceneSourceOperator.Status.class));
LuceneSourceOperator.Status status = (LuceneSourceOperator.Status) ops.get(0).status();
assertThat(status.processedShards().size(), Matchers.lessThanOrEqualTo(3));
assertThat(ops.get(1).operator(), containsString("EvalOperator"));
assertThat(ops.get(2).operator(), containsString("ValuesSourceReaderOperator"));
assertThat(ops.get(3).operator(), containsString("TimeSeriesAggregationOperator"));
assertThat(ops.get(4).operator(), containsString("ExchangeSinkOperator"));
assertThat(ops.get(1).operator(), containsString("ValuesSourceReaderOperator"));
assertThat(ops.get(2).operator(), containsString("EvalOperator"));
assertThat(ops.get(3).operator(), containsString("ValuesSourceReaderOperator"));
assertThat(ops.get(4).operator(), containsString("TimeSeriesAggregationOperator"));
assertThat(ops.get(5).operator(), containsString("ExchangeSinkOperator"));
}
}
}
Expand Down