You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "mingmxu (via GitHub)" <gi...@apache.org> on 2023/06/09 23:32:41 UTC

[GitHub] [pinot] mingmxu opened a new issue, #10884: returns requestId for each broker query request

mingmxu opened a new issue, #10884:
URL: https://github.com/apache/pinot/issues/10884

   ## what's the problem?
   Return a unique `requestId` always is important for tracing/debugging, to find related logging events for example. There're some limitation at the moment:
   1. it's not available to every queries in broker, only`MultiStageBrokerRequestHandler` generates one and included in `BrokerResponseNativeV2`;
   2. In java-client, `org.apache.pinot.client.BrokerResponse` is a reduced set of `org.apache.pinot.common.response.BrokerResponse`, `requestId` is lost with many other metadata fields;
   
   ## Proposed changes
   To address these limitations, I would like to suggest some changes as below:
   * generate `requestId` in [PinotClientRequest.java](https://github.com/apache/pinot/blob/880a074c7ae71ff3937171e504f6287d48c0fa7f/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java#L81), and add a new field `requestId` in `org.apache.pinot.common.response.BrokerResponse`;
   * remove `org.apache.pinot.client.BrokerResponse` and use  `response.BrokerResponse` directly;
   


-- 
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-unsubscribe@pinot.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] mingmxu commented on issue #10884: returns requestId for each broker query request

Posted by "mingmxu (via GitHub)" <gi...@apache.org>.
mingmxu commented on issue #10884:
URL: https://github.com/apache/pinot/issues/10884#issuecomment-1595991547

   surely will submit a PR on it. The `requestId` can be generated and set in `BrokerResponse` at method `org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler#handleRequest()`, using `MultiStageRequestIdGenerator` as the default one for both v1/v2. 
   I'll add a new `requestId` field in `org.apache.pinot.client.BrokerResponse` for now. Consolidating `org.apache.pinot.client.BrokerResponse` and `org.apache.pinot.common.response.BrokerResponse` would move to the next PR. 


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on issue #10884: returns requestId for each broker query request

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #10884:
URL: https://github.com/apache/pinot/issues/10884#issuecomment-1595412501

   Broker request id is generated within the `BrokerRequestHandler`, and I think we can keep it this way, but include it in the `org.apache.pinot.common.response.BrokerResponse`.
   Seems `org.apache.pinot.client.BrokerResponse` was introduced to avoid dependency on `pinot-common`, but the module already has dependency on `pinot-common` so we may remove it. We need to be cautious about backward compatibility though.
   
   @mingmxu Do you want to help contribute this?


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org