Marcel Kornacker has posted comments on this change. Change subject: PREVIEW IMPALA-2550: RPC batching ......................................................................
Patch Set 5: (9 comments) http://gerrit.cloudera.org:8080/#/c/3390/5/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 1: // Copyright 2012 Cloudera Inc. this file doesn't look like it's ready for review. lots of functions are missing comments and the structure has an unfinished feel to it (please correct me if that wasn't the intention) Line 93: // Helper functions. TODO: move to an own module? all of these need comments Line 97: void CollectOutputSinkTableIds(const TPlanFragment& fragment, why plural? also, why not just return it? Line 106: void CollectTupleIds(const TPlanFragment& fragment, this only collects tuple ids from scans, so maybe name it CollectScanTupleIds. Line 144: const std::unordered_set<TTupleId>& tuple_ids, remove std:: Line 205: fragment_instance_ctx->fragment_instance_id = never hurts to use __set Line 421: class Coordinator::RpcBuilder { why a class instead of a function? Line 423: static void BuildSingleFragmentInstance(const TQueryCtx& query_ctx, needs comment Line 457: instance_state_idx(instance_state_idx), fragment_idx(fragment_idx), formatting -- To view, visit http://gerrit.cloudera.org:8080/3390 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e3a9d4a78b59394aaf343df08f6bfda22c3148e Gerrit-PatchSet: 5 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
