mcvsubbu commented on PR #13661: URL: https://github.com/apache/pinot/pull/13661#issuecomment-2241720423
> > I suggest setting it (without any if check) inside the handleRequest() call, like it was before. If this is ok with you, I will remove the if around the set. The contract with the broker for external entities should be that the handleRequest() call provides the service, and emits metric that includes the time taken for the service. > > No, I don't agree. We should capture the query parsing time as part of the request handling. You may take a look at `PinotClientRequest.executeSqlQuery()` (the rest resource to handle query) where we first parse the query, then send it to the request handler if it is `DQL` (there are also `DML` queries not handled by request handler). The request arrival time should be set to the time when the request arrives the broker. I see. In that case, are we deprecating handleRequest() (and therefore BrokerRequestHandler)? Should PinotClientRequest be moved to spi? -- 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
