Pranay Singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/11021 )
Change subject: IMPALA-6214: Determine and warn about stuck fragment instances. ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/11021/1/be/src/runtime/krpc-data-stream-recvr.cc File be/src/runtime/krpc-data-stream-recvr.cc: http://gerrit.cloudera.org:8080/#/c/11021/1/be/src/runtime/krpc-data-stream-recvr.cc@230 PS1, Line 230: stringstream dstid; : : dstid << recvr_->dest_node_id(); : string frgId = PrintId(recvr_->fragment_instance_id()); : string watchdog_str = "fragment_instance_id= " + frgId + " node= " + dstid.str(); : memset(buf, BUFSZ, 0); : sprintf(buf, "%s : %d %s", __FILE__, __LINE__, watchdog_str.c_str()); > constructing this once per batch seems like a big performance red flag. I'll remove it. http://gerrit.cloudera.org:8080/#/c/11021/1/be/src/runtime/krpc-data-stream-recvr.cc@239 PS1, Line 239: kudu::SetStackTraceSignal(SIGRTMIN + 1); > you should call this at init time, not once per batch. I'm not even sure it I'll remove it. http://gerrit.cloudera.org:8080/#/c/11021/1/be/src/runtime/krpc-data-stream-recvr.cc@266 PS1, Line 266: data_arrival_cv_.wait(l); > maybe it would be simpler to just use a timed-wait variant here to periodic Thanks Todd I was thinking along the same lines, it's a good and easy fix for this case. -- To view, visit http://gerrit.cloudera.org:8080/11021 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I260a1d0a3477e5c6a46094e664500c3e2ed7de62 Gerrit-Change-Number: 11021 Gerrit-PatchSet: 1 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 24 Jul 2018 20:14:24 +0000 Gerrit-HasComments: Yes
