Provide all stats metadata to queries run by pinot-java-client #10465
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10465 +/- ##
============================================
- Coverage 70.26% 70.20% -0.07%
+ Complexity 6112 6110 -2
============================================
Files 2067 2067
Lines 111930 111916 -14
Branches 17027 17027
============================================
- Hits 78648 78570 -78
- Misses 27756 27807 +51
- Partials 5526 5539 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 36 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| map.put(NUM_GROUPS_LIMIT_REACHED, isNumGroupsLimitReached()); | ||
| map.put(TIME_USED_MS, getTimeUsedMs() + "ms"); | ||
| return map.toString(); | ||
| return _brokerResponse.toString(); |
There was a problem hiding this comment.
Curious why we were selectively pulling out stats in the first place? Any idea what other stats we will get to see.
This is the PR that added - #5892
There was a problem hiding this comment.
Before fix:
{numEntriesScannedPostFilter=0,
numDocsScanned=10,
numServersResponded=1,
numSegmentsMatched=1,
timeUsedMs=12ms,
numConsumingSegmentsQueried=0,
totalDocs=10,
numSegmentsQueried=1,
minConsumingFreshnessTimeMs=0ms,
numGroupsLimitReached=false,
numSegmentsProcessed=1,
numEntriesScannedInFilter=0,
numServersQueried=1}
After fix:
{"exceptions":[],"numServersQueried":0,"numServersResponded":0,"numSegmentsQueried":0,"numSegmentsProcessed":0,"numSegmentsMatched":0,"numConsumingSegmentsQueried":0,"numConsumingSegmentsProcessed":0,"numConsumingSegmentsMatched":0,"numDocsScanned":0,"numEntriesScannedInFilter":0,"numEntriesScannedPostFilter":0,"numGroupsLimitReached":false,"totalDocs":0,"timeUsedMs":0,"offlineThreadCpuTimeNs":0,"realtimeThreadCpuTimeNs":0,"offlineSystemActivitiesCpuTimeNs":0,"realtimeSystemActivitiesCpuTimeNs":0,"offlineResponseSerializationCpuTimeNs":0,"realtimeResponseSerializationCpuTimeNs":0,"offlineTotalCpuTimeNs":0,"realtimeTotalCpuTimeNs":0,"segmentStatistics":[],"traceInfo":{},"numRowsResultSet":0,"minConsumingFreshnessTimeMs":0,"numSegmentsPrunedByBroker":0,"numSegmentsPrunedByServer":0,"numSegmentsPrunedInvalid":0,"numSegmentsPrunedByLimit":0,"numSegmentsPrunedByValue":0,"explainPlanNumEmptyFilterSegments":0,"explainPlanNumMatchAllFilterSegments":0}
There was a problem hiding this comment.
Good point @swaminathanmanish. The change is simple, but maybe there was a reason to do not include something? Doesn't seem a security concern given the nature of the object, but it would be great to have a verification. @KKcorps @kishoreg @npawar were the ones involved in the first PR.
There was a problem hiding this comment.
Wouldn't this also return the query results?
There was a problem hiding this comment.
Current toString method is printing selected fields out of _brokerResponse JSON, but _brokerResponse.toString will send the entire stats JSON.
Limited set of stats metadata was available to queries run by pinot-java-client, making it difficult to troubleshoot query issue. This PR prints the entire stats JSON from server.
This is a bugfix