David Ribeiro Alves has posted comments on this change. Change subject: Allow to release an rpc transfer's data ......................................................................
Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/6592/5//COMMIT_MSG Commit Message: PS5, Line 10: obtain data for decoding > what do you mean by that? Done http://gerrit.cloudera.org:8080/#/c/6592/5/src/kudu/rpc/outbound_call.h File src/kudu/rpc/outbound_call.h: PS5, Line 324: should > should be obtained? or MUST be obtained? (i.e after releasing transfer data Done PS5, Line 323: // Returns a pointer to the transfer data, including the header and all : // sidecars. Pointers to individual sidecars should be obtained before : // calling this method. That is, this method is not meant to return : // data for decoding but rather to provide a way to release ownership : // of the whole transfer data. > maybe move this doc to RpcController so it's user-facing, and just leave a Done PS5, Line 326: // data for decoding but rather to provide a way to release ownership : // of the whole transfer data. : > if it only is used for ownership, then why do we need to get a Slice instea Done http://gerrit.cloudera.org:8080/#/c/6592/5/src/kudu/rpc/rpc_controller.h File src/kudu/rpc/rpc_controller.h: PS5, Line 192: // Resets this controller, but releases the transfer data before. : Status ReleaseTransferDataAndReset(Slice* release_data); > "transfer data" here is too implementation-centric. Done http://gerrit.cloudera.org:8080/#/c/6592/5/src/kudu/rpc/transfer.h File src/kudu/rpc/transfer.h: PS5, Line 87: // NOTE: buf_.release() doesn't memcpy if the transfer has completed, because : // buf_ must have been mandatorily resized from the initial capacity of 4 bytes : // to receive the whole message. : > not sure I follow. The initial capacity of faststring is 32 bytes, so it's removed this comment, the worse that can happen is that the data is memcpy'd in the that case but I don't think that matters. -- To view, visit http://gerrit.cloudera.org:8080/6592 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I55c2b2d24c347ccdffc054eeb4131eaf9e82f901 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
