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: 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

Reply via email to