[timeseries] Adding timeUsedMs to query statistics#17676
[timeseries] Adding timeUsedMs to query statistics#17676shauryachats wants to merge 3 commits intoapache:masterfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17676 +/- ##
============================================
- Coverage 63.25% 63.19% -0.06%
- Complexity 1499 1500 +1
============================================
Files 3174 3179 +5
Lines 190373 190723 +350
Branches 29089 29154 +65
============================================
+ Hits 120417 120525 +108
- Misses 60619 60829 +210
- Partials 9337 9369 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shauryachats
force-pushed
the
ts_timeused
branch
from
34df89a to
ae9a3a8
Compare
shauryachats
added
timeseries-engine
| timeSeriesBlock = _queryDispatcher.submitAndGet(requestContext.getRequestId(), dispatchablePlan, | ||
| timeSeriesRequest.getTimeout().toMillis(), requestContext); | ||
|
|
||
| long endToEndTimeMs = System.currentTimeMillis() - queryStartTime; |
There was a problem hiding this comment.
We should return time for every case. There are some if conditions like on L161 which will miss this.
There was a problem hiding this comment.
Moved it to the finally block. The error cases would still have zero time used since we only throw exceptions.
|
|
||
| for (DataTable.MetadataKey statKey : MIN_STATS_KEYS) { | ||
| String key = statKey.getName(); | ||
| String existingValue = aggregatedStats.getOrDefault(key, "0"); |
There was a problem hiding this comment.
Doesn't this mean that the result will always be 0?
There was a problem hiding this comment.
Good catch! Updated to Long.MAX_VALUE.
3 participants
Summary
The end to end time used for TSE query cannot be directly derived from the underlying v1 query execution, and hence this PR adds that to the query statistics. Also adds a bunch of other statistics.
Testing
Successfully added an integration test assertion for
timeUsedMsand verified thattimeUseMsis being populated in Quickstart UI.