date:20160719

Kudu Jenkins has posted comments on this change.

Change subject: Doxygen for C++ client API
..


Patch Set 12:

Build Started http://104.196.14.100/job/kudu-gerrit/2566/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: No

[kudu-CR] Doxygen for C++ client API

Hello Kudu Jenkins,

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

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

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

Change subject: Doxygen for C++ client API
..

Doxygen for C++ client API

If doxygen is available, build 'doxygen' taget to generate
Doxygen docs from client.h and other Kudu C++ client API files,
After the target is built, open
${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen/client_api/html/index.html
in your favorite browser to see the generated documentation.

If doxygen is available, the 'docs' target generates
doxygen documentaion as well since it depends on the 'doxygen' target.

Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
---
M CMakeLists.txt
A docs/support/doxygen/client_api.doxy.in
M src/kudu/client/client.h
M src/kudu/util/CMakeLists.txt
M thirdparty/download-thirdparty.sh
5 files changed, 1,294 insertions(+), 721 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones

[kudu-CR] Doxygen for C++ client API

Alexey Serbin has posted comments on this change.

Change subject: Doxygen for C++ client API
..


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3619/9/src/kudu/client/client.h
File src/kudu/client/client.h:

> 1. OK, then I think reverting back to one space makes sense.
NP: the crucial part is that we've got some progress :)


Line 432: class KUDU_EXPORT KuduTableCreator {
> I'm referring to how, in some of the old comments here, there were one-word
Oh, I see.  Yes, I also think it's nice to have that information.  I'm about to 
add that as preconditions for appropriate objects.  Please stay tuned -- will 
post with next revision of the patch.


http://gerrit.cloudera.org:8080/#/c/3619/11/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 767:   ///
> Nit: operation
Done


Line 1079:   /// flushes itself inline.
> Will this also include the "Flush any pending writes" sentence from Flush()
Good catch -- yes, that should be fixed.  Done.


Line 1332:   /// @note This method is unstable, and for internal use only.
> Nit: begin scanning
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

Adar Dembo has uploaded a new patch set (#10).

Change subject: KUDU-1358 (part 3): new multi-master stress test
..

KUDU-1358 (part 3): new multi-master stress test

This commit adds a stress test for multiple masters. The idea is simple:
issue DDL operations at a high rate while periodically restarting a master.

There's a balance to be struck both in the throughput of the operations and
in the periodicity of the restarts; we need to ensure that the masters can
make enough forward progress (in spite of the failures) to process all of
the requests without timing out. To assist, the client uses abnormally long
timeouts on all operations.

Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/master-stress-test.cc
2 files changed, 422 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] master: do not delete unknown tablets

Kudu Jenkins has posted comments on this change.

Change subject: master: do not delete unknown tablets
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/2560/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I331f2d5bb06c38daa7b09854dbb24a7881723551
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
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] master: do not delete unknown tablets

Adar Dembo has uploaded a new patch set (#6).

Change subject: master: do not delete unknown tablets
..

master: do not delete unknown tablets

Quoting from docs/design-docs/multi-master-1.0.md:

"The master and/or tserver must enforce that all actions take effect
iff they were sent by the master that is currently the leader.

After an exhaustive audit of all master state changes (see appendix A), it
was determined that the current protection mechanisms built into each RPC
are sufficient to provide fencing. The one exception is orphaned replica
deletion done in response to a heartbeat. To protect against that, true
orphans (i.e. tablets for which no persistent record exists) will not be
deleted at all. As the master retains deleted table/tablet metadata in
perpetuity, this should ensure that true orphans appear only under drastic
circumstances, such as a tserver that heartbeats to the wrong cluster."

The new test isn't ideal in that it must wait some time to allow the tserver
to receive an RPC from the master, but on my laptop it does fail without the
fix, and it should fail fairly often in other machines/environments too.

Change-Id: I331f2d5bb06c38daa7b09854dbb24a7881723551
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/master/catalog_manager.cc
3 files changed, 77 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I331f2d5bb06c38daa7b09854dbb24a7881723551
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 10:

Build Started http://104.196.14.100/job/kudu-gerrit/2564/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 10:

Build Started http://104.196.14.100/job/kudu-gerrit/2563/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 10:

Build Started http://104.196.14.100/job/kudu-gerrit/2562/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

Hello Dan Burkert,

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

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

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

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..

KUDU-1358 (part 1): master should accept heartbeat even if follower

This patch changes the master's heartbeat acceptance code so that heartbeats
are not rejected outright if the master is a follower. To be specific,
tablet reports are ignored, but heartbeats are processed just enough to warm
the TSDescriptor cache. That way, if this master is elected leader, it can
respond to a CreateTable() even before the first round of heartbeats.

An unfortunate side effect is that the "should this tserver register or send
full tablet report?" dance is now more complicated; I tried to cover all
cases in the modified unit test.

I also snuck in a fix to TSManager::RegisterTS: it wasn't actually returning
a TSDescriptor in its out parameter.

Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
---
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_descriptor.h
M src/kudu/master/ts_manager.cc
12 files changed, 251 insertions(+), 94 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/3609/10
-- 
To view, visit http://gerrit.cloudera.org:8080/3609
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] master: additional leader lock assertions in catalog manager

Adar Dembo has uploaded a new change for review.

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

Change subject: master: additional leader lock assertions in catalog manager
..

master: additional leader lock assertions in catalog manager

I went through the catalog manager entry points and added leader lock
assertions where necessary, updating tests as needed.

I also snuck in a couple cluster fixes:
1. MiniCluster::leader_mini_master() was unsafe because it didn't pass the
   (now held) leader lock back to the caller. It's only used in a few places
   though, so I removed it outright rather than fix it.
2. WaitForTabletServerCount() has been updated for both kinds of clusters.
   The new version waits for the correct count on every master, a necessary
   change now that tservers heartbeat to every master. Without this, we may
   stop waiting when the master that has seen all tservers was a follower
   and fail a subsequent CreateTable. The new version also ignore masters
   that have been shut down. This isn't essential, but good future-proofing.

Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-path-handlers.cc
10 files changed, 146 insertions(+), 105 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

Adar Dembo has uploaded a new patch set (#6).

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..

KUDU-1374: send full tablet report when new leader master is detected

This should help prevent missed tablet reports in very specific edge cases,
detailed in the bug report.

The new integration test fails some fraction of the time without change, and
passes 100% of the time with it.

Change-Id: Ic16fc46736476dba39616e79ecfe79eee48b3d7f
---
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/tserver/heartbeater.cc
3 files changed, 109 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic16fc46736476dba39616e79ecfe79eee48b3d7f
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] master: additional leader lock assertions in catalog manager

Kudu Jenkins has posted comments on this change.

Change subject: master: additional leader lock assertions in catalog manager
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2565/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/2561/

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

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

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

Adar Dembo has uploaded a new patch set (#10).

Change subject: KUDU-1358 (part 2): heartbeat to every master
..

KUDU-1358 (part 2): heartbeat to every master

Now that followers accept heartbeats, let's modify the tserver to send one
to every master. Spawning a heartbeater thread for each master seemed like
the natural way to do this; it should simplify dynamic master changes in the
future (i.e. just add or remove threads as needed).

The "dirty tablet" state is now encapsulated in the heartbeater threads
themselves, and the heartbeater must "fan out" to manipulate all of it. It's
a little noisy but I think it's reasonable. The alternative is for this
state to remain in the TSTabletManager, for the heartbeater to continue
tracking which master is the leader, and for it to only send tablet reports
to that master. This can be done with a few changes (e.g. adding term
numbers to the heartbeat response), but the only benefit is reduced network
traffic when tablets are dirty, so that didn't seem worth the complexity.

There's no new test here, but this code path is exercised in the test I
reenabled, and in the new stress test (follow-on patch).

Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
---
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/heartbeater.h
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
7 files changed, 311 insertions(+), 263 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

Adar Dembo has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3643/5//COMMIT_MSG
Commit Message:

Line 13: there was a way to mock a server-side krpc component as easily as it 
is to
> what about injecting a master crash in the AsyncSendFoo() method? that woul
Unfortunately, the code path that results in ts_proxy.FooAsync() is synchronous 
w.r.t. handling the heartbeats. So if we're going to induce a crash, it has to 
be somewhere in the RPC layer. Or we have to force the first RPC attempt to 
fail, then crash in the retry which is asynchronous.

I banged my head against the wall for a while here but I did produce a test 
that fails 4/10 times without the new heuristic. Take a look and let me know if 
you see a way to simplify it, or to make it fail more reliably.


Line 15: multi-master tests, though.
> is there a test you can loop before/after to verify this?
Done


http://gerrit.cloudera.org:8080/#/c/3643/5/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 175:   // Indicates that the thread should set a full tablet report. Set 
when
> s/set/send/
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic16fc46736476dba39616e79ecfe79eee48b3d7f
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] tablet peer: update status message on failure to start

Jean-Daniel Cryans has posted comments on this change.

Change subject: tablet_peer: update status message on failure to start
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4: -Verified

Actually, holding off the +1 so that we don't push this patch before the other 
patches.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4: Verified+1

ClientStressTest.TestStartScans is flaky.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-HasComments: No

[kudu-CR] [java client] Integrate with the replay cache

Hello Kudu Jenkins,

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

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

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

Change subject: [java client] Integrate with the replay cache
..

[java client] Integrate with the replay cache

This patch adds the required functionality to have the Java client use the
server-side replay cache.

Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
A java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java
A java/kudu-client/src/test/java/org/kududb/client/TestRequestTracker.java
11 files changed, 214 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Integrate with the replay cache

Kudu Jenkins has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/2559/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Integrate with the replay cache

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3631/5/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 257:   private final String clientId;
> looks like this isn't used?
Done


http://gerrit.cloudera.org:8080/#/c/3631/5/java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java
File java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java:

Line 64: return incompleteRpcs.peek();
> peek will return null if the queue is empty, so you can get rid of the isEm
Yeah it's just a question of timing, doing a peek then checking for null has 
the same issue, it might have already changed by the time you're checking... 
But your suggestion makes sense and saves a call!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] Re-enable multi-master tests

Adar Dembo has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1416 Upsert support for Flume sink

Mike Percy has posted comments on this change.

Change subject: KUDU-1416 Upsert support for Flume sink
..


Patch Set 1:

Yeah, just needs another rev

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Ara Ebrahimi 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/2558/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

Jean-Daniel Cryans has uploaded a new patch set (#4).

Change subject: [java-client] Re-enable multi-master tests
..

[java-client] Re-enable multi-master tests

This patch makes TestMasterFailover useful again. It also adds the killing of 
masters
to ITClient. Finally, it sets the raft heartbeat lower so that we don't wait 
1.5s for
leader elections.

TestGetMasterRegistrationReceived was added since a previous version of this 
patch
encountered a bug that such a simple unit test can detect.

Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
---
M 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java
A 
java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java
M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java
6 files changed, 233 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/3654/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans

[kudu-CR] [java-client] Re-enable multi-master tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
File 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java:

PS3, Line 109: and
 : // there's as many of them as responses received.
> This much is obvious from the code itself, but what's not obvious is the si
Good idea.


http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java:

Line 358: waitForAllTabletServers();
> I've been working on a change that'll allow ListTabletServers to be called 
I had a stale server-side compilation, with your new code it works. Sorry!


http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java
File 
java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java:

Line 91: // Permutation of the previous
> Nit: terminate with a period. Below too.
Done


Line 162: GetMasterRegistrationReceived grrm = new 
GetMasterRegistrationReceived(MASTERS, d);
> Nice variable name.
Thanks! :D


Line 177:   d.join(1000); // Don't care about the response.
> Is there any significance to the timeout value here? Could we call join() w
I forgot to remove it after adding a timeout on the test itself.


Line 188:   // Helper method that determines if the callback or errback should 
be called.
> This one and makeGMRR() can be static.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-HasComments: Yes

[kudu-CR] More disk reservation-itest flaky test workarounds

Mike Percy has posted comments on this change.

Change subject: More disk_reservation-itest flaky test workarounds
..


Patch Set 1: Code-Review+2 Verified+1

Merging this myself since I'm trying to get the flaky tests to stop complaining 
on AWS

I looped this 150x on my local box while burning up the CPUs and saw no 
failures.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] More disk reservation-itest flaky test workarounds

Mike Percy has submitted this change and it was merged.

Change subject: More disk_reservation-itest flaky test workarounds
..


More disk_reservation-itest flaky test workarounds

Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
Reviewed-on: http://gerrit.cloudera.org:8080/3683
Reviewed-by: Mike Percy 
Tested-by: Mike Percy 
---
M src/kudu/integration-tests/disk_reservation-itest.cc
1 file changed, 14 insertions(+), 3 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] More disk reservation-itest flaky test workarounds

Mike Percy has uploaded a new change for review.

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

Change subject: More disk_reservation-itest flaky test workarounds
..

More disk_reservation-itest flaky test workarounds

Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
---
M src/kudu/integration-tests/disk_reservation-itest.cc
1 file changed, 14 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy

[kudu-CR] More disk reservation-itest flaky test workarounds

Kudu Jenkins has posted comments on this change.

Change subject: More disk_reservation-itest flaky test workarounds
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2557/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad76fc32a2e49b32089230e6ee5b997b53af7b66
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] master: do not delete unknown tablets

Adar Dembo has posted comments on this change.

Change subject: master: do not delete unknown tablets
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3645/5//COMMIT_MSG
Commit Message:

Line 7: master: do not delete unknown tablets
> I wonder if we now need some tool to "reformat" a tablet server? or to inse
I'm almost positive that a user will one day blow away their master metadata 
directory and ask us to fix it. IIRC this happened with HDFS in the CDH3 days. 
The aggregate of all  tablet metadata should be sufficient to reconstruct the 
master metadata, provided all of the tablets are sufficiently replicated. But, 
I don't know if it makes sense to spend any time on this problem now, given 
that no one has run into this and our other priorities.

Reformatting a tserver is pretty easy: just stop it, delete its wal/data 
directories, and restart it. Or did you mean something else?

I don't really see a use case for dummy "deleted table/tablet" records, apart 
from the aforementioned recovery case, at which point these aren't "dummy" 
records. But, deleting orphaned tablets to reclaim space is a legit use case. 
The minimal fix is to add a gflag that defaults to off and grants the master 
permission to delete orphaned tablets. I'll do that here.

Beyond that, let me know what JIRAs I should file. :)


Line 9: Quoting from the multi-master design doc:
> nit: can you provide a link or source code path here?
Done


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

PS5, Line 1546: INFO)
> WARNING might be more appropriate here? or perhaps INFO if it's an incremen
I changed it to WARNING. It's hard to know in this context whether it's an 
incremental report or not, and besides, an incremental report only includes a 
tablet if its state has changed in some way.


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

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

[kudu-CR] Doxygen for C++ client API

Adar Dembo has posted comments on this change.

Change subject: Doxygen for C++ client API
..


Patch Set 11:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3619/9/src/kudu/client/client.h
File src/kudu/client/client.h:

> 1.  It's a purely stylistic change, it's not related to doxygen requirement
1. OK, then I think reverting back to one space makes sense.
2. Thanks, that makes sense.
3. Cool.

Crap, I just realized I didn't publish this after I looked at PS10. Now I 
understand why you hadn't changed anything based on #1 until we talked on 
HipChat. My mistake.


Line 432: 
> I don't understand what "Required" and "Optional" tags are in this context.
I'm referring to how, in some of the old comments here, there were one-word 
sentences like "Required." or "Optional." that help the user understand which 
builder methods they must call and which they can omit.


http://gerrit.cloudera.org:8080/#/c/3619/11/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 767:   /// the opration is complete. Otherwise, every operation returns 
immediately.
Nit: operation


Line 1079:   /// @copydoc KuduSession::Flush()
Will this also include the "Flush any pending writes" sentence from Flush()? If 
so, that would be kind of weird.


Line 1332:   /// @return Result status of the operation (beging scanning).
Nit: begin scanning


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..


Patch Set 2:

here's some example output on a cluster with a messed up table:
https://gist.github.com/697f2970c4fbaf5f5888b6864d628968

I think there's some more improvements to be made, like distinguishing between 
an under-replicated-but-available tablet vs an under-replicated-below-majority 
tablet.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] tablet peer: update status message on failure to start

Kudu Jenkins has posted comments on this change.

Change subject: tablet_peer: update status message on failure to start
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2556/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] tablet peer: update status message on failure to start

Hello Jean-Daniel Cryans, Mike Percy, Will Berkeley,

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

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

to review the following change.

Change subject: tablet_peer: update status message on failure to start
..

tablet_peer: update status message on failure to start

If the tablet peer fails to start up, we were calling SetFailed(),
but this didn't actually update the status message which would
later be reported as part of the TabletStatusPB. This made for
confusing debug experiences. This now surfaces the error.

Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
---
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
4 files changed, 22 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

Hello Jean-Daniel Cryans, Mike Percy, Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..

KUDU-1516 ksck should check for more raft-related status issues (partial)

This patch improves ksck. The main way it does so is by adding "tablet
server POV" information. ksck now gathers information about tablet
replicas from the tablet servers and cross-references this information
with the master metadata. This adds the following checks:

* each tablet has a majority of replicas on live tablet servers
* if a tablet has a majority of replicas on a live tablet
  server, then a majority of its tablets are in RUNNING state
* the assignments of tablets to tablet servers in the master agrees with
  the assignment of tablet replicas reported by the tablet servers

This patch does not include other desiderata from KUDU-1516, like
a consensus canary or a write op canary.

The code is also restructured quite a bit, so that all of the "fetch
information from tablet servers" work happens up front in a single
call. This paves the way a bit for a future enhancement in which
all of these RPCs are done on a thread-pool (since it can be
somewhat slow for large clusters).

To try to improve performance for clusters with a lot of data, I
also added a flag to the ListTablets RPC so that the response does
not include schema information, which is both large and irrelevant
for this use case.

This patch is based on some earlier work by Will Berkeley.

Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/master/master.proto
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/kudu-ksck.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver.proto
12 files changed, 354 insertions(+), 121 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2555/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Adar Dembo has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 8:

(2 comments)

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

Line 1345:   }
> Can't these vectors be of crefs instead? I just don't think any other metho
I think you missed this from earlier (as did I when I gave you a +2).


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

Line 1417:   bool has_changes = false;
This can now be removed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3611/9/src/kudu/integration-tests/master-stress-test.cc
File src/kudu/integration-tests/master-stress-test.cc:

PS9, Line 73: tT
> admin?
Done


Line 219: // TODO: Should be fixed with Exactly Once semantics.
> would be nice to tie these to JIRAs
Filed KUDU-1537.


Line 223:   num_altered++;
> why not get rid of these local variables and just IncrementBy(1) on the glo
At one point these were tight loops and I thought that'd be unnecessary 
contention on a shared resource. But that's no longer the case (and probably 
silly to begin with).


Line 323:   SleepFor(MonoDelta::FromMilliseconds(100));
> would be good to randomize the sleeps, so you might catch some slightly dif
Just for RestartMasterLoop(), right? Since the rest are interleaved by virtue 
of using multiple threads? Done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 8:

Build Started http://104.196.14.100/job/kudu-gerrit/2554/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..

KUDU-1311 [master] support adding and dropping range partitions

Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/common/partition.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/ksck.cc
19 files changed, 1,169 insertions(+), 190 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 164:   mutable std::atomic_int next_report_seq_;
> hrm, this really has to be mutable?
For the tablet report generation functions, yes. But as discussed below, it's 
probably less confusing to mark them non-const and remove this.


Line 522: TabletReportState state;
> maybe use = { seqno }; here? that way you'll get a warning at some point la
Done


PS9, Line 528: const 
> maybe makes more sense for this to not be 'const' since it changes the sequ
I agree that mutable and const here is kind of "cheating". Will remove both.


http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.h
File src/kudu/tserver/heartbeater.h:

Line 57:   // not dirty once the report has been acknowledged by every master.
> this makes it sound like it will keep reporting as "dirty" to all masters u
Yeah. I'll clarify the comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 9:

(1 comment)

> (1 comment)
 > 
 > Any way to system test this, like calling ListTabletServers against
 > a follower master? (or visiting its /tablet-servers web page?)

I added a new test in master_replication-itest. Let me know if it doesn't fit 
the bill.

http://gerrit.cloudera.org:8080/#/c/3609/9/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

PS9, Line 366:   };
> DISALLOW_COPY_AND_ASSIGN
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] C++ client: fix on KuduSession::GetPendingErrors()

Todd Lipcon has posted comments on this change.

Change subject: C++ client: fix on KuduSession::GetPendingErrors()
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafee46a6135ac2096d1f8b690da5b084a15eb73e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] C++ client: fix on KuduSession::GetPendingErrors()

Kudu Jenkins has posted comments on this change.

Change subject: C++ client: fix on KuduSession::GetPendingErrors()
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2553/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafee46a6135ac2096d1f8b690da5b084a15eb73e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] C++ client: fix on KuduSession::GetPendingErrors()

Alexey Serbin has uploaded a new change for review.

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

Change subject: C++ client: fix on KuduSession::GetPendingErrors()
..

C++ client: fix on KuduSession::GetPendingErrors()

If re-using the same std::vector instance as a placeholder
for repeated calls to KuduSession::GetPendingErrors(), irrelevant
error information can be pushed into the session error container.
Clean the placeholder container before swapping it with current
error container of the KuduSession.

Also, make it possible to pass NULL for the 'overflowed' out parameter.

Change-Id: Iafee46a6135ac2096d1f8b690da5b084a15eb73e
---
M src/kudu/client/error_collector.cc
1 file changed, 7 insertions(+), 2 deletions(-)


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

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

[kudu-CR] [java client] Integrate with the replay cache

Dan Burkert has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3631/5/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 257:   private final String clientId;
looks like this isn't used?


http://gerrit.cloudera.org:8080/#/c/3631/5/java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java
File java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java:

Line 64: return incompleteRpcs.peek();
peek will return null if the queue is empty, so you can get rid of the isEmpty 
call.  It also closes a potential TOCTOU (although I haven't though through if 
it actually matters).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add time/watermark based garbage collection to ResultTracker

Alexey Serbin has posted comments on this change.

Change subject: Add time/watermark based garbage collection to ResultTracker
..


Patch Set 13:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/3628/13/src/kudu/rpc/exactly_once_rpc-test.cc
File src/kudu/rpc/exactly_once_rpc-test.cc:

Line 64: } // namespace
nit: anonymous namespace, otherwise it looks like namespace name is missing.


Line 94: RetriableRpcStatus status;
nit: is it possible to make status local variable for related scopes only?  It 
does not look like the status variable needs to go though all the scopes to 
accumulate some changes on its initial value.


Line 243:   SleepFor(MonoDelta::FromMilliseconds(rand() % 10));
Do we want these pseudo-random numbers to repeat in the same sequence for every 
run of the test?  If not, consider initializing random with seed, i.e. calling 
srandom(time(nullptr)) or something like that before the while() cycle.


Line 257:   rand() % (2 * FLAGS_remember_responses_ttl_msecs)));
Ditto for the rand().


Line 259:   SleepFor(MonoDelta::FromMilliseconds(rand() % 10));
Ditto for the rand().


Line 270:   // Stubbornly sends the same request to the server, this should 
observe three states
Nit: period is missing in the end of the sentence.


Line 271:   // The request should be successful at first, then its result 
should be gc'd and the the
typo: strayed extra 'the'.


Line 297: SleepFor(MonoDelta::FromMilliseconds(rand() % 10));
Ditto for the rand().


Line 513: // This test creates a thread continuously makes requests to the 
server, some lasting longer
nit: makes --> making


http://gerrit.cloudera.org:8080/#/c/3628/13/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

Line 262: // The timestamp of the last time this CompletionRecord was 
updated.
nit: 'The timestamp of the last CompletionRecord update.'  ?


Line 301: SequenceNumber stale_before_seq_no;
Is it OK to leave it uninitialized in the constructor (I didn't see it was 
initialized).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java client] Redo how we manage exceptions

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [java client] Redo how we manage exceptions
..

[java client] Redo how we manage exceptions

Right now the exceptions are hard to handle in the Java client. They're all
generic and you need to do a lot of introspection. For example, if you try
to create a table that already exists, you need to start searching the
exception's message to know if it's that or some other problem that gave
you the error.

With this patch we now only one main kind of public exception: KuduException.
We still have Recoverable/NonRecoverableException but those are now
package-private and only used internally. PleaseThrottleException is kept public
for the async API.

KuduException has a new field, `status`, which is your regular Kudu Status
object. Wherever we can we try to recreate the Status objects that are sent
to us from the servers, else we create our own. Now for the example above we
can just query the exception's status with `isNotFound()`.

The sync APIs is also modified to only throw KuduExceptions instead of plain
Exceptions.

Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
D java/kudu-client/src/main/java/org/kududb/client/ConnectionResetException.java
M 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
D java/kudu-client/src/main/java/org/kududb/client/InvalidResponseException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/KuduException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java
M java/kudu-client/src/main/java/org/kududb/client/KuduScanner.java
D java/kudu-client/src/main/java/org/kududb/client/KuduServerException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduSession.java
D java/kudu-client/src/main/java/org/kududb/client/MasterErrorException.java
M 
java/kudu-client/src/main/java/org/kududb/client/NoLeaderMasterFoundException.java
M java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java
M java/kudu-client/src/main/java/org/kududb/client/NonRecoverableException.java
M java/kudu-client/src/main/java/org/kududb/client/PleaseThrottleException.java
M java/kudu-client/src/main/java/org/kududb/client/RecoverableException.java
M java/kudu-client/src/main/java/org/kududb/client/RowResultIterator.java
M java/kudu-client/src/main/java/org/kududb/client/Status.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
D 
java/kudu-client/src/main/java/org/kududb/client/TabletServerErrorException.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java
M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java
26 files changed, 401 insertions(+), 520 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3055/9
-- 
To view, visit http://gerrit.cloudera.org:8080/3055
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [java client] Redo how we manage exceptions

Kudu Jenkins has posted comments on this change.

Change subject: [java client] Redo how we manage exceptions
..


Patch Set 9:

Build Started http://104.196.14.100/job/kudu-gerrit/2552/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java client] Redo how we manage exceptions

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Redo how we manage exceptions
..


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/KuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/KuduClient.java:

Line 99:   public boolean isAlterTableDone(String name) throws KuduException, 
InterruptedException {
> It's not clear to me why this one throws Interrupted but not any of the oth
Ugh yeah looks like in other places where we call Deferred.join we just process 
everything as Exception.


http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/KuduException.java
File java/kudu-client/src/main/java/org/kududb/client/KuduException.java:

Line 37:  * sse if you're using the non-async API, such as {@link KuduSession} 
instead of
> s/sse/see
Done


http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java
File 
java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java:

Line 26: public class NonCoveredRangeException extends NonRecoverableException {
> Can this be default (package private) visibility?
Good catch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [java-client] fix soundness hole in flushing async kudu session

Dan Burkert has submitted this change and it was merged.

Change subject: [java-client] fix soundness hole in flushing async kudu session
..


[java-client] fix soundness hole in flushing async kudu session

This fixes an issue where AsyncKuduSession#apply in AUTO_FLUSH_BACKGROUND mode
could refresh the active buffer and add an initial operation to it, but fail to
set a flush timer on it. Writing a regression test proved to be very difficult
since it relies on a lot of timing variables around how fast batches can flush.

Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Reviewed-on: http://gerrit.cloudera.org:8080/3676
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Jean-Daniel Cryans 
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Doxygen for C++ client API

Kudu Jenkins has posted comments on this change.

Change subject: Doxygen for C++ client API
..


Patch Set 11:

Build Started http://104.196.14.100/job/kudu-gerrit/2551/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: No

[kudu-CR] Doxygen for C++ client API

Hello Kudu Jenkins,

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

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

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

Change subject: Doxygen for C++ client API
..

Doxygen for C++ client API

If doxygen is available, build 'doxygen' taget to generate
Doxygen docs from client.h and other Kudu C++ client API files,
After the target is built, open
${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen/client_api/html/index.html
in your favorite browser to see the generated documentation.

If doxygen is available, the 'docs' target generates
doxygen documentaion as well since it depends on the 'doxygen' target.

Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
---
M CMakeLists.txt
A docs/support/doxygen/client_api.doxy.in
M src/kudu/client/client.h
M src/kudu/util/CMakeLists.txt
M thirdparty/download-thirdparty.sh
5 files changed, 1,271 insertions(+), 721 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones

[kudu-CR] [java-client] fix soundness hole in flushing async kudu session

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] fix soundness hole in flushing async kudu session
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] fix soundness hole in flushing async kudu session

Adar Dembo has posted comments on this change.

Change subject: [java-client] fix soundness hole in flushing async kudu session
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Adar Dembo has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] doxygen for C++ client API

Kudu Jenkins has posted comments on this change.

Change subject: doxygen for C++ client API
..


Patch Set 10:

Build Started http://104.196.14.100/job/kudu-gerrit/2550/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: No

[kudu-CR] doxygen for C++ client API

Alexey Serbin has posted comments on this change.

Change subject: doxygen for C++ client API
..


Patch Set 9:

(64 comments)

http://gerrit.cloudera.org:8080/#/c/3619/9/src/kudu/client/client.h
File src/kudu/client/client.h:

PS9, Line 91: Before a callback is registered, all internal client log events
: /// are logged to the stderr.
> Nit: why combine this sentence into the previous paragraph instead of leavi
Done


Line 131: ///   Signal number to use for internal
> Nit: for internal what? Maybe you meant "Signal number to use internally"?
Done


PS9, Line 222: Each KuduClient instance is sandboxed with no
 : /// global cross-client state.
> Not related to your change, but this isn't strictly true anymore. Basically
Done


Line 247:   /// @return Pointer to newly created instance: it's the caller's
> Nit: semi-colon is more appropriate here.
Done


Line 277:   /// Check if table alteration is in process
> Nit: in-progress, to be consistent with IsCreateTableInProgress.
Done


PS9, Line 282:   ///   Set to @c true if an AlterTable operation is in-progress;
 :   ///   set to @c false otherwise
> Nit: can you reword to be consistent with IsCreateTableInProgress, since th
Done


Line 287:   /// Get scheme for a table
> Nit: schema, not scheme.
Done


Line 290:   ///   Name of the table if question
> Nit: in question
Done


Line 292:   ///   Raw pointer to the schema object: caller gets ownership
> Nit: semi-colon.
Done


Line 310:   ///   Substring-filter to use; empty sub-string filter matches any 
tables
> Nit: Substring filter
Done


Line 320:   ///   Set only on success: set to @c true iff table exists
> Nit: semi-colon
Done


PS9, Line 326:   /// If the table has not been opened before, then open the 
table
 :   /// with the given name.  If the table has not been opened 
before
 :   /// for this client, this will do an RPC to ensure that the 
table exists
 :   /// and look up its schema.
> Also not related to this change, but the RPC and schema lookup are always d
Done


Line 346:   /// @return A new session object: caller is responsible for 
destroying it.
> Nit: semi-colon.
Done


Line 359:   /// @return @c true iff it's a multi-master session
> This isn't related to sessions per-se, it's true if the client was configur
Done


Line 365:   /// @return Default timeout for RPC operations.
> Nit: we do distinguish between "operations" and "RPCs". The former are logi
Done


Line 376:   /// by this client.  Check KuduScanner for more details on 
timestamps.
> Nit: why the change from "See KuduScanner" to "Check KuduScanner"? Was the 
I was playing with @see directive here, but doxygen itself inserts hyperlink to 
KuduScanner here.  I dropped the '@' after realizing that.  OK, now it's back 
to 'See'.


PS9, Line 385: To use this the user must obtain
 :   /// the HybridTime encoded timestamp from the first client with
 :   /// KuduClient::GetLatestObservedTimestamp() and then set it
 :   /// in the new client with this method.
> Nit: let's separate this into a new paragraph.
Done


Line 432: class KUDU_EXPORT KuduTableCreator {
> The "Required" and "Optional" tags are gone. I think they were useful; can 
I don't understand what "Required" and "Optional" tags are in this context.  
Could you clarify?


Line 438:   /// This method must be called at an object before .
> This sentence is unclear, and there's an extra space at the end.
Done


Line 447:   /// Set the schema with which to create next table.
> Nit: the subsequent builder methods refer to the table to be created as sim
Done


PS9, Line 462: two
> Nit: to be consistent with the rest of this paragraph, use '2' instead of '
Done


PS9, Line 476:   /// This method takes a seed value, which can be used to 
randomize the
 :   /// mapping of rows to hash buckets.  Setting the seed may 
provide some
 :   /// amount of protection against denial of service attacks 
when the hashed
 :   /// columns contain user provided values.
> Can you add a small note at the beginning explaining that this is exactly t
Done


PS9, Line 482: Names of columns to build partition
> To be consistent with set_range_partition_columns(), how about "to use for 
Done


Line 499:   ///   Name of columns to use for partitioning.  Every column must be
> Nit: Names of columns
Done


PS9, Line 524:   /// Add a partition range based on lower and upper bounds.
 :   ///
 :   /// Add a partition range bound to the table with an inclusive 
lower bound
 :   /// and exclusive upper bound.
> These two sentences are almost the same, maybe we can just the second sente
Done


Line 533:   /// If this method is not called, the table's range is be unbounded.
> Nit: "is be"?
Done


Line 539:   ///   on the table range.  If a column of the @c lower_bound row is

[kudu-CR] doxygen for C++ client API

Hello Kudu Jenkins,

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

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

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

Change subject: doxygen for C++ client API
..

doxygen for C++ client API

If doxygen is available, build 'doxygen' taget to generate
Doxygen docs from client.h and other Kudu C++ client API files,
After the target is built, open
${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen/client_api/html/index.html
in your favorite browser to see the generated documentation.

If doxygen is available, the 'docs' target generates
doxygen documentaion as well since it depends on the 'doxygen' target.

Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
---
M CMakeLists.txt
A docs/support/doxygen/client_api.doxy.in
M src/kudu/client/client.h
M src/kudu/util/CMakeLists.txt
M thirdparty/download-thirdparty.sh
5 files changed, 1,283 insertions(+), 717 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3619/10
-- 
To view, visit http://gerrit.cloudera.org:8080/3619
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Misty Stanley-Jones

[kudu-CR] [java-client] fix soundness hole in flushing async kudu session

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] fix soundness hole in flushing async kudu session
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2549/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] fix soundness hole in flushing async kudu session

Hello Adar Dembo,

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

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

to review the following change.

Change subject: [java-client] fix soundness hole in flushing async kudu session
..

[java-client] fix soundness hole in flushing async kudu session

This fixes an issue where AsyncKuduSession#apply in AUTO_FLUSH_BACKGROUND mode
could refresh the active buffer and add an initial operation to it, but fail to
set a flush timer on it. Writing a regression test proved to be very difficult
since it relies on a lot of timing variables around how fast batches can flush.

Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
1 file changed, 3 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic838b65e2ccf5c48f0b51db235936b834aa24cae
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo

[kudu-CR] [java client] Redo how we manage exceptions

Dan Burkert has posted comments on this change.

Change subject: [java client] Redo how we manage exceptions
..


Patch Set 8:

(3 comments)

Looking great overall.

http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/KuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/KuduClient.java:

Line 99:   public boolean isAlterTableDone(String name) throws KuduException, 
InterruptedException {
It's not clear to me why this one throws Interrupted but not any of the other 
methods.  Seems like either all or none should.


http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/KuduException.java
File java/kudu-client/src/main/java/org/kududb/client/KuduException.java:

Line 37:  * sse if you're using the non-async API, such as {@link KuduSession} 
instead of
s/sse/see


http://gerrit.cloudera.org:8080/#/c/3055/8/java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java
File 
java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java:

Line 26: public class NonCoveredRangeException extends NonRecoverableException {
Can this be default (package private) visibility?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 7:

Build Started http://104.196.14.100/job/kudu-gerrit/2548/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Dan Burkert has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 6:

(10 comments)

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

PS6, Line 853: KuduPartialRow* lower_bound,
 :   
KuduPartialRow* upper_bound
> Should we first check that the bounds aren't null?
Done


http://gerrit.cloudera.org:8080/#/c/3648/6/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 406:   Status WaitForMasterUpAndRunning() WARN_UNUSED_RESULT;
> Maybe doc this new method? And since it's an ExternalMaster method, maybe r
Done


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

PS6, Line 1267:   return Status::NotFound("New partition conflicts 
with existing partition",
  :   step.DebugString());
> Can we point out the conflict in these messages? Or is that too verbose?
It will be pretty verbose, and I don't think it's too useful at this point 
(experience may prove me wrong, though).


PS6, Line 1272: auto prev = std::prev(existing_iter);
  : TabletMetadataLock metadata(prev->second, 
TabletMetadataLock::READ);
> Nit: combine?
Done


Line 1284: if (metadata.pb.partition().partition_key_start()< 
upper_bound) {
> Nit: partition_key_start() < upper_bound
Done


Line 1331: existing_tablets.erase(existing_iter);
> Maybe CHECK that this returns 1? Or add EraseOrDie to map-util.h and use th
This is erasing by iterator, not by key, so it can't fail (and doesn't return 
an int).


Line 1551:   {
> Please add a comment here rationalizing the order of operations.
Done


Line 1563: table->AddRemoveTablets(tablets_to_add, tablets_to_drop);
> I don't remember why we thought this should be done with the global catalog
Done


http://gerrit.cloudera.org:8080/#/c/3648/6/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

Line 207:   // Adds all tablets to the return vector in partition key sorted 
order.
> Nit: returned vector
I really meant 'return' here, although I see how it's confusing, so I just 
removed 'return' altogether.


Line 230:   std::map tablet_map() const {
> Let's make the TabletInfoMap typedef public and use it in this method.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..

KUDU-1311 [master] support adding and dropping range partitions

Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/common/partition.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/ksck.cc
19 files changed, 1,164 insertions(+), 191 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] Re-enable multi-master tests

Adar Dembo has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..

Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
File
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java:

PS3, Line 109: and
: // there's as many of them as responses received.
This much is obvious from the code itself, but what's not obvious is the
significance. Could you modify the comment to explain why this matters?

http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java:

Line 358: waitForAllTabletServers();
I've been working on a change that'll allow ListTabletServers to be called on
follower masters, as that reflects the new world order, where all masters
receive heartbeats in order to keep their TS caches warm.

With that change in place, I don't think waitForAllTabletServers() can be used
as a proxy for "wait for a new master to be elected and for tservers to report
to it". getTablesList() comes close (because it can only succeed after a new
leader master is elected) but that's still not the same thing.

Why do you actually need to wait? AFAICT killMasterLeader() is only used in
TestMasterFailover.testKillLeader(), and the operations that follow the call
should all succeed even without waiting, because the client itself is supposed
to wait. The only exception is the createTable(), but that will be fixed with
my patches.

http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java
File
java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java:

Line 91: // Permutation of the previous
Nit: terminate with a period. Below too.

Line 162: GetMasterRegistrationReceived grrm = new
GetMasterRegistrationReceived(MASTERS, d);
Nice variable name.

Line 177: d.join(1000); // Don't care about the response.
Is there any significance to the timeout value here? Could we call join()
without a timeout?

Line 188: // Helper method that determines if the callback or errback should
be called.
This one and makeGMRR() can be static.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-HasComments: Yes

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Dan Burkert has submitted this change and it was merged.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


[c++-client]: cache non-covering ranges in meta cache

This commit introduces a few features to the meta cache, all with the aim of
making it compatible with the upcoming add/drop range partitions feature.

1) Non-covered range partitions are now cached in the meta cache.  This is
achieved by storing MetaCacheEntry objects in the meta cache's partition-key
index instead of RemoteTablets.  The MetaCacheEntry holds either a RemoteTablet,
in which case it represents a covered partition range, or it represents a
non-covered partition range.

2) Entries are now removed from the meta cache's partition-key index when it can
be determined that the entries are no longer valid from the results of a
GetTableLocations RPC.

3) A basic TTL has been added to the GetTableLocationsResponsePB so that the
client can properly refresh the meta cache when necessary. The TTL is
configurable by the master, and defaults to one hour.

Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Reviewed-on: http://gerrit.cloudera.org:8080/3581
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/schema.h
M src/kudu/client/table-internal.cc
M src/kudu/gutil/map-util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/ksck_remote.cc
10 files changed, 438 insertions(+), 139 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Dan Burkert has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3581/9/src/kudu/client/client.h
File src/kudu/client/client.h:

PS9, Line 293:   FRIEND_TEST(ClientTest, TestGetTabletServerBlacklist);
 :   FRIEND_TEST(ClientTest, TestMasterDown);
 :   FRIEND_TEST(ClientTest, TestMasterLookupPermits);
 :   FRIEND_TEST(ClientTest, TestMetaCacheExpiry);
 :   FRIEND_TEST(ClientTest, TestNonCoveringRangePartitions);
 :   FRIEND_TEST(ClientTest, 
TestReplicatedTabletWritesWithLeaderElection);
 :   FRIEND_TEST(ClientTest, TestScanFaultTolerance);
 :   FRIEND_TEST(ClientTest, TestScanTimeout);
 :   FRIEND_TEST(ClientTest, TestWriteWithDeadMaster);
> Now that the entire test class is marked as a friend, are these still neede
Yes, these individual tests are accessing the data_ field.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Dan Burkert has submitted this change and it was merged.

Change subject: [c++-client] add LESS and GREATER column predicates
..


[c++-client] add LESS and GREATER column predicates

Change-Id: I5ed9f2cec55205d38c1a3f004c286d6e436baa90
Reviewed-on: http://gerrit.cloudera.org:8080/3674
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_predicate.cc
M src/kudu/client/scan_predicate.h
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
6 files changed, 194 insertions(+), 2 deletions(-)

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



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

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

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Adar Dembo has posted comments on this change.

Change subject: [c++-client] add LESS and GREATER column predicates
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] [java client] Integrate with the replay cache

Kudu Jenkins has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/2547/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Integrate with the replay cache

Hello Kudu Jenkins,

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

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

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

Change subject: [java client] Integrate with the replay cache
..

[java client] Integrate with the replay cache

This patch adds the required functionality to have the Java client use the
server-side replay cache.

Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/kududb/client/Batch.java
M java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java
M java/kudu-client/src/main/java/org/kududb/client/Operation.java
A java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java
A java/kudu-client/src/test/java/org/kududb/client/TestRequestTracker.java
11 files changed, 218 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3631/5
-- 
To view, visit http://gerrit.cloudera.org:8080/3631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] Re-enable multi-master tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 3: Verified+1

So much flakiness right now on jenkins.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-HasComments: No

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Hello Adar Dembo, Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: [c++-client] add LESS and GREATER column predicates
..

[c++-client] add LESS and GREATER column predicates

Change-Id: I5ed9f2cec55205d38c1a3f004c286d6e436baa90
---
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_predicate.cc
M src/kudu/client/scan_predicate.h
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
6 files changed, 194 insertions(+), 2 deletions(-)


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

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

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Kudu Jenkins has posted comments on this change.

Change subject: [c++-client] add LESS and GREATER column predicates
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2546/

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

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

[kudu-CR] [java-client] Re-enable multi-master tests

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2545/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] Re-enable multi-master tests
..

[java-client] Re-enable multi-master tests

This patch makes TestMasterFailover useful again. It also adds the killing of 
masters
to ITClient. Finally, it sets the raft heartbeat lower so that we don't wait 
1.5s for
leader elections.

TestGetMasterRegistrationReceived was added since a previous version of this 
patch
encountered a bug that such a simple unit test can detect.

Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
---
M 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java
A 
java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java
M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java
6 files changed, 243 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](gh-pages) Clean up community web page and add commits@ mailing list

Mike Percy has submitted this change and it was merged.

Change subject: Clean up community web page and add commits@ mailing list
..


Clean up community web page and add commits@ mailing list

Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Reviewed-on: http://gerrit.cloudera.org:8080/3666
Reviewed-by: Todd Lipcon 
Tested-by: Mike Percy 
---
M community.md
1 file changed, 30 insertions(+), 15 deletions(-)

Approvals:
  Mike Percy: Verified
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR](gh-pages) Add dropdown menu for Community nav button

Mike Percy has submitted this change and it was merged.

Change subject: Add dropdown menu for Community nav button
..


Add dropdown menu for Community nav button

* The dropdown is disabled on very small screens (mobile)
* The dropdown is disabled on smallish touch-enabled screens (iPad)
* Also remove justified nav CSS file, since it's no longer used
* Also remove "Fork me on GitHub" ribbon

Change-Id: I8c0f2671e2257a4249b4b26d1cc8513ee0e47362
Reviewed-on: http://gerrit.cloudera.org:8080/3665
Reviewed-by: Todd Lipcon 
Tested-by: Mike Percy 
---
M _includes/bottom_common.html
M _includes/top_common.html
D css/justified-nav.css
M css/kudu.css
4 files changed, 163 insertions(+), 156 deletions(-)

Approvals:
  Mike Percy: Verified
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8c0f2671e2257a4249b4b26d1cc8513ee0e47362
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR](gh-pages) Clean up community web page and add commits@ mailing list

Mike Percy has posted comments on this change.

Change subject: Clean up community web page and add commits@ mailing list
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add dropdown menu for Community nav button

Mike Percy has posted comments on this change.

Change subject: Add dropdown menu for Community nav button
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c0f2671e2257a4249b4b26d1cc8513ee0e47362
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Clean up community web page and add commits@ mailing list

Todd Lipcon has posted comments on this change.

Change subject: Clean up community web page and add commits@ mailing list
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add dropdown menu for Community nav button

Todd Lipcon has posted comments on this change.

Change subject: Add dropdown menu for Community nav button
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c0f2671e2257a4249b4b26d1cc8513ee0e47362
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1416 Upsert support for Flume sink

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1416 Upsert support for Flume sink
..


Patch Set 1:

What's the story on this one? Needs another rev, right?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Ara Ebrahimi 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master

2016-07-19 Thread Dinesh Bhat (Code Review)

Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1492: Show column encodings/compression on table page in 
master
..


Patch Set 2:

(6 comments)

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

Line 7: KUDU-1492: Show column encodings/compression on table page in master
> This is a good summary of best practices for this: http://chris.beams.io/po
yep, that helps. thanks.


Line 8: 
> Usually our commit messages are a little more informative (see other commit
thanks, addressed them both.


http://gerrit.cloudera.org:8080/#/c/3667/1/src/kudu/server/webui_util.cc
File src/kudu/server/webui_util.cc:

Line 35:std::stringstream* output) {
> This is not part of this change, but just an additional comment.  Does it m
That's a good point, I will note this for future changes. I would prefer to 
keep this as-is otherwise this means changing the signatures of callers too at 
few other places.


Line 39:   << "EncodingCompression"
> How about we use two different columns for each of encoding and compression
yep, either way is fine with me. I had clubbed them thinking they both are 
column attrs. Updated the patch with new output format.


PS1, Line 40: 
> spaces instead of tabs. check out google style guide.
Thanks david for catching, yes my editor settings were at default.


Line 56:   *output << 
Substitute("$0$1$2$3"
> I, as a reader, would appreciate some text example of the output in the com
Hmmm, I thought about it, but the output wasn't fitting within 80 columns and 
spilling over wasn't looking pretty either so chucked that idea.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master

2016-07-19 Thread Dinesh Bhat (Code Review)

Hello Mike Percy, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1492: Show column encodings/compression on table page in 
master
..

KUDU-1492: Show column encodings/compression on table page in master

This displays column  atributes for the table schema in picture.
If the table schema doesn't specify these attributes,
AUTO_ENCODING or DEFAULT_COMPRESSION are displayed which means
whatever is the current default value attributes in the given
release. For eg, current release has PLAIN_ENCODING for encoding
and NO_COMPRESSION for compression.
Sample results are posted in JIRA KUDU-1492.

Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8
---
M src/kudu/server/webui_util.cc
1 file changed, 14 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] Attempt to unflake disk reservation-itest a little more

Mike Percy has submitted this change and it was merged.

Change subject: Attempt to unflake disk_reservation-itest a little more
..


Attempt to unflake disk_reservation-itest a little more

Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
Reviewed-on: http://gerrit.cloudera.org:8080/3675
Reviewed-by: Mike Percy 
Tested-by: Mike Percy 
---
M src/kudu/integration-tests/disk_reservation-itest.cc
1 file changed, 2 insertions(+), 4 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] Attempt to unflake disk reservation-itest a little more

Mike Percy has posted comments on this change.

Change subject: Attempt to unflake disk_reservation-itest a little more
..


Patch Set 1: Code-Review+2 Verified+1

Pushing this trivial change through, would like to see if it is sufficient

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Attempt to unflake disk reservation-itest a little more

Kudu Jenkins has posted comments on this change.

Change subject: Attempt to unflake disk_reservation-itest a little more
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2544/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Attempt to unflake disk reservation-itest a little more

Mike Percy has uploaded a new change for review.

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

Change subject: Attempt to unflake disk_reservation-itest a little more
..

Attempt to unflake disk_reservation-itest a little more

Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
---
M src/kudu/integration-tests/disk_reservation-itest.cc
1 file changed, 2 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3e8e7a1fb19d78514e4723fd74998ff2f8cc8e74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy

[kudu-CR] KUDU-1492: Show column encodings/compression on table page in master

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1492: Show column encodings/compression on table page in 
master
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2543/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I990a8d790ab71a05be04f0b7468b5da0894478e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Todd Lipcon has posted comments on this change.

Change subject: [c++-client] add LESS and GREATER column predicates
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3674/1/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:

PS1, Line 98:  
nit: extra space


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

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

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..


Patch Set 1:

OK, thanks again for getting it started, will see if I can pick up where you 
left off

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Adar Dembo has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 6:

(11 comments)

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

PS6, Line 853: KuduPartialRow* lower_bound,
 :   
KuduPartialRow* upper_bound
Should we first check that the bounds aren't null?


http://gerrit.cloudera.org:8080/#/c/3648/6/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 406:   Status WaitForMasterUpAndRunning() WARN_UNUSED_RESULT;
Maybe doc this new method? And since it's an ExternalMaster method, maybe 
rename to "WaitForCatalogManager" or something more specific?


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

Line 1345:   }
> yes, otherwise the messages would need to be copied.
Can't these vectors be of crefs instead? I just don't think any other methods 
here modify the input, and it doesn't seem like something we should do.


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

PS6, Line 1267:   return Status::NotFound("New partition conflicts 
with existing partition",
  :   step.DebugString());
Can we point out the conflict in these messages? Or is that too verbose?


PS6, Line 1272: auto prev = std::prev(existing_iter);
  : TabletMetadataLock metadata(prev->second, 
TabletMetadataLock::READ);
Nit: combine?


Line 1284: if (metadata.pb.partition().partition_key_start()< 
upper_bound) {
Nit: partition_key_start() < upper_bound


Line 1331: existing_tablets.erase(existing_iter);
Maybe CHECK that this returns 1? Or add EraseOrDie to map-util.h and use that?


Line 1551:   {
Please add a comment here rationalizing the order of operations.


Line 1563: table->AddRemoveTablets(tablets_to_add, tablets_to_drop);
I don't remember why we thought this should be done with the global catalog 
manager spinlock held. Can you remind me?


http://gerrit.cloudera.org:8080/#/c/3648/6/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

Line 207:   // Adds all tablets to the return vector in partition key sorted 
order.
Nit: returned vector


Line 230:   std::map tablet_map() const {
Let's make the TabletInfoMap typedef public and use it in this method.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [c++-client] add LESS and GREATER column predicates

Adar Dembo has posted comments on this change.

Change subject: [c++-client] add LESS and GREATER column predicates
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Adar Dembo has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 9: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3581/9/src/kudu/client/client.h
File src/kudu/client/client.h:

PS9, Line 293:   FRIEND_TEST(ClientTest, TestGetTabletServerBlacklist);
 :   FRIEND_TEST(ClientTest, TestMasterDown);
 :   FRIEND_TEST(ClientTest, TestMasterLookupPermits);
 :   FRIEND_TEST(ClientTest, TestMetaCacheExpiry);
 :   FRIEND_TEST(ClientTest, TestNonCoveringRangePartitions);
 :   FRIEND_TEST(ClientTest, 
TestReplicatedTabletWritesWithLeaderElection);
 :   FRIEND_TEST(ClientTest, TestScanFaultTolerance);
 :   FRIEND_TEST(ClientTest, TestScanTimeout);
 :   FRIEND_TEST(ClientTest, TestWriteWithDeadMaster);
Now that the entire test class is marked as a friend, are these still needed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions