Skip to content
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

Support flattened label field with downsampling. #118816

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

martijnvg
Copy link
Member

If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes #116319

If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
@martijnvg martijnvg added >bug :StorageEngine/Downsampling Downsampling (replacement for rollups) - Turn fine-grained time-based data into coarser-grained data v9.0.0 v8.17.1 v8.18.0 labels Dec 17, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

- '{"@timestamp": "2021-04-28T19:50:53.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9"}, "agent": { "id": "second", "version": "2.1.9" }, "value": 25 }}'
- '{"index": {}}'
- '{"@timestamp": "2021-04-28T19:51:03.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9"}, "agent": { "id": "second", "version": "2.1.10" }, "value": 17 }}'

Copy link
Contributor

@kkrik-es kkrik-es Dec 17, 2024

Choose a reason for hiding this comment

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

Let's add a mix of flattened contents, with dotted names, null, missing entries, single value etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 - will add more tests

…ic#118838 (will fix later)

and enabled subobjects for "A flattened field as label field" test
@martijnvg martijnvg changed the title Support flattened field with downsampling. Support flattened label field with downsampling. Dec 17, 2024
@martijnvg
Copy link
Member Author

@kkrik-es I noticed a different issue when flattened field is defined as dimensions, see #118838. So I removed the yaml test with flattened field as dimension, and I will fix that in a followup PR. This PR will then only address flattened label fields. The fix for flattened dimensions requires more changes in downsampling and I like to keep this PR small.

@martijnvg martijnvg requested a review from kkrik-es December 17, 2024 15:25
assertEquals("dummy", producer.name());
assertEquals("last_value", producer.label().name());

var bytes = List.of("a\0value_a", "b\0value_b", "c\0value_c", "d\0value_d");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get a mix of values here as well, including null and dotted names.

isEmpty = false;
List<BytesRef> values = new ArrayList<>(docValuesCount);
for (int i = 0; i < docValuesCount; i++) {
values.add(new BytesRef(docValues.nextValue().toString()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only change, compared to the overriden method? Can this be moved to write to avoid overriding both?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Maybe we complete reuse the collect(...) super method and just do a bytes ref conversation in write(...) method so that FlattenedFieldSyntheticWriterHelper can use the collected values.

Copy link
Member Author

Choose a reason for hiding this comment

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

done: 03e9d7a

@martijnvg martijnvg added the auto-backport Automatically create backport pull requests when merged label Dec 17, 2024
@martijnvg martijnvg merged commit 37807b8 into elastic:main Dec 18, 2024
16 checks passed
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Dec 18, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.17
8.x

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Dec 18, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
elasticsearchmachine pushed a commit that referenced this pull request Dec 18, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes #116319
elasticsearchmachine pushed a commit that referenced this pull request Dec 18, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes #116319
rjernst pushed a commit to rjernst/elasticsearch that referenced this pull request Dec 18, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
navarone-feekery pushed a commit to navarone-feekery/elasticsearch that referenced this pull request Dec 26, 2024
If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
sarog pushed a commit to sarog/elasticsearch that referenced this pull request Jan 22, 2025
…astic#118935)

If flattened field is configured as non-dimension and non-metric field, then downsampling fails to execute successfully. Downsampling doesn't know how to use the flattened field or how to serialize it. This change addresses this.

Closes elastic#116319
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >bug :StorageEngine/Downsampling Downsampling (replacement for rollups) - Turn fine-grained time-based data into coarser-grained data Team:StorageEngine v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Downsample functionality should handle flattened field as labels correctly.
3 participants