Fresh results #34
Fresh results #34
Conversation
| traces.add(spans); | ||
| long from = (request.endTs() - request.lookback()) * 1000; | ||
| long to = request.endTs() * 1000; | ||
| long checkpoint = to - (30 * 1000 * 1000); // 30 sec before upper bound |
There was a problem hiding this comment.
this checkpoint represent the bucket range
| LOG.debug("Traces found from query {}: {}", queryRequest, traces.size()); | ||
| return traces; | ||
| } | ||
| traces.sort(Comparator.<List<Span>>comparingLong(o -> o.get(0).timestampAsLong()).reversed()); |
There was a problem hiding this comment.
Resulting traces list should have values close to limit, so sorting should not be expensive.
| // double check service names are unique | ||
| if (!serviceNames.contains(keyValue.value)) serviceNames.add(keyValue.value); | ||
| }); | ||
| try (KeyValueIterator<String, String> all = serviceStore.all()) { |
There was a problem hiding this comment.
sheesh do we allow dupes? sounds like we should tighten up that javadoc
There was a problem hiding this comment.
yeap, that surprise me on one of my deployments where I started to get duplicate service names. Will create an issue to double check this.
| }); | ||
| } | ||
| } else { | ||
| while (checkpoint > from && traces.size() < request.limit()) { |
There was a problem hiding this comment.
cc @llinder @michaelsembwever this seems familiar though in cassandra I think we try to do async lazy chained calls.. not sure if the traceIdsByTsStore.range here is a remote call or not.
There was a problem hiding this comment.
All stores are local tho
There was a problem hiding this comment.
Unless we'd support openzipkin/zipkin#2784 where calls could span multiple instances
| } | ||
| return traces.size() == 1 | ||
| && traces.get(0).size() == 1; // last trace is returned first | ||
| }); | ||
| await().atMost(5, TimeUnit.SECONDS) |
There was a problem hiding this comment.
side question.. why all the waits? I know in some cases you can't get a callback of storage quorum met. in such case you could add a sleep to a command that wraps storage to declutter the IT's main code..
There was a problem hiding this comment.
Good question, probably copy-pasta heh. I think only first one is needed, will clean up.
|
@adriancole , I'd love to move it to oz-contrib! |
Fix #33
This PR includes an attempt to solve the "return oldest" issue by creating a bucketed range query procedure and validate on every bucket if we have enough traces to return a result, instead of testing every trace on request range, or return first results.