Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9690 )
Change subject: IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/9690/1/be/src/runtime/krpc-data-stream-recvr.h File be/src/runtime/krpc-data-stream-recvr.h: http://gerrit.cloudera.org:8080/#/c/9690/1/be/src/runtime/krpc-data-stream-recvr.h@240 PS1, Line 240: RuntimeProfile::Counter* deserialize_row_batch_timer_; it would be helpful to distinguish which counters belong to which profile. we could encode it in the field names, or you could add comments to break up the counters into two buckets: /// Following counters owned by dequeue_profile_. ... /// Following counters owned by enqueue_profile_. http://gerrit.cloudera.org:8080/#/c/9690/1/be/src/runtime/krpc-data-stream-recvr.h@297 PS1, Line 297: wait_time_start_ for uint64 values that hold time, it's nice to encode the units in the name: e.g. wait_start_ns_ http://gerrit.cloudera.org:8080/#/c/9690/1/be/src/runtime/krpc-data-stream-recvr.cc File be/src/runtime/krpc-data-stream-recvr.cc: http://gerrit.cloudera.org:8080/#/c/9690/1/be/src/runtime/krpc-data-stream-recvr.cc@209 PS1, Line 209: (num_pending_enqueue_ + Let's add a brief comment explaining this dcheck. -- To view, visit http://gerrit.cloudera.org:8080/9690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd Gerrit-Change-Number: 9690 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Comment-Date: Mon, 19 Mar 2018 22:58:17 +0000 Gerrit-HasComments: Yes
