Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9202 )
Change subject: IMPALA-6396: Exchange node's memory usage should include its receiver's ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/9202/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9202/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-6396: Exchange node's memory usage should include its receiver's > How does this approach compare with tracking and logging the receiver's ind The receiver itself still has its own MemTracker after this change. So, it will still be printed out separately if one were to call MemTracker::LogUsage() (e.g. when memory limit is exceeded). It's true that one cannot tell the breakdown of the exchange node memory usage from the profile. That said, most of the memory usage from an exchange node is from its receiver anyway. FWIW, one also cannot tell the peak memory usage of the receiver from the profile even before this change. http://gerrit.cloudera.org:8080/#/c/9202/2//COMMIT_MSG@16 PS2, Line 16: This makes it : easier to identify the peak memory usage of the receivers : of different exchange nodes in the runtime profile and : query summary. > Could you just state why that is the case? I'm guessing it's because the ex Yes, they use different variants of the MemTracker constructors. The exchange node's MemTracker will add a counter to the RuntimeProfile. Please let me know if you think we need to track the peak memory usage of the receiver separately (e.g. it can be added to the sender_side_profile_). As far as I can tell, most of the memory consumption of an exchange node should be from its receiver so I didn't bother to have two counters which show mostly the same numbers. http://gerrit.cloudera.org:8080/#/c/9202/2/be/src/exec/exchange-node.cc File be/src/exec/exchange-node.cc: http://gerrit.cloudera.org:8080/#/c/9202/2/be/src/exec/exchange-node.cc@85 PS2, Line 85: state->fragment_instance_id(), id_, num_senders_, > nit: input_row_desc could fit on previous line. Done http://gerrit.cloudera.org:8080/#/c/9202/2/be/src/runtime/krpc-data-stream-mgr.cc File be/src/runtime/krpc-data-stream-mgr.cc: http://gerrit.cloudera.org:8080/#/c/9202/2/be/src/runtime/krpc-data-stream-mgr.cc@100 PS2, Line 100: DCHECK(profile != nullptr); > nit: DCHECK that parent_tracker != nullptr Done -- To view, visit http://gerrit.cloudera.org:8080/9202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ca3c47d87bfcd221d34565eda1878f3c15d5c45 Gerrit-Change-Number: 9202 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Comment-Date: Wed, 07 Feb 2018 23:38:15 +0000 Gerrit-HasComments: Yes
