Adar Dembo has posted comments on this change.

Change subject: KUDU-687: expose additional tablet metadata in C++ client

Patch Set 1:

File src/kudu/client/client.h:

Line 488:     NON_PARTICIPANT = 3,
> how come this one has a value but not the others?
My bad. I had values in there originally, then removed them all (or so I 

Line 495:   Role role() const;
> what's the intended user use case for this one? maybe we can offer somethin
ksck keeps track of replicas that are leaders and those that are followers. 
AFAICT it only actually uses is_leader (see Ksck::VerifyTablet).

I'll replace this with an is_leader() boolean instead.
File src/kudu/client/

Line 243: void RemoteTablet::GetRemoteReplicas(vector<RemoteReplica>* replicas) 
const {
> should clear replicas first
File src/kudu/client/

PS1, Line 39: encoding
> 'role type'
Whoops, copy/pasta.

Line 49:     default: LOG(FATAL) << "Unexpected encoding type: " << role;
> not sure this is a good idea, since if we added a new role, clients would s
We need to return something; what should we do? Or should all of these be 
modified to return a Status in the event of an unknown input type?

I guess this is a moot point as I'm going to remove the new enum in favor of a 
boolean for is_leader.
File src/kudu/client/

Line 258:     // they've been moved.
> you could use ElementDeleter and just call client_replicas.clear(); after s
Dan convinced me that following std::move(), it is undefined behavior to access 
an object. But, I see SO posts that agree with your assessment, so I'll switch 
this back.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to