Adar Dembo has posted comments on this change. Change subject: KUDU-687: expose additional tablet metadata in C++ client ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/client.h 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 thought). 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. http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 243: void RemoteTablet::GetRemoteReplicas(vector<RemoteReplica>* replicas) const { > should clear replicas first Done http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/replica-internal.cc File src/kudu/client/replica-internal.cc: 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. http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/scan_token-internal.cc File src/kudu/client/scan_token-internal.cc: 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 http://gerrit.cloudera.org:8080/4146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
