egalpin commented on code in PR #13742:
URL: https://github.com/apache/pinot/pull/13742#discussion_r1747732334
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/QueryResponse.java:
##########
@@ -49,12 +50,12 @@ enum Status {
/**
* Returns the current server responses without blocking.
*/
- Map<ServerRoutingInstance, ServerResponse> getCurrentResponses();
+ Map<ServerRoutingInstance, List<ServerResponse>> getCurrentResponses();
Review Comment:
I fully recognize that I may be missing some important details here, but can
you help me understand the implications of changing this interface? I see that
the interface `QueryResponse` and its only implementation `AsyncQueryResponse`
are defined in the `org.apache.pinot.core.transport` package, but all usages
are isolated to `org.apache.pinot.broker`.
I agree that the change needs to be backward compatible with the data sent
from Pinot servers to the brokers so that mismatched versions of code running
on brokers Vs servers can interoperate during upgrade. But I'm not yet certain
that this interface needs to remain unchanged in order to accomplish that.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
