mcvsubbu commented on PR #13661: URL: https://github.com/apache/pinot/pull/13661#issuecomment-2240793647
> Currently, we always set request arrival time when the `RequestContext` is just created (since it is a property within it). Are you running some some custom code which is directly hooked to the `BrokerRequestHandler.handleRequest()`? Sorry I didn't expect this usage pattern during the refactor. I'd suggest following the same way to set it when the `RequestContext` is created for consistency. If you want to make it more robust, we can set it in `BaseBrokerRequestHandler.handleRequest()` because we might not use delegate all the time The `BaseBrokerStarter` class returns the delegate class from the `getBrokerRequestHandler()` handler. So, this class (and the `handleRequest()` methods within this class) are meant to be external interface to a broker. So, i am not sure what you mean when you say that we may not use delegate all the time. Can you explain a bit more? Secondly, yes, installations may have some code before (and after return from) `handleRequest()`. Dont you think that the callers of `handleRequest()` should measure their own overhead if they are doing pre and post processing and add it on to the value returned by handleRequest() call for query time? It does not seem to be correct that the metric emitted includes only the pre-processing time but not the post-processing time. Agree? IIRC used to be that request context was created inside handleRequest and returned. Somehow that interface has now changed to include requestContext as an argument, and I think that has caused the confusion. Why is the "client" code directly calling BrokerRequestHandler? (isnt the client remote)? I can change it to <= 0 if you like, but I also think the contract of this method needs a re-look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
