Todd Lipcon has posted comments on this change.

Change subject: rpc: call-level feature flags
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/2239/7/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

Line 554:   kSupportedServerRpcFeatureFlags = {};
there should be a scoped resetter here (save off old value and set it back). 
maybe something like http://the-witness.net/news/2012/11/scopeexit-in-c11/ 
would be nice to have in util


http://gerrit.cloudera.org:8080/#/c/2239/7/src/kudu/rpc/transfer.cc
File src/kudu/rpc/transfer.cc:

Line 126: OutboundTransfer::OutboundTransfer(uint64_t call_id,
you pass -1 here elsewhere, maybe unsigned is not a good idea? int64_t should 
allow the -1 and still be enough forever


http://gerrit.cloudera.org:8080/#/c/2239/7/src/kudu/rpc/transfer.h
File src/kudu/rpc/transfer.h:

Line 109:   // The required features are checked against the set of features 
that the
this makes it sound like the checking happens in the constructor, rather than 
when it first starts to transfer on a connection


-- 
To view, visit http://gerrit.cloudera.org:8080/2239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I46e46bcf80b93fc6cce50f37dd71a6e6539047f8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to