Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14715 )
Change subject: IMPALA-4618: Fixing #Hosts and adding #Instances in exec summary ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/14715/3/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/14715/3/be/src/runtime/coordinator.cc@237 PS3, Line 237: // counting the number of hosts and instances nit: Might want to fix capitalization and punctuation (see comments below), e.g. "Count the number of hosts and instances." http://gerrit.cloudera.org:8080/#/c/14715/3/be/src/runtime/coordinator.cc@238 PS3, Line 238: const std::vector<FInstanceExecParams>& instance_params = nit: remove std:: http://gerrit.cloudera.org:8080/#/c/14715/3/be/src/runtime/coordinator.cc@241 PS3, Line 241: nit: remove space (follow surrounding code, e.g. line 228 and 229) http://gerrit.cloudera.org:8080/#/c/14715/3/be/src/runtime/coordinator.cc@242 PS3, Line 242: host_set.insert(instance.host); Do we need to take a copy here? How about "const FInstanceExecParams& instance"? http://gerrit.cloudera.org:8080/#/c/14715/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/14715/3/shell/impala_shell.py@1080 PS3, Line 1080: return self.construct_table_with_header(["Operator", "#Hosts", "#Instances", how about #Inst to keep things concise (Please go with the consensus on the email thread on dev@) -- To view, visit http://gerrit.cloudera.org:8080/14715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bdf9a06d9bd842b2397cd16c28294b6bec7af69 Gerrit-Change-Number: 14715 Gerrit-PatchSet: 3 Gerrit-Owner: Norbert Luksa <norbert.lu...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Norbert Luksa <norbert.lu...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Wed, 20 Nov 2019 19:53:23 +0000 Gerrit-HasComments: Yes
