ankitsultana commented on code in PR #10157:
URL: https://github.com/apache/pinot/pull/10157#discussion_r1083004367


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/ServerAddress.java:
##########
@@ -26,10 +26,16 @@ public class ServerAddress {
 
   private final String _hostname;
   private final int _port;
+  private final int _partition;
 
-  public ServerAddress(String hostname, int port) {
+  public ServerAddress(String hostname, int port, int partition) {

Review Comment:
   Can we avoid calling this `partition`?
   
   Partition has become quite overloaded now. One may confuse it with either 
query partition, data partition or Pinot's instance-partition.



##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/ServerAddress.java:
##########
@@ -26,10 +26,16 @@ public class ServerAddress {
 

Review Comment:
   Can we add a javadoc to describe the exact role of `ServerAddress`?
   
   Also can you clarify why we need both `VirtualServer` and `ServerAddress`?



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

Reply via email to