Enable skippers on @timestamp and host.name fields in LogsDB

[romseygeek]

This was enabled and hidden behind a feature flag a while ago, but then disabled when
performance suffered. Recent benchmarks show that performance is likely to be back
on par with changes made in lucene in the intervening time.

This commit re-enables the skippers for the @timestamp and host.name fields in
logsdb, keeping everything gated behind the feature flag.

[elasticsearchmachine]

Pinging @elastic/es-storage-engine (Team:StorageEngine)

[elasticsearchmachine]

Hi @romseygeek, I've created a changelog YAML for you.

[martijnvg]

Thanks Alan, LGTM!

I think doc value skippers will now also be enabled for _tsid field? (in snapshot versions) (If I understand TimeSeriesIdFieldMapper#getInstance(...) correctly)

[martijnvg]

Let's label this as a non-issue? Because released versions will not run with doc value skippers enabled And when the feature flag gets removed then we label it as a feature or enhancement.

[romseygeek]

I think doc value skippers will now also be enabled for _tsid field? (in snapshot versions) (If I understand TimeSeriesIdFieldMapper#getInstance(...) correctly)

Yes, that looks right. It doesn't remove the backing index for this field though, it only enables the skipper.

[martijnvg]

Yes, that looks right. It doesn't remove the backing index for this field though, it only enables the skipper.

Yes, the _tsid meta field was never configured with an inverted index.

[romseygeek]

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

[martijnvg]

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

[romseygeek]

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}}

[martijnvg]

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?

[martijnvg]

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?

[martijnvg]

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.

[romseygeek]

Buildkite benchmark this with tsdb please

[romseygeek]

Buildkite benchmark this with tsdb please

[gbanasiak]

@romseygeek As both baseline and contender failed in https://buildkite.com/elastic/elasticsearch-pull-request-performance-benchmark/builds/33, and also nightly failed for tsdb benchmark, it seems a bug was introduced in ES, unrelated to this PR. Have we identified and fixed it?

[romseygeek]

@gbanasiak thanks, I was checking in the wrong place for errors and I thought it was a transient but it looks as though there's a problem with aggregate metric doubles. I'll dig further.

[romseygeek]

Buildkite benchmark this with tsdb please

[romseygeek]

I am confused... I kicked off that last benchmark at 11:33, but following the build #34 link takes me to a failed build that started at 9:15, before we reverted the changes to the TSDB track that were causing the failures. @gbanasiak are the links wrong or did something get cached and the broken build get re-used?

[romseygeek]

Buildkite benchmark this with tsdb please

[romseygeek]

Trying a benchmark again after I've merged in main...

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 7f64beb
• Baseline: 434cb5a (env ID 28376feb-2613-4fe8-a38b-9139d8eaab40)
• Contender: 7f64beb (env ID e1b9ab9a-bf51-487c-812a-c4b96de2ed4e)
• Benchmark results

This build ran two tsdb benchmarks to evaluate performance impact of this PR.

History

• 💚 Build #35 succeeded 6c05d0f
• 💚 Build #34 succeeded 6c05d0f
• 💔 Build #33 failed 6c05d0f

cc @romseygeek