Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4889: Use client sidecars for Thrift RPCs ......................................................................
Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/6473/1/be/src/rpc/rpc.h File be/src/rpc/rpc.h: Line 101: // RPC invocation using Execute(). why synchronous? Line 102: Status GetInboundSidecar(int idx, kudu::Slice* sidecar) { 'inbound' sounds a bit misleading, my initial response was 'you mean server-side?' instead of 'this is a return value'. Line 169: int idx = -1; that shouldn't be necessary. Line 170: AddSidecar(kudu::Slice(serialized), &idx); looks like this is making a copy, bummer. is there some way to avoid that? Line 176: RETURN_IF_ERROR(GetInboundSidecar(response_proto.sidecar_idx(), &sidecar)); do we not want to reserve this mechanism for large pieces of data? Line 183: Rpc(const Rpc& other) { comment why needed Line 207: // Rpc controller storage. Used only for synchronous RPCs so that the caller can access how do we do async rpcs again? -- To view, visit http://gerrit.cloudera.org:8080/6473 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fcc87c6b67aa167d70ae022663b14bc90261c95 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
