[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-31 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged.

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..


Specify guaranteed semantics of GetTableLocations RPC

This commit specifies additional semantics guaranteed by the master's
GetTableLocations RPC. Technically this is a breaking change as some of the
existing behavior is changed, but in practice no known published client version
is affected. These guarantees are necessary in order to implement a meta cache
in the presence of non-covering partitions.

Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Reviewed-on: http://gerrit.cloudera.org:8080/3240
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
4 files changed, 58 insertions(+), 24 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-31 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..


Patch Set 3: Code-Review+2

-- 
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-31 Thread Dan Burkert (Code Review)
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3240

to look at the new patch set (#3).

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..

Specify guaranteed semantics of GetTableLocations RPC

This commit specifies additional semantics guaranteed by the master's
GetTableLocations RPC. Technically this is a breaking change as some of the
existing behavior is changed, but in practice no known published client version
is affected. These guarantees are necessary in order to implement a meta cache
in the presence of non-covering partitions.

Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
---
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
4 files changed, 58 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3240/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-31 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..


Patch Set 1:

actually it's missing tests for a few of the guarantees, I'll add specific 
tests for those.

-- 
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..


Patch Set 1:

(1 comment)

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 tabl
no 'break' here?


-- 
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-27 Thread Adar Dembo (Code Review)
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Specify guaranteed semantics of GetTableLocations RPC

2016-05-27 Thread Dan Burkert (Code Review)
Dan Burkert has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3240

Change subject: Specify guaranteed semantics of GetTableLocations RPC
..

Specify guaranteed semantics of GetTableLocations RPC

This commit specifies additional semantics guaranteed by the master's
GetTableLocations RPC. Technically this is a breaking change as some of the
existing behavior is changed, but in practice no known published client version
is affected. These guarantees are necessary in order to implement a meta cache
in the presence of non-covering partitions.

Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
3 files changed, 22 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3240/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert