Skip to content

Conversation

@romseygeek
Copy link
Contributor

The @timestamp field in logsdb no longer stores data in a points index,
so we need to retrieve the max timestamp field form the docvalues skipper
instead.

Fixes #137208

@romseygeek romseygeek self-assigned this Nov 17, 2025
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.3.0 labels Nov 17, 2025
@romseygeek romseygeek added >non-issue :StorageEngine/Logs You know, for Logs and removed needs:triage Requires assignment of a team area label labels Nov 17, 2025
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Should we also update ReadOnlyEngine#getRawFieldRange(...) to use doc value skippers?

@romseygeek
Copy link
Contributor Author

Should we also update ReadOnlyEngine#getRawFieldRange(...) to use doc value skippers?

Yes, but I want to do that separately as testing it will be a bit more involved.

: new DataStreamOptions.Template(DataStreamFailureStore.builder().enabled(true).buildTemplate());
Template idxTemplate = new Template(null, new CompressedXContent("""
Settings.Builder settingsBuilder = Settings.builder();
if (randomBoolean()) {
Copy link
Member

Choose a reason for hiding this comment

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

Up to you, but I'd prefer to have two tests, one for logsdb and one not. I subscribe to the view (there was a discussion on Slack about this) that randomization in tests should be used for stuff you don't care about, whereas in this case we actually care about both cases, and it'd be better if we definitely found out right away if we broke it rather than only having a 50/50 chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, I'll update.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@romseygeek romseygeek merged commit e1167ff into elastic:main Nov 17, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Datastream stats return 0 as maximum_timestamp

4 participants