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

Fix ordering of indexScanKeys after TraceID parsing, closes #1808 #1809

Merged
merged 3 commits into from
Oct 9, 2019

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Sep 23, 2019

Which problem is this PR solving?

Bug #1808 indexSeeksToTraceIDs returns keys in wrong order after parsing the TraceID, while durationQueries returned them in correct order.

Short description of the changes

Add slice sorting after TraceID parsing to ensure the order is correct. Also add read-order test for this method.

@pavolloffay pavolloffay added the storage/badger Issues related to badger storage label Sep 23, 2019
@burmanm burmanm force-pushed the badger_bug_1808 branch 2 times, most recently from 80c5768 to 02b3cb7 Compare September 23, 2019 14:37
@codecov
Copy link

codecov bot commented Sep 23, 2019

Codecov Report

Merging #1809 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1809      +/-   ##
==========================================
+ Coverage   98.21%   98.21%   +<.01%     
==========================================
  Files         195      195              
  Lines        9602     9606       +4     
==========================================
+ Hits         9431     9435       +4     
  Misses        134      134              
  Partials       37       37
Impacted Files Coverage Δ
plugin/storage/badger/spanstore/reader.go 96.75% <100%> (+0.03%) ⬆️
cmd/agent/app/builder.go 100% <0%> (ø) ⬆️
cmd/ingester/app/consumer/processor_factory.go 100% <0%> (ø) ⬆️

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 7d339ef...86d7b60. Read the comment docs.

Signed-off-by: Michael Burman <yak@iki.fi>
Signed-off-by: Michael Burman <yak@iki.fi>
@objectiser
Copy link
Contributor

@yurishkuro Think your comment has been addressed - is it ok to merge?

@yurishkuro yurishkuro merged commit b217e1b into jaegertracing:master Oct 9, 2019
radekg pushed a commit to Klarrio/jaeger that referenced this pull request Oct 28, 2019
…cing#1808 (jaegertracing#1809)

* Fix ordering of indexScanKeys after TraceID parsing, closes jaegertracing#1808

Signed-off-by: Michael Burman <yak@iki.fi>

* Address comments

Signed-off-by: Michael Burman <yak@iki.fi>

* ..

Signed-off-by: Michael Burman <yak@iki.fi>
Signed-off-by: radekg <radek@gruchalski.com>
@yurishkuro yurishkuro added this to the Release 1.15 milestone Nov 7, 2019
backjo pushed a commit to backjo/jaeger that referenced this pull request Dec 19, 2019
…cing#1808 (jaegertracing#1809)

* Fix ordering of indexScanKeys after TraceID parsing, closes jaegertracing#1808

Signed-off-by: Michael Burman <yak@iki.fi>

* Address comments

Signed-off-by: Michael Burman <yak@iki.fi>

* ..

Signed-off-by: Michael Burman <yak@iki.fi>
Signed-off-by: Jonah Back <jonah@jonahback.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
storage/badger Issues related to badger storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants