Improved metrics for server grpc query #13177
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13177 +/- ##
============================================
+ Coverage 61.75% 62.25% +0.50%
- Complexity 207 1294 +1087
============================================
Files 2436 2529 +93
Lines 133233 138289 +5056
Branches 20636 21400 +764
============================================
+ Hits 82274 86090 +3816
- Misses 44911 45782 +871
- Partials 6048 6417 +369
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pinot-core/src/main/java/org/apache/pinot/core/transport/grpc/GrpcResultsBlockStreamer.java
Show resolved
Hide resolved
| private class GrpcQueryTransportFilter extends ServerTransportFilter { | ||
| @Override | ||
| public Attributes transportReady(Attributes transportAttrs) { | ||
| LOGGER.info("gRPC transportReady: REMOTE_ADDR {}", |
There was a problem hiding this comment.
Will this potentially flood the log? Since we already have the metrics, maybe no need to log it
There was a problem hiding this comment.
Proxy creates the transport when it gets the first query for a server, and does not close it, so no risk of flooding the log file.
There was a problem hiding this comment.
This is also logging the remote address to help in troubleshooting.
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java
Outdated
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java
Outdated
Show resolved
Hide resolved
| @@ -162,13 +187,16 @@ public void submit(ServerRequest request, StreamObserver<ServerResponse> respons | |||
| _serverMetrics.addMeteredGlobalValue(ServerMeter.NO_TABLE_ACCESS, 1); | |||
| responseObserver.onError( | |||
| Status.NOT_FOUND.withDescription(exceptionMsg).withCause(unsupportedOperationException).asException()); | |||
| return; | |||
There was a problem hiding this comment.
Request should terminate after onError, so added return.
…nged execution time at table level.
soumitra-st
force-pushed
the
grpc-query-server-metrics
branch
from
95a864b
to
a0ba48d
Compare
Added transport filter to keep track of number of gRPC connections, and added metrics to keep track of total execution times of successful and failed queries.
Added bytes sent metric in GrpcResultsBlockStreamer method - HERE
observability