[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


KUDU-1307 [master] support tables with range partition bounds

This commit adds support to the Kudu master for creating tables with range
partition bounds. Bounds are used to create gaps in the range partition that are
not covered by a tablet. A new master feature flag has been added so that when
client support is added for creating tables with partition bounds, old masters
will not silently ignore the bounds. Adding support to the clients is left to a
follow up commit.

Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
Reviewed-on: http://gerrit.cloudera.org:8080/2806
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java
M src/kudu/client/client.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_operations.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tserver/tablet_server-test.cc
19 files changed, 803 insertions(+), 152 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


Patch Set 12: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2806/11/src/kudu/integration-tests/table_locations-itest.cc
File src/kudu/integration-tests/table_locations-itest.cc:

Line 107:   for (const pair& bound : bounds) {
: encoder.Add(RowOperationsPB::RANGE_LOWER_BOUND, 
std::get<0>(bound));
: encoder.Add(RowOperationsPB::RANGE_UPPER_BOUND, 
std::get<1>(bound));
:   }
> Nit: use auto, and use .first/.second instead of get<0>/get<1>
Done


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

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

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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/2806

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..

KUDU-1307 [master] support tables with range partition bounds

This commit adds support to the Kudu master for creating tables with range
partition bounds. Bounds are used to create gaps in the range partition that are
not covered by a tablet. A new master feature flag has been added so that when
client support is added for creating tables with partition bounds, old masters
will not silently ignore the bounds. Adding support to the clients is left to a
follow up commit.

Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
---
M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java
M src/kudu/client/client.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_operations.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tserver/tablet_server-test.cc
19 files changed, 803 insertions(+), 152 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/2806/12
-- 
To view, visit http://gerrit.cloudera.org:8080/2806
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2806/11/src/kudu/integration-tests/table_locations-itest.cc
File src/kudu/integration-tests/table_locations-itest.cc:

Line 107:   for (const pair& bound : bounds) {
: encoder.Add(RowOperationsPB::RANGE_LOWER_BOUND, 
std::get<0>(bound));
: encoder.Add(RowOperationsPB::RANGE_UPPER_BOUND, 
std::get<1>(bound));
:   }
Nit: use auto, and use .first/.second instead of get<0>/get<1>


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

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

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

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

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

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..

KUDU-1307 [master] support tables with range partition bounds

This commit adds support to the Kudu master for creating tables with range
partition bounds. Bounds are used to create gaps in the range partition that are
not covered by a tablet. A new master feature flag has been added so that when
client support is added for creating tables with partition bounds, old masters
will not silently ignore the bounds. Adding support to the clients is left to a
follow up commit.

Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
---
M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java
M src/kudu/client/client.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_operations.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tserver/tablet_server-test.cc
19 files changed, 803 insertions(+), 152 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/2806/11
-- 
To view, visit http://gerrit.cloudera.org:8080/2806
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04a7ed3e85c993fdbcb313a419f15031103736ce
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


Patch Set 10:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/2806/10//COMMIT_MSG
Commit Message:

Line 11: not covered by a tablet. A new feature master feature flag has been 
added so
> "A new feature master feature flag" is a little awkward, is there an extra 
Done


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:

Line 241: Arena arena(1024, 1024 * 1024);
> What's this doing here?
Done


Line 250:   vector>* range_partitions) const {
> To Todd's suggestions, perhaps we should DCHECK(range_partitions->empty()),
Done


Line 264:   "Range bound has lower bound equal to or above the upper 
bound",
> So FWIW, I was wrong in telling you to use capital letters for the beginnin
Done


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/common/partition.h
File src/kudu/common/partition.h:

Line 257: 
> Nit: unnecessary new line?
Done


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/integration-tests/table_locations-itest.cc
File src/kudu/integration-tests/table_locations-itest.cc:

Line 43: namespace kudu {
> Is this typical? Don't we prefer to put all the "using" statements in one g
I think we have it both ways, I'll switch this case.


Line 95:const 
vector Can we use pair here instead of tuple? I agree with Todd that for 2-tuples,
Done


Line 144:   KuduPartialRow row = KuduPartialRow();
> Nit: KuduPartialRow row();
Done


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

Line 411: KuduPartialRow a_lower = KuduPartialRow();
> Nit: these would be more terse without the copy constructor:
Done


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

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

[kudu-CR] KUDU-1307 [master] support tables with range partition bounds

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

Change subject: KUDU-1307 [master] support tables with range partition bounds
..


Patch Set 10:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/2806/10//COMMIT_MSG
Commit Message:

Line 11: not covered by a tablet. A new feature master feature flag has been 
added so
"A new feature master feature flag" is a little awkward, is there an extra 
'feature' here?


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:

Line 241: Arena arena(1024, 1024 * 1024);
What's this doing here?


Line 250:   vector>* range_partitions) const {
To Todd's suggestions, perhaps we should DCHECK(range_partitions->empty()), the 
same way you've done that for the other Encode functions?


Line 264:   "Range bound has lower bound equal to or above the upper 
bound",
So FWIW, I was wrong in telling you to use capital letters for the beginning of 
error messages. Todd reminded me of that later; since we append status messages 
to one another, it's best for them to start in lower-case so the composed 
sentences look less awkward. Sorry.


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/common/partition.h
File src/kudu/common/partition.h:

Line 257: 
Nit: unnecessary new line?


http://gerrit.cloudera.org:8080/#/c/2806/10/src/kudu/integration-tests/table_locations-itest.cc
File src/kudu/integration-tests/table_locations-itest.cc:

Line 43: namespace kudu {
Is this typical? Don't we prefer to put all the "using" statements in one giant 
block together, prefixing with kudu:: if necessary?


Line 95:const 
vector
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes