[kudu-CR] KUDU-1918 Prevent hijacking of scanner IDs

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6348 )

Change subject: KUDU-1918 Prevent hijacking of scanner IDs
..


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/6348/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/6348/6//COMMIT_MSG@15
PS6, Line 15: scans
> Including fault tolerant ones, right?
Right. I'm not going to call that out specifically since fault-tolerant scans 
and non-fault-tolerant scans will be caught by the same endpoint.


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/client/client-test.cc@5753
PS6, Line 5753: the
> Can you test with a fault tolerant scan as well?
Done


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners-test.cc
File src/kudu/tserver/scanners-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners-test.cc@45
PS6, Line 45:
> nit: move it to L44.
Done


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h@69
PS6, Line 69: The client
> nit: mention it has to be the same client.
It doesn't have to be the same client ID. It just has to be the same user.


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h@82
PS6, Line 82: // Create a new scanner with a unique ID, inserting it into the 
map.
> nit: update it to reflect how 'remote_user' is used.
Done


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_server-test.cc@3406
PS6, Line 3406: // Test that scanners can only be accessed by the user who 
created it.
> nit: add a doc for this?
Done


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_service.cc@1302
PS6, Line 1302: !s.ok()) {
> I think it is probably rare to get an NOT_AUTHORIZED error, but do you thin
Yeah good point. Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4
Gerrit-Change-Number: 6348
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 03 Nov 2018 00:38:31 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1918 Prevent hijacking of scanner IDs

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has uploaded a new patch set (#7) to the change originally created 
by Todd Lipcon. ( http://gerrit.cloudera.org:8080/6348 )

Change subject: KUDU-1918 Prevent hijacking of scanner IDs
..

KUDU-1918 Prevent hijacking of scanner IDs

This makes the scanner remember its RemoteUser, and ensures that when
continuing a scan, the new requestor matches the original requestor.

This prevents one user from somehow obtaining a scanner ID from another
and then "hijacking" the in-progress scan.

This restricts scans, checksum scans, and keep-alive requests.

Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/tserver/scanners-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
13 files changed, 272 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/6348/7
--
To view, visit http://gerrit.cloudera.org:8080/6348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4
Gerrit-Change-Number: 6348
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11869 )

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..


Patch Set 3:

The alter table randomized failure looks unrelated but like it could be a real 
thing: a tablet failed because it's cmeta was missing after its tablet server 
was restarted. The master was trying to create the tablet replica on the server 
at the time- the tablet server was trying to create the tablet when it crashed 
and it ended up with missing cmeta. The cluster tried to copy over the failed 
tablet replica but couldn't. I think there might be an issue filed for this 
already.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sat, 03 Nov 2018 00:07:05 +
Gerrit-HasComments: No


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11869 )

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..


Patch Set 3:

I'm not sure about the kudu-tool-test failure. TSAN reports a deadlock on a 
rowset's compact_flush_lock, but there's not much more information than that. 
This patch touches code close to that, but it doesn't actually change anything 
that AFAICT interacts with locking, so not sure what is going on here.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 23:59:33 +
Gerrit-HasComments: No


[kudu-CR] [rebalancer] location-aware rebalancer (part 9/n)

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11862 )

Change subject: [rebalancer] location-aware rebalancer (part 9/n)
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc
File src/kudu/tools/placement_policy_util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc@142
PS2, Line 142: lhs,
 :
> There's a default comparison operator in C++? I didn't think there was. How
Woops, it seems I confused that with default copy constructor which works on 
field-by-field basis.  It seems I was too grumpy yesterday night :)


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc@143
PS2, Line 143: hs) {
> nonsense
Done


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc
File src/kudu/tools/rebalancer.cc:

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@129
PS2, Line 129: information
> information
Done


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@137
PS2, Line 137: lance.
> violations
Done


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@138
PS2, Line 138: RETURN_NOT_OK(PrintCrossLocationBalanceStats(ci, out));
> It looks like the output could be kind of long. Can we move placement polic
Done


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@145
PS2, Line 145: ons.res
> sorted
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
Gerrit-Change-Number: 11862
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 23:30:20 +
Gerrit-HasComments: Yes


[kudu-CR] [rebalancer] location-aware rebalancer (part 9/n)

2018-11-02 Thread Alexey Serbin (Code Review)
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: [rebalancer] location-aware rebalancer (part 9/n)
..

[rebalancer] location-aware rebalancer (part 9/n)

Updated reporting functionality of the rebalancer tool to output
information on placement policy violations and other relevant
information for location-aware clusters.

Added one simple integration test as well.

Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
---
M src/kudu/tools/placement_policy_util-test.cc
M src/kudu/tools/placement_policy_util.cc
M src/kudu/tools/placement_policy_util.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
6 files changed, 457 insertions(+), 141 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
Gerrit-Change-Number: 11862
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11869 )

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..


Patch Set 3:

>  I need to check how the undos might have done away.

We do this as part of merge compaction. So either we should probably turn off 
rowset compaction for the test, since we specifically want to test the undo 
delta block GC MM op.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 23:28:31 +
Gerrit-HasComments: No


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11869 )

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..


Patch Set 3:

The tablet history gc test failure looks like it was caused by this patch, but 
not a bug. Basically, the following happened:

0. The test is tuned to do maintenance ops constantly.
1. Rows are inserted. A flush happens while rows are still being inserted.
2. More rows are inserted. A second flush happens.
3. Wait for maintenance ops to flush some undos.
4. The clock is advanced to make the undos old enough to GC.
5. The two DRS are compacted because they are small. This GC's the undos as 
part of the process? Or it doesn't compact undos into redos because the AHM 
passed for the redos? I need to check how the undos might have done away.
6. While the compaction is working, all the undo gc ops run but can't gc the 
blocks of the rowsets being compacted.
7. The output rowset has no undos. The test times out waiting for its undos to 
be gc'd and for that to be shown in the undo op metrics.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 23:23:44 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1918 Prevent hijacking of scanner IDs

2018-11-02 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6348 )

Change subject: KUDU-1918 Prevent hijacking of scanner IDs
..


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/6348/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/6348/6//COMMIT_MSG@15
PS6, Line 15: scans
Including fault tolerant ones, right?


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/client/client-test.cc@5753
PS6, Line 5753: Open
Can you test with a fault tolerant scan as well?


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners-test.cc
File src/kudu/tserver/scanners-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners-test.cc@45
PS6, Line 45: using rpc::RemoteUser;
nit: move it to L44.


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h@69
PS6, Line 69: The client
nit: mention it has to be the same client.


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/scanners.h@82
PS6, Line 82: // Create a new scanner with a unique ID, inserting it into the 
map.
nit: update it to reflect how 'remote_user' is used.


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_server-test.cc@3406
PS6, Line 3406: TEST_F(TabletServerTest, TestScannerCheckMatchingUser) {
nit: add a doc for this?


http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/6348/6/src/kudu/tserver/tablet_service.cc@1302
PS6, Line 1302: PREDICT_FALSE
I think it is probably rare to get an NOT_AUTHORIZED error, but do you think it 
is rare to get SCANNER_EXPIRED error as well?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4
Gerrit-Change-Number: 6348
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 23:15:12 +
Gerrit-HasComments: Yes


[kudu-CR] [tools] reference comparison mode for rebalancing algo tests

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11870


Change subject: [tools] reference comparison mode for rebalancing algo tests
..

[tools] reference comparison mode for rebalancing algo tests

Introduced comparison mode for the rebalancing algorithms' tests.

There are some implementation details like using the hash (not tree)
dictionary collections and additional randomness among the moves of
equal cost in the cross-location rebalancing algorithms.  In some
scenarios, when the order of the resulting moves might be different
from one run to another due to the mentioned details, it's necessary
to normalize the reference and the actual results before comparison.

Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91
---
M src/kudu/tools/rebalance_algo-test.cc
1 file changed, 57 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91
Gerrit-Change-Number: 11870
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11869 )

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11869/1/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:

http://gerrit.cloudera.org:8080/#/c/11869/1/src/kudu/tablet/rowset_info.cc@32
PS1, Line 32:
> warning: #includes are not sorted properly [llvm-include-order]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 21:28:52 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd 
Lipcon,

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

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

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

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..

[compaction] KUDU-1400: Improve rowset compaction policy to consider merging 
small DRSs

This implements the small rowset compaction scheme explained in the
KUDU-1400 design doc [1]. While the implementation is simple, the
reasoning behind it is nuanced, and I'll defer the explanations of them
to the design doc rather than repeating them here.

The three relevant constant factors:

--compaction_minimum_improvement_score
--compaction_small_rowset_tradeoff
kSupportAdjust

have been tuned to work well with each other. See the relevant tests for
reasoning on why, and validation of what the results should be.

Since compaction policy's performance is important, I ran the YCSB
benchmark before and after the change using 'perf stat' to compare the
performance:

Command:

Before:

 Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1231.095442 task-clock#0.997 CPUs utilized   
 ( +-  2.00% )
   148 context-switches  #0.120 K/sec   
 ( +-  2.43% )
 9 cpu-migrations#0.007 K/sec   
 ( +- 16.14% )
 3,630 page-faults   #0.003 M/sec   
 ( +-  0.00% )
 3,251,530,478 cycles#2.641 GHz 
 ( +-  2.04% )
stalled-cycles-frontend
stalled-cycles-backend
 5,772,319,429 instructions  #1.78  insns per cycle 
 ( +-  0.01% )
 1,070,627,520 branches  #  869.654 M/sec   
 ( +-  0.01% )
13,583,368 branch-misses #1.27% of all branches 
 ( +-  0.10% )

   1.235037947 seconds time elapsed 
 ( +-  2.00% )

After:

Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1297.749333 task-clock#0.994 CPUs utilized   
 ( +-  2.17% )
   158 context-switches  #0.122 K/sec   
 ( +-  3.01% )
14 cpu-migrations#0.011 K/sec   
 ( +- 17.48% )
 3,636 page-faults   #0.003 M/sec
 3,509,480,140 cycles#2.704 GHz 
 ( +-  2.65% )
stalled-cycles-frontend
stalled-cycles-backend
 6,800,316,126 instructions  #1.94  insns per cycle 
 ( +-  0.01% )
 1,073,111,416 branches  #  826.902 M/sec   
 ( +-  0.01% )
13,574,780 branch-misses #1.26% of all branches 
 ( +-  0.15% )

   1.305058206 seconds time elapsed 
 ( +-  2.16% )

A follow up will integrate the below design doc with the existing one,
docs/design-docs/compaction-policy.md.

[1]: 
https://docs.google.com/document/d/1yTfxt0_2p5EfIjCnjJCt3o-nB9xk-Kl2O8yKTA1LQrQ/edit?usp=sharing

Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
4 files changed, 278 insertions(+), 37 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd 
Lipcon,

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

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

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

Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..

[compaction] KUDU-1400: Improve rowset compaction policy to consider merging 
small DRSs

This implements the small rowset compaction scheme explained in the
KUDU-1400 design doc [1]. While the implementation is simple, the
reasoning behind it is nuanced, and I'll defer the explanations of them
to the design doc rather than repeating them here.

The three relevant constant factors:

--compaction_minimum_improvement_score
--compaction_small_rowset_tradeoff
kSupportAdjust

have been tuned to work well with each other. See the relevant tests for
reasoning on why, and validation of what the results should be.

Since compaction policy's performance is important, I ran the YCSB
benchmark before and after the change using 'perf stat' to compare the
performance:

Command:

Before:

 Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1231.095442 task-clock#0.997 CPUs utilized   
 ( +-  2.00% )
   148 context-switches  #0.120 K/sec   
 ( +-  2.43% )
 9 cpu-migrations#0.007 K/sec   
 ( +- 16.14% )
 3,630 page-faults   #0.003 M/sec   
 ( +-  0.00% )
 3,251,530,478 cycles#2.641 GHz 
 ( +-  2.04% )
stalled-cycles-frontend
stalled-cycles-backend
 5,772,319,429 instructions  #1.78  insns per cycle 
 ( +-  0.01% )
 1,070,627,520 branches  #  869.654 M/sec   
 ( +-  0.01% )
13,583,368 branch-misses #1.27% of all branches 
 ( +-  0.10% )

   1.235037947 seconds time elapsed 
 ( +-  2.00% )

After:

Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1297.749333 task-clock#0.994 CPUs utilized   
 ( +-  2.17% )
   158 context-switches  #0.122 K/sec   
 ( +-  3.01% )
14 cpu-migrations#0.011 K/sec   
 ( +- 17.48% )
 3,636 page-faults   #0.003 M/sec
 3,509,480,140 cycles#2.704 GHz 
 ( +-  2.65% )
stalled-cycles-frontend
stalled-cycles-backend
 6,800,316,126 instructions  #1.94  insns per cycle 
 ( +-  0.01% )
 1,073,111,416 branches  #  826.902 M/sec   
 ( +-  0.01% )
13,574,780 branch-misses #1.26% of all branches 
 ( +-  0.15% )

   1.305058206 seconds time elapsed 
 ( +-  2.16% )

A follow up will integrate the below design doc with the existing one,
docs/design-docs/compaction-policy.md.

[1]: 
https://docs.google.com/document/d/1yTfxt0_2p5EfIjCnjJCt3o-nB9xk-Kl2O8yKTA1LQrQ/edit?usp=sharing

Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
4 files changed, 277 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/11869/2
--
To view, visit http://gerrit.cloudera.org:8080/11869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [test] optional table creation in TabletServerIntegrationTestBase

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11861 )

Change subject: [test] optional table creation in 
TabletServerIntegrationTestBase
..

[test] optional table creation in TabletServerIntegrationTestBase

Creation of the test table in TabletServerIntegrationTestBase is now
optional.  If not specified otherwise, the test table is created by
default, preserving the original behavior.

The motivation for this change is that sometimes a derived test
might need the functionality of the TabletServerIntegrationTestBase
but expects not a single table created after SetUp().

The updated behavior will be used in a follow-up patch.

Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Reviewed-on: http://gerrit.cloudera.org:8080/11861
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 24 insertions(+), 12 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11869


Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to 
consider merging small DRSs
..

[compaction] KUDU-1400: Improve rowset compaction policy to consider merging 
small DRSs

This implements the small rowset compaction scheme explained in the
KUDU-1400 design doc [1]. While the implementation is simple, the
reasoning behind it is nuanced, and I'll defer the explanations of them
to the design doc rather than repeating them here.

The three relevant constant factors:

--compaction_minimum_improvement_score
--compaction_small_rowset_tradeoff
kSupportAdjust

have been tuned to work well with each other. See the relevant tests for
reasoning on why, and validation of what the results should be.

Since compaction policy's performance is important, I ran the YCSB
benchmark before and after the change using 'perf stat' to compare the
performance:

Command:

Before:

 Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1231.095442 task-clock#0.997 CPUs utilized   
 ( +-  2.00% )
   148 context-switches  #0.120 K/sec   
 ( +-  2.43% )
 9 cpu-migrations#0.007 K/sec   
 ( +- 16.14% )
 3,630 page-faults   #0.003 M/sec   
 ( +-  0.00% )
 3,251,530,478 cycles#2.641 GHz 
 ( +-  2.04% )
stalled-cycles-frontend
stalled-cycles-backend
 5,772,319,429 instructions  #1.78  insns per cycle 
 ( +-  0.01% )
 1,070,627,520 branches  #  869.654 M/sec   
 ( +-  0.01% )
13,583,368 branch-misses #1.27% of all branches 
 ( +-  0.10% )

   1.235037947 seconds time elapsed 
 ( +-  2.00% )

After:

Performance counter stats for 'bin/compaction_policy-test 
--gtest_filter=*Ycsb*' (10 runs):

   1297.749333 task-clock#0.994 CPUs utilized   
 ( +-  2.17% )
   158 context-switches  #0.122 K/sec   
 ( +-  3.01% )
14 cpu-migrations#0.011 K/sec   
 ( +- 17.48% )
 3,636 page-faults   #0.003 M/sec
 3,509,480,140 cycles#2.704 GHz 
 ( +-  2.65% )
stalled-cycles-frontend
stalled-cycles-backend
 6,800,316,126 instructions  #1.94  insns per cycle 
 ( +-  0.01% )
 1,073,111,416 branches  #  826.902 M/sec   
 ( +-  0.01% )
13,574,780 branch-misses #1.26% of all branches 
 ( +-  0.15% )

   1.305058206 seconds time elapsed 
 ( +-  2.16% )

A follow up will integrate the below design doc with the existing one,
docs/design-docs/compaction-policy.md.

[1]: 
https://docs.google.com/document/d/1yTfxt0_2p5EfIjCnjJCt3o-nB9xk-Kl2O8yKTA1LQrQ/edit?usp=sharing

Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
4 files changed, 274 insertions(+), 36 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has removed a vote on this change.

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11865
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 2: Verified+1

Overriding Jenkins, dist-test failure KUDU-2432.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 21:06:44 +
Gerrit-HasComments: No


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..

move HashAlgorithm from common.proto to hash.proto

Commit 8af288a26 added a dependency from the bloom filter code onto common,
creating a circular dependency. Normally that's caught by cmake, but the
commit didn't actually establish the util->common dependency in cmake, which
meant that the underlying parallelism of the build (and whether it was a
clean or incremental build) dictated whether common.pb.h was present on-disk
at the time that bloom_filter.h was included.

This patch resolves the circular dependency by moving the HashAlgorithm enum
out of common.proto and into a new hash.proto within util. An alternative
would be to move the bloom filter code itself into common, but as it isn't
specific to Kudu, I think util is a better home for it.

Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Reviewed-on: http://gerrit.cloudera.org:8080/11865
Reviewed-by: Grant Henke 
Reviewed-by: Andrew Wong 
Tested-by: Adar Dembo 
---
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
M src/kudu/util/CMakeLists.txt
M src/kudu/util/bloom_filter.h
A src/kudu/util/hash.proto
5 files changed, 55 insertions(+), 19 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved
  Andrew Wong: Looks good to me, approved
  Adar Dembo: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11659 )

Change subject: [sentry] add AuthzProvider
..


Patch Set 13: Code-Review+2

LGTM as long as Alexey's content


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:59:59 +
Gerrit-HasComments: No


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:58:38 +
Gerrit-HasComments: No


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar 
Dembo,

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

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

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

Change subject: [sentry] add AuthzProvider
..

[sentry] add AuthzProvider

This commit adds a high-level abstraction which handles authorizations
on Kudu operations, called AuthzProvider. It has a default implementation
which always allow any operations for any users, and a SentryAuthzProvider
implementation which connects to the Sentry service for authorization
metadata. AuthzProvider, along with its implementations, is placed in the
master module. The idea is to decouple it from Sentry since in the future,
other authorization implementations might be introduced.

A follow-up commit will integrate the AuthzProvider into CatalogManager
to perform authorization checks on Master RPCs.

Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/authz_provider.h
A src/kudu/master/default_authz_provider.h
A src/kudu/master/sentry_authz_provider-test.cc
A src/kudu/master/sentry_authz_provider.cc
A src/kudu/master/sentry_authz_provider.h
A src/kudu/sentry/sentry-test-base.h
M src/kudu/sentry/sentry_action-test.cc
M src/kudu/sentry/sentry_action.cc
M src/kudu/sentry/sentry_action.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
14 files changed, 994 insertions(+), 117 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/11659/13
--
To view, visit http://gerrit.cloudera.org:8080/11659
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:57:54 +
Gerrit-HasComments: No


[kudu-CR] [examples] Add basic Spark example (scala)

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11788 )

Change subject: [examples] Add basic Spark example (scala)
..


Patch Set 12: Code-Review+1

LGTM


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 12
Gerrit-Owner: Mitch Barnett 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:32:14 +
Gerrit-HasComments: No


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Adar Dembo (Code Review)
Hello Kudu Jenkins, Andrew Wong, Grant Henke,

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

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

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

Change subject: move HashAlgorithm from common.proto to hash.proto
..

move HashAlgorithm from common.proto to hash.proto

Commit 8af288a26 added a dependency from the bloom filter code onto common,
creating a circular dependency. Normally that's caught by cmake, but the
commit didn't actually establish the util->common dependency in cmake, which
meant that the underlying parallelism of the build (and whether it was a
clean or incremental build) dictated whether common.pb.h was present on-disk
at the time that bloom_filter.h was included.

This patch resolves the circular dependency by moving the HashAlgorithm enum
out of common.proto and into a new hash.proto within util. An alternative
would be to move the bloom filter code itself into common, but as it isn't
specific to Kudu, I think util is a better home for it.

Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
---
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
M src/kudu/util/CMakeLists.txt
M src/kudu/util/bloom_filter.h
A src/kudu/util/hash.proto
5 files changed, 55 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/11865/2
--
To view, visit http://gerrit.cloudera.org:8080/11865
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] [examples] Add basic Spark example (scala)

2018-11-02 Thread Mitch Barnett (Code Review)
Mitch Barnett has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11788 )

Change subject: [examples] Add basic Spark example (scala)
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc
File examples/scala/spark-example/README.adoc:

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@34
PS11, Line 34: spark-example/
> remove
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 11
Gerrit-Owner: Mitch Barnett 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:12:35 +
Gerrit-HasComments: Yes


[kudu-CR] [examples] Add basic Spark example (scala)

2018-11-02 Thread Mitch Barnett (Code Review)
Hello Will Berkeley, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Greg 
Solovyev,

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

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

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

Change subject: [examples] Add basic Spark example (scala)
..

[examples] Add basic Spark example (scala)

This patch adds a basic Kudu-Spark example that utilizes the
Kudu-Spark integration.
It will allow users to pull down the pom.xml and scala source,
then build and execute from their local machine.

Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
---
A examples/scala/spark-example/README.adoc
A examples/scala/spark-example/pom.xml
A 
examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
3 files changed, 287 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 12
Gerrit-Owner: Mitch Barnett 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [examples] Add basic Spark example (scala)

2018-11-02 Thread Mitch Barnett (Code Review)
Mitch Barnett has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11788 )

Change subject: [examples] Add basic Spark example (scala)
..


Patch Set 11:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc
File examples/scala/spark-example/README.adoc:

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@32
PS11, Line 32: compile and
> remove
Done


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@33
PS11, Line 33: spark-example/
> `spark-example` instead.
Done


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@41
PS11, Line 41: There are a few Java system properties defined in 
SparkExample.scala:
 :
 : - kuduMasters: A comma-separated list of Kudu master addresses. 
Defaults to
 :   'localhost:7051'.
 : - tableName: The name of the table you wish to create on the 
Kudu cluster.
 :   Defaults to 'spark_test'.
 :
 : To run this as a spark2 'spark-submit' job, use the spark-submit 
command
 : as follows from the spark-example directory - it requires that 
you have built
 : and compiled the package using the 'mvn package' command above. 
You will also
 : need a Spark on YARN cluster and a Kudu cluster, both of which 
should be
 : resolvable and accessible from the host executing the command:
> I think we should redo this section a little bit. How do you like:
Done


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@57
PS11, Line 57: 
> What's the default? We want client mode so people will see the log messages
Yeah, the default is client. I've omitted this parameter.


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File 
examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@65
PS11, Line 65: defaul
> default
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 11
Gerrit-Owner: Mitch Barnett 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 20:11:01 +
Gerrit-HasComments: Yes


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11865/1/src/kudu/common/CMakeLists.txt
File src/kudu/common/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/11865/1/src/kudu/common/CMakeLists.txt@25
PS1, Line 25:   DEPS pb_util_proto protobuf util_compression_proto
Do you need a dep here?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 19:55:58 +
Gerrit-HasComments: Yes


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 1: -Code-Review

The release build failed with a related looking error:

```11:52:07 ../../../lib/libkudu_common_proto.so: undefined reference to 
`kudu::protobuf_kudu_2futil_2fhash_2eproto::InitDefaults()'
11:52:07 ../../../lib/libkudu_common_proto.so: undefined reference to 
`kudu::protobuf_kudu_2futil_2fhash_2eproto::AddDescriptors()'
11:52:07 ../../../lib/libkudu_common_proto.so: undefined reference to 
`kudu::HashAlgorithm_IsValid(int)'
11:52:07 collect2: error: ld returned 1 exit status```


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 19:52:51 +
Gerrit-HasComments: No


[kudu-CR] [examples] Add basic Spark example (scala)

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11788 )

Change subject: [examples] Add basic Spark example (scala)
..


Patch Set 11:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc
File examples/scala/spark-example/README.adoc:

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@32
PS11, Line 32: compile and
remove


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@33
PS11, Line 33: spark-example/
`spark-example` instead.


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@33
PS11, Line 33: a new
 : java executable
a Spark application jar


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@34
PS11, Line 34: target/
`target` instead


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@34
PS11, Line 34: within
in


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@34
PS11, Line 34: spark-example/
remove


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@41
PS11, Line 41: There are a few Java system properties defined in 
SparkExample.scala:
 :
 : - kuduMasters: A comma-separated list of Kudu master addresses. 
Defaults to
 :   'localhost:7051'.
 : - tableName: The name of the table you wish to create on the 
Kudu cluster.
 :   Defaults to 'spark_test'.
 :
 : To run this as a spark2 'spark-submit' job, use the spark-submit 
command
 : as follows from the spark-example directory - it requires that 
you have built
 : and compiled the package using the 'mvn package' command above. 
You will also
 : need a Spark on YARN cluster and a Kudu cluster, both of which 
should be
 : resolvable and accessible from the host executing the command:
I think we should redo this section a little bit. How do you like:

To configure the kudu-spark example, there are two Java system properties 
available:

- kuduMasters: a comma-separated list of Kudu master addresses. Default: 
localhost:7051.
- tableName: the name of the table to use for the example program. This table 
should not exist in Kudu. Defaults to 'spark_test'.

The application can be run using `spark-submit`. For example, to run the 
example against a Spark cluster running on YARN, use a command like the 
following:

(long command i ain't typin)

You will need the Kudu cluster to be up and running and Spark correctly 
configured for the example to work.


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/README.adoc@57
PS11, Line 57: 
What's the default? We want client mode so people will see the log messages 
showing the app running vs Kudu. If the default is client, just remove this 
param.


http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File 
examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:

http://gerrit.cloudera.org:8080/#/c/11788/11/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@65
PS11, Line 65: defaul
default



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 11
Gerrit-Owner: Mitch Barnett 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 19:05:55 +
Gerrit-HasComments: Yes


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11659 )

Change subject: [sentry] add AuthzProvider
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84
PS10, Line 84: u
> I mean look how Kudu RPC works -- you don't specify number of retries, you
Hmm, I think the scenario is bit different where here is the flag for Kudu as a 
Sentry client where the retry with exponential backoff 
(https://github.com/apache/kudu/blob/master/src/kudu/thrift/client.h#L236). And 
the timeouts are per retry.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 18:57:16 +
Gerrit-HasComments: Yes


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11865/1/src/kudu/common/CMakeLists.txt
File src/kudu/common/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/11865/1/src/kudu/common/CMakeLists.txt@72
PS1, Line 72:   gutil
Was this just reorganized to be alphabetical?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 18:54:09 +
Gerrit-HasComments: Yes


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11865 )

Change subject: move HashAlgorithm from common.proto to hash.proto
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 02 Nov 2018 18:53:12 +
Gerrit-HasComments: No


[kudu-CR] move HashAlgorithm from common.proto to hash.proto

2018-11-02 Thread Adar Dembo (Code Review)
Hello Andrew Wong, Grant Henke,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: move HashAlgorithm from common.proto to hash.proto
..

move HashAlgorithm from common.proto to hash.proto

Commit 8af288a26 added a dependency from the bloom filter code onto common,
creating a circular dependency. Normally that's caught by cmake, but the
commit didn't actually establish the util->common dependency in cmake, which
meant that the underlying parallelism of the build (and whether it was a
clean or incremental build) dictated whether common.pb.h was present on-disk
at the time that bloom_filter.h was included.

This patch resolves the circular dependency by moving the HashAlgorithm enum
out of common.proto and into a new hash.proto within util. An alternative
would be to move the bloom filter code itself into common, but as it isn't
specific to Kudu, I think util is a better home for it.

Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
---
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
M src/kudu/util/CMakeLists.txt
M src/kudu/util/bloom_filter.h
A src/kudu/util/hash.proto
5 files changed, 55 insertions(+), 18 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b0a4077335de75dfed6aa1c6d78d9e197250976
Gerrit-Change-Number: 11865
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] [rebalancer] location-aware rebalancer (part 9/n)

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11862 )

Change subject: [rebalancer] location-aware rebalancer (part 9/n)
..


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc
File src/kudu/tools/placement_policy_util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc@142
PS2, Line 142: default comparision
 : // operator
There's a default comparison operator in C++? I didn't think there was. How it 
should it work? I don't think we should use it, even in a test.

I think tuples have a comparison operator if their components do, though.


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/placement_policy_util-test.cc@143
PS2, Line 143: nonsence
nonsense


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc
File src/kudu/tools/rebalancer.cc:

http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@129
PS2, Line 129: iformation
information


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@137
PS2, Line 137: vilations
violations


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@138
PS2, Line 138: RETURN_NOT_OK(PrintPolicyViolationInfo(raw_info, out));
It looks like the output could be kind of long. Can we move placement policy 
violations to the bottom, where they're most likely to be seen when this is run 
from the command line. Alternatively, you can leave them at the top, but print 
an extra message at the bottom if there are placement policy violations, one 
directing users to look up here for details.


http://gerrit.cloudera.org:8080/#/c/11862/2/src/kudu/tools/rebalancer.cc@145
PS2, Line 145: sorated
sorted



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
Gerrit-Change-Number: 11862
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 18:41:59 +
Gerrit-HasComments: Yes


[kudu-CR] [test] optional table creation in TabletServerIntegrationTestBase

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11861 )

Change subject: [test] optional table creation in 
TabletServerIntegrationTestBase
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 18:35:55 +
Gerrit-HasComments: No


[kudu-CR] [rebalancer] location-aware rebalancer (part 9/n)

2018-11-02 Thread Alexey Serbin (Code Review)
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: [rebalancer] location-aware rebalancer (part 9/n)
..

[rebalancer] location-aware rebalancer (part 9/n)

Updated reporting functionality of the rebalancer tool to output
information on placement policy violations and other relevant
information for location-aware clusters.

Added one simple integration test as well.

Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
---
M src/kudu/tools/placement_policy_util-test.cc
M src/kudu/tools/placement_policy_util.cc
M src/kudu/tools/placement_policy_util.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
6 files changed, 455 insertions(+), 139 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11862/2
--
To view, visit http://gerrit.cloudera.org:8080/11862
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
Gerrit-Change-Number: 11862
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [test] optional table creation in TabletServerIntegrationTestBase

2018-11-02 Thread Alexey Serbin (Code Review)
Hello Will Berkeley, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: [test] optional table creation in 
TabletServerIntegrationTestBase
..

[test] optional table creation in TabletServerIntegrationTestBase

Creation of the test table in TabletServerIntegrationTestBase is now
optional.  If not specified otherwise, the test table is created by
default, preserving the original behavior.

The motivation for this change is that sometimes a derived test
might need the functionality of the TabletServerIntegrationTestBase
but expects not a single table created after SetUp().

The updated behavior will be used in a follow-up patch.

Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 24 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/11861/2
--
To view, visit http://gerrit.cloudera.org:8080/11861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Reviewed-on: http://gerrit.cloudera.org:8080/11827
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong 
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 291 insertions(+), 268 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 17:31:27 +
Gerrit-HasComments: No


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11659 )

Change subject: [sentry] add AuthzProvider
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84
PS10, Line 84: 1
> I think one time retry is general enough to avoid to end the connection too
I mean look how Kudu RPC works -- you don't specify number of retries, you 
specify just the timeout.  The client retries the call with exponential 
back-off (where the max wait is limited at about 2 seconds), and the timeout is 
the only parameter that controls those retries.

Also, those timeouts -- are those timeouts per every retry or just overall 
including all retries?  If the latter, how do you control which parameter 
actually controls the retry behavior: the overall timeout or maximum number of 
retries?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 17:15:08 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:33:57 +
Gerrit-HasComments: No


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar 
Dembo,

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

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

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

Change subject: [sentry] add AuthzProvider
..

[sentry] add AuthzProvider

This commit adds a high-level abstraction which handles authorizations
on Kudu operations, called AuthzProvider. It has a default implementation
which always allow any operations for any users, and a SentryAuthzProvider
implementation which connects to the Sentry service for authorization
metadata. AuthzProvider, along with its implementations, is placed in the
master module. The idea is to decouple it from Sentry since in the future,
other authorization implementations might be introduced.

A follow-up commit will integrate the AuthzProvider into CatalogManager
to perform authorization checks on Master RPCs.

Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/authz_provider.h
A src/kudu/master/default_authz_provider.h
A src/kudu/master/sentry_authz_provider-test.cc
A src/kudu/master/sentry_authz_provider.cc
A src/kudu/master/sentry_authz_provider.h
A src/kudu/sentry/sentry-test-base.h
M src/kudu/sentry/sentry_action-test.cc
M src/kudu/sentry/sentry_action.cc
M src/kudu/sentry/sentry_action.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
14 files changed, 994 insertions(+), 117 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)


[kudu-CR] [sentry] add AuthzProvider

2018-11-02 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11659 )

Change subject: [sentry] add AuthzProvider
..


Patch Set 12:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/authz_provider.h
File src/kudu/master/authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/authz_provider.h@63
PS11, Line 63:   // Checks if retrieving metadata about the table is authorized 
for the
> nit: "metadata" seems like it might be a Sentry construct, which is fine, b
Done


http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.h
File src/kudu/master/sentry_authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.h@39
PS11, Line 39:
 : // An implementation of AuthzProvider that connects to the 
Sentry Service
 : // for authorization metadata and allow or deny the actions 
performed by
 : // users based on the metadata.
> nit: "An implementation of AuthzProvider that connects to Apache Sentry for
I would prefer to not use Apache Sentry here in order to match how we refer 
'Sentry' in other places.


http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.cc@246
PS11, Line 246:   return Authorize(db_authorizable, db_action, user);
  : }
  :
  : Status SentryAuthzProvider::AuthorizeGetTableMetadata(const 
std::string& table_name,
  :   const 
std::string& user) {
  :   // Retrieving table metadata requires 'METADATA ON TABLE' 
privilege.
  :   TSentryAuthorizable authorizable;
  :   RETURN_NOT_OK(GetAuthorizable(table_name, 
AuthorizableScope::TABLE, ));
  :   SentryAction action = 
SentryAction(SentryAction::Action::METADATA);
  :   return Authorize(authorizable, action, user);
  : }
  :
> Now that this is tested elsewhere, it doesn't need to be part of the class.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:24:07 +
Gerrit-HasComments: Yes


[kudu-CR] (05/05) delta applier: support for diff scan style iteration

2018-11-02 Thread Adar Dembo (Code Review)
Hello Mike Percy, David Ribeiro Alves, Kudu Jenkins, Grant Henke, Todd Lipcon,

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

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

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

Change subject: (05/05) delta_applier: support for diff scan style iteration
..

(05/05) delta_applier: support for diff scan style iteration

This patch incorporates all of the previous work to provide diff scan
support for an entire DiskRowSet. There's a new "fuzzy" test in
diskrowset-test that exercises this functionality.

Change-Id: I895350a3c7728df88a87a36b904ffdea4f3f6b7a
---
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_applier.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset-test.cc
5 files changed, 286 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895350a3c7728df88a87a36b904ffdea4f3f6b7a
Gerrit-Change-Number: 11860
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] (04/05) delta store: support iteration with is deleted virtual column

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11859 )

Change subject: (04/05) delta_store: support iteration with is_deleted virtual 
column
..


Patch Set 2: Verified+1

RELEASE build hung in a Python test. I posted 
http://gerrit.cloudera.org:8080/11863 to address that.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibafcae9f9f82a6c2d9a8afedb8fbf07d6e3069ec
Gerrit-Change-Number: 11859
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:10:30 +
Gerrit-HasComments: No


[kudu-CR] python: make test scantoken more robust to exceptions

2018-11-02 Thread Adar Dembo (Code Review)
Hello Jordan Birdsell, Andrew Wong,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: python: make test_scantoken more robust to exceptions
..

python: make test_scantoken more robust to exceptions

One of my precommit tests failed with:

  22:49:05 kudu/tests/test_client.py .
  22:49:06 kudu/tests/test_scanner.py 
  22:49:08 kudu/tests/test_scantoken.py ..Build timed out (after 50 minutes). 
Marking the build as failed.
  23:39:08 Build was aborted

I don't know why this happened, but looking at commit 1d928951e, it seems
like if pool.map() raised an exception, we wouldn't close or join the pool,
which would lead to the same test hanging behavior as before. I don't know
how or why pool.map() raised an exception, but this will hopefully surface
the actual root cause.

Change-Id: I9b5a70d6b5f115cc4de740ded6ecc48574b9c707
---
M python/kudu/tests/test_scantoken.py
1 file changed, 5 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9b5a70d6b5f115cc4de740ded6ecc48574b9c707
Gerrit-Change-Number: 11863
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Jordan Birdsell 


[kudu-CR] (04/05) delta store: support iteration with is deleted virtual column

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/11859 )

Change subject: (04/05) delta_store: support iteration with is_deleted virtual 
column
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11859
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ibafcae9f9f82a6c2d9a8afedb8fbf07d6e3069ec
Gerrit-Change-Number: 11859
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11827 )

Change subject: [compaction] Cleanup of compaction policy code
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h
File src/kudu/tablet/compaction_policy.h:

http://gerrit.cloudera.org:8080/#/c/11827/3/src/kudu/tablet/compaction_policy.h@66
PS3, Line 66: std::numeric_limits::max()
> Ah, I see.  Would it be a better choice making this method pure virtual?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:07:26 +
Gerrit-HasComments: Yes


[kudu-CR] [compaction] Cleanup of compaction policy code

2018-11-02 Thread Will Berkeley (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

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

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

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

Change subject: [compaction] Cleanup of compaction policy code
..

[compaction] Cleanup of compaction policy code

I'm reading over the compaction policy code before starting to work in
earnest on KUDU-1400, and I made a number of small changes that I think
help make the code more readable.

This patch contains no functional changes.

Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/compaction_policy.h
M src/kudu/tablet/svg_dump.cc
M src/kudu/tablet/svg_dump.h
M src/kudu/tablet/tablet.cc
6 files changed, 291 insertions(+), 268 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/11827/6
--
To view, visit http://gerrit.cloudera.org:8080/11827
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I955ec66a510f04fd869f7d969a643e4d0f7f415f
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] (02/05) delta store: convert DeltaIterator::PrepareBatch flags into bitfield

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11857 )

Change subject: (02/05) delta_store: convert DeltaIterator::PrepareBatch flags 
into bitfield
..


Patch Set 2: Verified+1

Overriding Jenkins, flaky TSAN test failure in DefaultSourceTest.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id500ec3cc9459a78ae1098be3f3cd0cacb7a7a1a
Gerrit-Change-Number: 11857
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Nov 2018 15:50:01 +
Gerrit-HasComments: No


[kudu-CR] (02/05) delta store: convert DeltaIterator::PrepareBatch flags into bitfield

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/11857 )

Change subject: (02/05) delta_store: convert DeltaIterator::PrepareBatch flags 
into bitfield
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11857
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Id500ec3cc9459a78ae1098be3f3cd0cacb7a7a1a
Gerrit-Change-Number: 11857
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [test] optional table creation in TabletServerIntegrationTestBase

2018-11-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11861 )

Change subject: [test] optional table creation in 
TabletServerIntegrationTestBase
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11861/1/src/kudu/integration-tests/ts_itest-base.cc
File src/kudu/integration-tests/ts_itest-base.cc:

http://gerrit.cloudera.org:8080/#/c/11861/1/src/kudu/integration-tests/ts_itest-base.cc@550
PS1, Line 550:   if (create_table) {
 : NO_FATALS(CreateTable());
 :   }
 :   if (create_table) {
 : WaitForTSAndReplicas();
 : ASSERT_FALSE(tablet_replicas_.empty());
 : tablet_id_ = (*tablet_replicas_.begin()).first;
 :   } else {
 : WaitForTabletServers();
 :   }
Combine?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 02 Nov 2018 15:49:18 +
Gerrit-HasComments: Yes


[kudu-CR] [test] optional table creation in TabletServerIntegrationTestBase

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11861


Change subject: [test] optional table creation in 
TabletServerIntegrationTestBase
..

[test] optional table creation in TabletServerIntegrationTestBase

Creation of the test table in TabletServerIntegrationTestBase is now
optional.  If not specified otherwise, the test table is created by
default, preserving the original behavior.

The motivation for this change is that sometimes a derived test
might need the functionality of the TabletServerIntegrationTestBase
but expects not a single table created after SetUp().

The updated behavior will be used in a follow-up patch.

Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 26 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I818a2eaf76fdee11ae792c27eddae311b775efeb
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 


[kudu-CR] [rebalancer] location-aware rebalancer (part 9/n)

2018-11-02 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11862


Change subject: [rebalancer] location-aware rebalancer (part 9/n)
..

[rebalancer] location-aware rebalancer (part 9/n)

Updated reporting functionality of the rebalancer tool to output
information on placement policy violations and other relevant
information for location-aware clusters.

Added one simple integration test as well.

Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
---
M src/kudu/tools/placement_policy_util.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
4 files changed, 432 insertions(+), 104 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8407e9f8cf6b41a6aeb075372d852125d9739e08
Gerrit-Change-Number: 11862
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin