Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8914 )
Change subject: IMPALA-6193: Track memory of incoming data streams ...................................................................... Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h File be/src/rpc/rpc-mgr.h: http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h@126 PS7, Line 126: WARN_UNUSED_RESULT > I didn't know we had that. The commit message of the change that introduces Makes sense. http://gerrit.cloudera.org:8080/#/c/8914/10/be/src/runtime/krpc-data-stream-recvr.cc File be/src/runtime/krpc-data-stream-recvr.cc: http://gerrit.cloudera.org:8080/#/c/8914/10/be/src/runtime/krpc-data-stream-recvr.cc@333 PS10, Line 333: ctx->DiscardTransfer(); I thought more about this function here and there seems to be two problems here: - this causes the buffer of the InboundTransfer to be freed in the deserialization thread instead of the reactor thread in which this buffer was originally allocated on. This is anti-pattern in TCMalloc. - there seems to be divergence in behavior between the fast path (in which the RPC is not deferred) and the slow path when an RPC is deferred. In particular, we don't call DiscardTransfer() in the fast path but we do so in the slow path. -- To view, visit http://gerrit.cloudera.org:8080/8914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4 Gerrit-Change-Number: 8914 Gerrit-PatchSet: 10 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 25 Jan 2018 19:47:55 +0000 Gerrit-HasComments: Yes
