Skip to content

[query] Add determinism to Graphite sorting and reduce functions - #3164

Merged
robskillington merged 2 commits into
masterfrom
r/fix-graphite-limit-lowest-not-ordered
Feb 4, 2021
Merged

[query] Add determinism to Graphite sorting and reduce functions#3164
robskillington merged 2 commits into
masterfrom
r/fix-graphite-limit-lowest-not-ordered

Conversation

@robskillington

@robskillington robskillington commented Feb 3, 2021

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

Fixes the Graphite sorting and reduce functions to be deterministic if there is multiple ranked series at the same sort order. This is done by using sort.Stable and if there was no incoming sort order into the function then applying sort by name to have determinism on which sort.Stable will pick as results.

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

NONE

Does this PR require updating code package or user-facing documentation?:

NONE

@codecov

codecov Bot commented Feb 3, 2021

Copy link
Copy Markdown

Codecov Report

Merging #3164 (f422699) into master (a7a0580) will decrease coverage by 32.2%.
The diff coverage is 39.4%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3164      +/-   ##
=========================================
- Coverage    72.4%   40.1%   -32.3%     
=========================================
  Files        1085     993      -92     
  Lines      101127   92688    -8439     
=========================================
- Hits        73233   37190   -36043     
- Misses      22839   51901   +29062     
+ Partials     5055    3597    -1458     
Flag Coverage Δ
aggregator 52.4% <ø> (-23.5%) ⬇️
cluster 36.2% <ø> (-48.6%) ⬇️
collector 73.3% <ø> (-11.0%) ⬇️
dbnode 43.2% <ø> (-35.7%) ⬇️
m3em 50.2% <ø> (-24.2%) ⬇️
m3ninx 49.5% <ø> (-23.7%) ⬇️
metrics ?
msg 73.2% <ø> (-0.9%) ⬇️
query 22.0% <39.4%> (-45.2%) ⬇️
x 46.8% <ø> (-33.8%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7a0580...f422699. Read the comment docs.

@arnikola arnikola left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@robskillington
robskillington merged commit 985f866 into master Feb 4, 2021
@robskillington
robskillington deleted the r/fix-graphite-limit-lowest-not-ordered branch February 4, 2021 03:47
soundvibe pushed a commit that referenced this pull request Feb 4, 2021
* master:
  [query] Add determinism to Graphite sorting and reduce functions (#3164)
  [docs] Update footer (#3165)
  Return an Iterator from ReadEncoded (#3147)
  [changelog] Add changelog for 1.1.0 (#3160)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants