Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15497 )
Change subject: IMPALA-8005: Randomize partitioning exchanges. ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc File be/src/runtime/data-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@496 PS4, Line 496: // each sender sent all values : EXPECT_EQ(k / num_senders, *j); : if (k/num_senders != > Couple small style things: Thanks for the pointers. Since you did not have a preference and the Google style guide says "Prefer using return values over output parameters: they improve readability and often provide the same or better performance." I went ahead and used return values. http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@516 PS4, Line 516: StartSender(TPartitionType::HASH_PARTITIONED, buffer_size, : reset_hash_seed); : } : JoinSenders(); : CheckSenders(); : JoinReceivers(); : receiver_data_map.clear(); : : for (int i = 0; i < receiver_info_.size(); i++) { : > Nit: For the output parameter receiver_data_map, go ahead and do receiver_d Done http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@596 PS4, Line 596: eset the hash seed with a new query id. Useful for testing h > Since you did this above for the 'config' variable, I think it would be cle Done http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/krpc-data-stream-sender.cc@739 PS4, Line 739: } : > Do we need this? We set exchange_hash_seed_ above. Thanks for the catch. Removed. -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Thu, 26 Mar 2020 23:13:12 +0000 Gerrit-HasComments: Yes
