Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12615 )

Change subject: KUDU-2711 (part 3). Only send one TSInfoPB per server in 
GetTableLocations
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12615/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12615/1//COMMIT_MSG@12
PS1, Line 12: In the
            : worst case, this may slightly regress performance if the number of
            : replicas in the response is much fewer than the number of TS in 
the
            : cluster, since there is some extra overhead of building the index
            : mapping.
> I suppose we could run the GetTableLocations benchmark with a table where t
Will, could you run this experiment?


http://gerrit.cloudera.org:8080/#/c/12615/1/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/12615/1/src/kudu/master/master.proto@373
PS1, Line 373:   repeated ReplicaPB replicas = 4;
> I'm not sure I understand. What do you mean it would "look weird"?
Well, we typically add a "DEPRECATED_" prefix to protobuf fields when we want 
to emphasize that they're deprecated:

  $ git grep DEPRECATED_
  src/kudu/consensus/consensus.proto:  optional OpId DEPRECATED_committed_index 
= 5;
  src/kudu/consensus/log.proto:  optional uint32 DEPRECATED_major_version = 1;
  src/kudu/consensus/log.proto:  optional uint32 DEPRECATED_minor_version = 2;
  src/kudu/master/master.proto:  optional bytes DEPRECATED_start_key = 1;
  src/kudu/master/master.proto:  optional bytes DEPRECATED_end_key   = 2;
  src/kudu/master/master.proto:  optional bool DEPRECATED_stale = 5;
  src/kudu/master/master.proto:  optional AppStatusPB DEPRECATED_error = 2;
  src/kudu/rpc/rpc_header.proto:  optional UserInformationPB 
DEPRECATED_user_info = 2;
  src/kudu/tserver/tserver.proto:  repeated ColumnRangePredicatePB 
DEPRECATED_range_predicates = 3;

But I think we've only done that when there's no referring code that actually 
uses it, which isn't the case here. And if you prefixed the field with 
DEPRECATED_, usages of the generated protobuf code would look pretty funky 
(i.e. message.DEPRECATED_replicas_size()).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief65d0825e919f681b7ade6a856c103201f8305c
Gerrit-Change-Number: 12615
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Tue, 12 Mar 2019 01:30:10 +0000
Gerrit-HasComments: Yes

Reply via email to