Adar Dembo has posted comments on this change.

Change subject: Specify guaranteed semantics of GetTableLocations RPC
......................................................................


Patch Set 1:

(2 comments)

Would be nice to see a test (perhaps in master-test) for the new semantics.

http://gerrit.cloudera.org:8080/#/c/3240/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

Line 3093: StatusToPB(Status::ServiceUnavailable("Tablet not running"),
         :                  resp->mutable_error()->mutable_status());
BuildLocationsForTablet() can also return Status::NotFound for deleted tablets, 
so this handling needs to be a little more generic, and should probably take 
the return value of BuildLocationsForTablet() into account.

Moreover, what will the semantics be once "drop range" support is implemented 
and tablets can be deleted? Will deleted tablets be filtered out in 
GetTabletsInRange()? Or will they show up here?


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

Line 415: non-covered ranges
How about a bullet point saying this explicitly (i.e. a gap in range coverage 
between any pair of tablets indicates a non-covered range).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to