Memory store limit queries return most recent traces #1394
Conversation
jacobmarble
requested review from
black-adder,
jpkrohling,
objectiser,
pavolloffay,
tiffon,
vprithvi and
yurishkuro
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1394 +/- ##
==========================================
+ Coverage 98.79% 98.79% +<.01%
==========================================
Files 189 189
Lines 8986 8989 +3
==========================================
+ Hits 8878 8881 +3
Misses 84 84
Partials 24 24
Continue to review full report at Codecov.
|
ghost
assigned 
Fixes #1393 Signed-off-by: Jacob Marble <[email protected]>
|
@yurishkuro I think this is a fairly simple PR. Do you mind taking a look? |
|
@pavolloffay @objectiser thoughts on this PR? |
|
@yurishkuro I'm in favour of having stable results - even though in-memory should only be used for demo/test purposes, it avoids unnecessary questions about why the results are not consistent. |
|
Stable results are good, my main concern was with performance implications, since the complete data set is now being scanned. I supposed we can punt on the performance aspect of this memory storage, if people are really serious about performance, they may want to use Badger (which I hope to merge next week). |
|
Agree -- I wouldn't stress too much about the performance of this as this is only about the in-memory storage. |
|
@jacobmarble it seems that this merge failed,"This branch is out-of-data with the base branch". so the latest “jaegertracing/all-in-one” is still out of order |
|
it looks like there was a race condition in one of the tests |
|
the results are in order now .thank you @jacobmarble @yurishkuro |
Fixes #1393
Which problem is this PR solving?
Memory store query results are not stable.
Short description of the changes
Sorts memory store query results before limit.