Speed up synthetic source again#89600
Merged
nik9000 merged 2 commits intoelastic:mainfrom Aug 26, 2022
Merged
Conversation
Member
Author
|
Flipping to draft while I find a unit test I can add. |
When I added support for stored fields to synthetic _source (elastic#87480) I accidentally caused a performance regression. Our friends working on building the nightly charts for tsdb caught it. It looked like: ``` | 50th percentile latency | default_1k | 20.1228 | 41.289 | 21.1662 | ms | +105.18% | | 90th percentile latency | default_1k | 26.7402 | 42.5878 | 15.8476 | ms | +59.27% | | 99th percentile latency | default_1k | 37.0881 | 45.586 | 8.49786 | ms | +22.91% | | 99.9th percentile latency | default_1k | 43.7346 | 48.222 | 4.48742 | ms | +10.26% | | 100th percentile latency | default_1k | 46.057 | 56.8676 | 10.8106 | ms | +23.47% | ``` This fixes the regression and puts us in line with how we were: ``` | 50th percentile latency | default_1k | 20.1228 | 24.023 | 3.90022 | ms | +19.38% | | 90th percentile latency | default_1k | 26.7402 | 29.7841 | 3.04392 | ms | +11.38% | | 99th percentile latency | default_1k | 37.0881 | 36.8038 | -0.28428 | ms | -0.77% | | 99.9th percentile latency | default_1k | 43.7346 | 39.0192 | -4.71531 | ms | -10.78% | | 100th percentile latency | default_1k | 46.057 | 42.9181 | -3.13889 | ms | -6.82% | ``` A 20% bump in the 50% latency isn't great, but it four microseconds per document which is acceptable.
Member
Author
|
Oh! I found some fun bugs! |
Member
Author
|
Yeah! There's a problem with empty values. But I think I can work it out. Well, I think I have. But running more tests. |
Member
Author
|
I pushed an update that fixes the tests and I think that fixes it an the perf is still ok: |
Collaborator
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
romseygeek
approved these changes
Aug 26, 2022
Member
Author
|
Thanks @romseygeek ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When I added support for stored fields to synthetic _source (#87480) I
accidentally caused a performance regression. Our friends working on
building the nightly charts for tsdb caught it. It looked like:
on my laptop that recreates to something like:
This fixes the regression and puts us in line with how we were:
A 20% bump in the 50% latency isn't great, but it four microseconds per
document which is acceptable.