Return JSON instead of str in the broker debug api /debug/serverRoutingStats #15018
Conversation
vrajat
added
backward-incompat
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15018 +/- ##
============================================
+ Coverage 61.75% 63.68% +1.93%
- Complexity 207 1483 +1276
============================================
Files 2436 2727 +291
Lines 133233 152460 +19227
Branches 20636 23565 +2929
============================================
+ Hits 82274 97096 +14822
- Misses 44911 48051 +3140
- Partials 6048 7313 +1265
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
cc @somandal |
| @@ -198,6 +198,10 @@ private void updateStatsUponResponseArrival(String serverInstanceId, long latenc | |||
| } | |||
| } | |||
|
|
|||
| public Map<String, ServerRoutingStatsEntry> getServerRoutingStats() { | |||
There was a problem hiding this comment.
can you add tests for this in ServerRoutingStatsManagerTest to validate that it looks as expected?
| @@ -198,6 +198,10 @@ private void updateStatsUponResponseArrival(String serverInstanceId, long latenc | |||
| } | |||
| } | |||
|
|
|||
| public Map<String, ServerRoutingStatsEntry> getServerRoutingStats() { | |||
| return _serverQueryStatsMap; | |||
There was a problem hiding this comment.
Let us check for _isEnabled here? _serverQueryStatsMap would be null if config is disabled.
There was a problem hiding this comment.
Oh wait - I checked for isEnabled in the PinotBrokerDebug file. Is that OK ? Right now, it is the caller's responsibility to check for isEnabled and null.
Fix for #15017
The o/p of the api is not easily machine readable. An example o/p is:
Instead return a json since its more easily analyzed using scripts. For example, the following json o/p of the same data:
Can be processed by shell scripts to create csv files and plot them.
This change is backward-incompatible. However this is a debug API.