ankitsultana commented on code in PR #10285:
URL: https://github.com/apache/pinot/pull/10285#discussion_r1106613517
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryDispatcher.java:
##########
@@ -196,12 +196,13 @@ private static DataSchema toResultSchema(DataSchema
inputSchema, List<Pair<Integ
@VisibleForTesting
public static MailboxReceiveOperator
createReduceStageOperator(MailboxService<TransferableBlock> mailboxService,
- List<VirtualServer> sendingInstances, long jobId, int stageId,
DataSchema dataSchema, VirtualServerAddress server,
+ List<VirtualServer> sendingInstances, long jobId, int stageId, int
reducerStageId, DataSchema dataSchema,
+ VirtualServerAddress server,
Review Comment:
self-review: Move timeoutMs in this line.
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/MailboxIdentifier.java:
##########
@@ -50,4 +50,8 @@ public interface MailboxIdentifier {
* @return true if sender and receiver are in the same JVM.
*/
boolean isLocal();
+
Review Comment:
self-review: Add doc comments.
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/mailbox/GrpcMailboxServiceTest.java:
##########
@@ -78,7 +78,8 @@ public void testHappyPath()
JsonMailboxIdentifier mailboxId = new JsonMailboxIdentifier(
"happypath",
new VirtualServerAddress("localhost",
_mailboxService1.getMailboxPort(), 0),
- new VirtualServerAddress("localhost",
_mailboxService2.getMailboxPort(), 0));
+ new VirtualServerAddress("localhost",
_mailboxService2.getMailboxPort(), 0),
Review Comment:
self-review: Use static constants for sender/receiver stage-id here and in
other places.
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/executor/RoundRobinSchedulerTest.java:
##########
@@ -33,9 +33,12 @@
public class RoundRobinSchedulerTest {
+ private static final int DEFAULT_RECEIVER_STAGE_ID = 1;
Review Comment:
self-review: Add DEFAULT_SENDER_STAGE_ID variable for consistency
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/JsonMailboxIdentifier.java:
##########
@@ -106,6 +116,16 @@ public boolean isLocal() {
return _fromAddress.equals(_toAddress);
}
+ @Override
Review Comment:
self-review: Add JsonIgnore annotations and make getters order same as
declarations.
--
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]
