Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16474 )
Change subject: WIP IMPALA-10178 Run-time profile shall report skews ...................................................................... Patch Set 4: (6 comments) Read through out of curiousity. Cool! http://gerrit.cloudera.org:8080/#/c/16474/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16474/4//COMMIT_MSG@13 PS4, Line 13: Can you include an example of what the output looks like? http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile-counters.h@431 PS4, Line 431: bool HasSkew() { nit: move to .cc file to reduce compilation time. I guess this is gonna conflict with https://gerrit.cloudera.org/#/c/15798/ a bit cause I changed the AveragedCounter data structures. http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile.cc@611 PS4, Line 611: const std::string RuntimeProfile::PROFILE_INFO_KEY_SKEW_SUMMARY = "FRAGMENT_SKEW_SUMMARY"; nit: put constants up the top of the file. http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile.cc@613 PS4, Line 613: std::string can omit std:: from std::string, std::vector in .cc files where common/names.h is included. http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile.cc@620 PS4, Line 620: Chilren Children http://gerrit.cloudera.org:8080/#/c/16474/4/be/src/util/runtime-profile.cc@648 PS4, Line 648: auto auto& to avoid an implied copy -- To view, visit http://gerrit.cloudera.org:8080/16474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91041f2856eef8293ea78f1721f97469062589a1 Gerrit-Change-Number: 16474 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 18 Sep 2020 22:11:51 +0000 Gerrit-HasComments: Yes
