You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2020/07/02 17:03:45 UTC

[kudu-CR] WIP: Enable arenas for RPC request and response

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16136

to look at the new patch set (#2).

Change subject: WIP: Enable arenas for RPC request and response
......................................................................

WIP: Enable arenas for RPC request and response

This changes the RPC server side to allocate a protobuf Arena for each
request. The request RPC and response are allocated from the Arena,
ensuring that any sub-messages, strings, repeated fields, etc, use that
Arena for allocation as well. Everything is deleted en-masse when the
InboundCall object (which owns the Arena) is destructed.

This is mostly a straight-forward change except for some required
changes to some oddly-structured code in the RaftConsensus
implementation. Specifically, we were calling ExtractSubRange() on the
a const_casted version of the request in order to extract out the
ReplicateMsgs sent by the leader, but we were doing that _after_ pulling
out pointers to those messages. In the non-Arena case, the released
pointers are the same as the pointers prior to the release, but when the
request comes from an Arena, "release()" actually allocates a new copy
of the message on the heap.

Marking this WIP since my fix there is a bit messy, and some benchmark
numbers are probably worth grabbing here, though in my non-scientific
tests, this improves things for RPCs with lots of nested strings and
sub-messages.

Change-Id: I810931900fc2b5f1dec1265abadfb33fb41d29bf
---
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/service_if.cc
7 files changed, 38 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/16136/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16136
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I810931900fc2b5f1dec1265abadfb33fb41d29bf
Gerrit-Change-Number: 16136
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)