[kudu-CR] [python] added KuduPartialRow::SetNoCopy{Binary,String}()

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [python] added KuduPartialRow::SetNoCopy{Binary,String}()
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: If73894c3037f1158cf7b9d3aafecd228a0204bbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [clock] minor clean-up on the Clock class

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [clock] minor clean-up on the Clock class
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2027 retry scan RPC if negotiation times out

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: KUDU-2027 retry scan RPC if negotiation times out
..


Patch Set 14:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7037/14//COMMIT_MSG
Commit Message:

PS14, Line 15: handling. Now, regardless of whether it was the RPC or the 
overall
 : operation timeout, the corresponding tablet server is marked as 
failed
 : and blacklisted.
> we don't currently invalidate this, right? Given that systems like Impala u
Right -- without this patch (i.e. currently) we don't invalidate corresponding 
entries in the client's meta-cache on read timeout.

As I understand, if the scan source is set to anything than LEADER_ONLY, the 
client should try other available replicas.  And if it runs out of replicas, 
marking them failed, then it will not be able to continue -- that's a good 
observation.

Should we re-fetch information on replicas in case of running out of available 
replicas in client meta-cache (stopping doing that after some tries)?

What else can we do to handle this situation if not marking failed ones?  Do 
some round-robin among available sources?


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

PS14, Line 1991:   // The scanner should timeout on the next batch not fetched 
by the Open().
   :   // The client should not retry until the overall timeout 
expires, giving up
   :   // after the fist RPC timeout error: the only available 
replica is marked
   :   // as failed by the first timeout error.
> I'm not really understanding this comment.
Agreed, it sounds like some mumbling.  Let me rephrase it:

In the scenario below, the scanner has some rows not fetched upon calling 
KuduScanner::Open() because scanner_max_batch_size_bytes is set to a very small 
value.  There will be timeout on first call of KuduScanner::NextBatch() after 
scanner is opened.  Since the only available entry in the client meta-cache is 
marked as failed after timing out on the call of KuduScanner::NextBatch(), the 
client has no other sources for the scanner to try and will fail right after 
the KuduScanner::NextBatch() call.

Is it better?


Line 2021: ASSERT_LT(sw.elapsed().wall_millis(), kScanTimeoutMs);
> in the case that we know we won't be able to retry a scan elsewhere, should
It sounds like a good idea.

Now there is allow_time_for_failover provision as well, it's just not activated 
in this case because I simplified the logic at a few call sites.  I'll take a 
look.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05a3c38cc07c9b6edd1ae773d9850e92645f3b14
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] consensus: Get rid of a bunch of cmeta wrappers

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: consensus: Get rid of a bunch of cmeta wrappers
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I90537d8ba945e45671415a1b5f899166343f28cb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No


[kudu-CR] kserver: consolidate randomized failure monitors

2017-06-28 Thread Adar Dembo (Code Review)
Hello David Ribeiro Alves, Mike Percy, Todd Lipcon,

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

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

to review the following change.

Change subject: kserver: consolidate randomized failure monitors
..

kserver: consolidate randomized failure monitors

The failure detector design (i.e. the split between FailureDetector and
RandomizedFailureMonitor) makes it quite simple to consolidate the threads
used by failure detectors: simply share a single RandomizedFailureMonitor.
Most of this patch is plumbing to enable that.

What's actually interesting?
- When failure detection is activated, we create a cycle between
  RaftConsensus and FailureDetector in order to protect against failure
  callbacks triggering after RaftConsensus is destroyed. The cycle is broken
  during Shutdown. If/when RaftConsensus is changed to use a standard
  shared_ptr, we can bind with a weak_ptr and avoid the cycle.
- kserver now depends on consensus; this means it can use gflags defined in
  consensus. Accordingly, the raft pool's custom idle time has been restored
  (it was removed in commit 3fb84a7).

Change-Id: I096e3e89bf6e8925f6ea0382fc319d7382237787
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/kserver/CMakeLists.txt
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
12 files changed, 112 insertions(+), 58 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I096e3e89bf6e8925f6ea0382fc319d7382237787
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] WIP: consensus peers: schedule raft heartbeats through messenger

2017-06-28 Thread Adar Dembo (Code Review)
Hello David Ribeiro Alves, Mike Percy, Todd Lipcon,

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

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

to review the following change.

Change subject: WIP: consensus_peers: schedule raft heartbeats through messenger
..

WIP: consensus_peers: schedule raft heartbeats through messenger

This patch switches Peer to a Messenger-based approach (i.e.
Messenger::ScheduleOnReactor) for scheduling heartbeats. The problem with
ResettableHeartbeater was that it creates a dedicated thread per replica,
and I didn't see an easy way to share it across replicas.

The scheduling is inspired by libev's "Let the timer time out, but then
re-arm it as required." approach[1]. Callbacks are never canceled (since
Messenger doesn't provide a way to do that); instead, both the desired
period and time of last heartbeat are maintained as out-of-band state.

WIP because I want feedback on the general approach, and because it needs
better encapsulation and docs.

1. 
http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#code_ev_timer_code_relative_and_opti

Change-Id: Iac8e09fe02dd32885ef0cf644cb093b1c8e6afb8
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_peers.h
M src/kudu/consensus/leader_election-test.cc
M src/kudu/consensus/peer_manager.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
7 files changed, 148 insertions(+), 39 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iac8e09fe02dd32885ef0cf644cb093b1c8e6afb8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [java] KUDU-2013 re-acquire authn token if expired
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java:

Line 668: TO_BE_RENEGOTIATED, // The connection negotiation has failed and 
has to be be re-negotiated.
> An update: after looking at how to avoid re-connecting a connection, I came
And another update: I did that.  So, Connection to TCP socket is now 1-to-1 
relationship (see PS5).

Also, in PS5 all currently established connections are now represented in the 
connection cache.  Prior to PS5 it was possible to have secondary-credentials 
connection to master be alive and outside of the cache when primary-credentials 
connection to the same master kicked it out from the cache upon acquiring a new 
authentication token.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] raft consensus: failure detector doesn't need external synchronization

2017-06-28 Thread Adar Dembo (Code Review)
Hello Mike Percy,

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

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

to review the following change.

Change subject: raft_consensus: failure detector doesn't need external 
synchronization
..

raft_consensus: failure detector doesn't need external synchronization

While working with TimerFailureDetector I saw that it's thread-safe
through and through. Thus, RaftConsensus doesn't need to externally
synchronize it. So I renamed some functions and removed a few lock
acquisition sites.

AFAICT, the only reason not to do this is for one of the overloads of
SnoozeFailureDetector() which calls LOG_WITH_PREFIX_UNLOCKED(). I don't
think that's useful enough to warrant the locking, though.

Change-Id: I637030c3ae2470aa0a8e1067ac6a9b69ea1793b4
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
2 files changed, 29 insertions(+), 35 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I637030c3ae2470aa0a8e1067ac6a9b69ea1793b4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

2017-06-28 Thread Edward Fancher (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 49 insertions(+), 21 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] time manager: fix unsynchronized access to GetSerialTimestamp()

2017-06-28 Thread Adar Dembo (Code Review)
Hello David Ribeiro Alves, Mike Percy,

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

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

to review the following change.

Change subject: time_manager: fix unsynchronized access to GetSerialTimestamp()
..

time_manager: fix unsynchronized access to GetSerialTimestamp()

This produced the occasional race, which I've attached to the bottom of the
commit message.

I also revoked friendship for RaftConsensus. This was done solely to get
access to GetSerialTimestamp; making the function public seems like the
lesser of two evils to me.

WARNING: ThreadSanitizer: data race (pid=15727)
  Read of size 8 at 0x7b1c00013818 by thread T126 (mutexes: write M2582, write 
M2285):
#0 kudu::operator<(kudu::Timestamp const&, kudu::Timestamp const&) 
/data/jenkins-workspace/kudu-workspace/src/kudu/common/timestamp.h:105:22 
(libtserver.so+0xe1768)
#1 kudu::operator>(kudu::Timestamp const&, kudu::Timestamp const&) 
/data/jenkins-workspace/kudu-workspace/src/kudu/common/timestamp.h:109:14 
(libtserver.so+0xd9170)
#2 kudu::operator<=(kudu::Timestamp const&, kudu::Timestamp const&) 
/data/jenkins-workspace/kudu-workspace/src/kudu/common/timestamp.h:113:16 
(libtablet.so+0x19e990)
#3 kudu::consensus::TimeManager::GetSafeTimeUnlocked() 
/data/jenkins-workspace/kudu-workspace/src/kudu/consensus/time_manager.cc:291:11
 (libconsensus.so+0xbddad)
#4 kudu::consensus::TimeManager::GetSafeTime() 
/data/jenkins-workspace/kudu-workspace/src/kudu/consensus/time_manager.cc:267:10
 (libconsensus.so+0xbdeb9)
#5 
kudu::consensus::PeerMessageQueue::RequestForPeer(std::__1::basic_string const&, 
kudu::consensus::ConsensusRequestPB*, 
std::__1::vector >*, 
bool*) 
/data/jenkins-workspace/kudu-workspace/src/kudu/consensus/consensus_queue.cc:469:50
 (libconsensus.so+0x70a3a)
#6 kudu::consensus::Peer::SendNextRequest(bool) 
/data/jenkins-workspace/kudu-workspace/src/kudu/consensus/consensus_peers.cc:177:22
 (libconsensus.so+0x649c2)
#7 kudu::consensus::Peer::SignalRequest(bool)::$_0::operator()() const 
/data/jenkins-workspace/kudu-workspace/src/kudu/consensus/consensus_peers.cc:134:3
 (libconsensus.so+0x67a02)
#8 
boost::detail::function::void_function_obj_invoker0::invoke(boost::detail::function::function_buffer&) 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/function/function_template.hpp:159:11
 (libconsensus.so+0x67809)
#9 boost::function0::operator()() const 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/function/function_template.hpp:770:14
 (libkrpc.so+0xb0111)
#10 kudu::FunctionRunnable::Run() 
/data/jenkins-workspace/kudu-workspace/src/kudu/util/threadpool.cc:56:5 
(libkudu_util.so+0x1c4b1d)
#11 kudu::ThreadPool::DispatchThread(bool) 
/data/jenkins-workspace/kudu-workspace/src/kudu/util/threadpool.cc:621:22 
(libkudu_util.so+0x1c1e04)
#12 boost::_mfi::mf1::operator()(kudu::ThreadPool*, bool) const 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/bind/mem_fn_template.hpp:165:29
 (libkudu_util.so+0x1ca38e)
#13 void boost::_bi::list2, 
boost::_bi::value >::operator(), boost::_bi::list0>(boost::_bi::type, boost::_mfi::mf1&, boost::_bi::list0&, int) 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/bind/bind.hpp:319:9
 (libkudu_util.so+0x1ca2cd)
#14 boost::_bi::bind_t, boost::_bi::list2, 
boost::_bi::value > >::operator()() 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/bind/bind.hpp:1222:16
 (libkudu_util.so+0x1ca233)
#15 
boost::detail::function::void_function_obj_invoker0, 
boost::_bi::list2, boost::_bi::value 
> >, void>::invoke(boost::detail::function::function_buffer&) 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/function/function_template.hpp:159:11
 (libkudu_util.so+0x1c9fd1)
#16 boost::function0::operator()() const 
/data/jenkins-workspace/kudu-workspace/thirdparty/installed/tsan/include/boost/function/function_template.hpp:770:14
 (libkrpc.so+0xb0111)
#17 kudu::Thread::SuperviseThread(void*) 
/data/jenkins-workspace/kudu-workspace/src/kudu/util/thread.cc:591:3 
(libkudu_util.so+0x1b975e)

  Previous write of size 8 at 0x7b1c00013818 by thread T76 (mutexes: write 
M2279):
#0 kudu::consensus::TimeManager::GetSerialTimestamp() 

[kudu-CR] consensus: Get rid of a bunch of cmeta wrappers

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: consensus: Get rid of a bunch of cmeta wrappers
..


Patch Set 1: Verified+1

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

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


[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

2017-06-28 Thread Edward Fancher (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 51 insertions(+), 22 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Create ConsensusMetadataManager
..

Create ConsensusMetadataManager

This patch creates an API for a class that manages consensus
metadata. This abstracts out the management of ConsensusMetadata files,
providing the following benefits:

1) An instance of this class can be plumbed throughout the various
classes that deal with reading, creating, and modifying consensus
metadata so that we don't have to pass the individual cmeta instances
around.

2) It provides additional abstraction and flexibility that will make it
easier to change the underlying implementation of ConsensusMetadata in
the future as needed, perhaps if we wanted to make them log structured
and group committed across all replicas on a tablet server.

Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
---
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
A src/kudu/consensus/consensus_meta_manager-test.cc
A src/kudu/consensus/consensus_meta_manager.cc
A src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
24 files changed, 603 insertions(+), 160 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Create ConsensusMetadataManager
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS7, Line 51: lock_guard l(lock_);
> Good point. But I guess it's moot, I'll try to simplify.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] consensus: Get rid of a bunch of cmeta wrappers

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has uploaded a new change for review.

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

Change subject: consensus: Get rid of a bunch of cmeta wrappers
..

consensus: Get rid of a bunch of cmeta wrappers

We can directly access consensus metadata in RaftConsensus now, so get
rid of some useless wrapper functions now.

Change-Id: I90537d8ba945e45671415a1b5f899166343f28cb
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
2 files changed, 34 insertions(+), 85 deletions(-)


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

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


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Create ConsensusMetadataManager
..

Create ConsensusMetadataManager

This patch creates an API for a class that manages consensus
metadata. This abstracts out the management of ConsensusMetadata files,
providing the following benefits:

1) An instance of this class can be plumbed throughout the various
classes that deal with reading, creating, and modifying consensus
metadata so that we don't have to pass the individual cmeta instances
around.

2) It provides additional abstraction and flexibility that will make it
easier to change the underlying implementation of ConsensusMetadata in
the future as needed, perhaps if we wanted to make them log structured
and group committed across all replicas on a tablet server.

Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
---
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
A src/kudu/consensus/consensus_meta_manager-test.cc
A src/kudu/consensus/consensus_meta_manager.cc
A src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
24 files changed, 602 insertions(+), 155 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Create ConsensusMetadataManager
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

Line 44: // Ensure we don't interleave with other ops on this tablet.
> No, they are mutexed in this implementation.
Hmm. I was about to post a simplification but it was too weak and didn't 
reliably maintain a single cmeta instance for concurrent Create() and Load(). 
So I am going to stick with the version posted here in Rev 7.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Create ConsensusMetadataManager
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

Line 44
> in that case, if a load races with a create, can it see partially written d
No, they are mutexed in this implementation.

However I see that this impl is confusing for reviewers. So I am going to 
weaken the semantics a bit and simplify the impl a lot.


PS7, Line 51: 
> nit: can't you just lock in CancelOpInProgress?
Good point. But I guess it's moot, I'll try to simplify.


http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

PS2, Line 169: meta
> maybe just on the public method then.
Not excited about this because it's a slippery slope. But OK - I'll do it in 
this file. A separate patch makes more sense for a pervasive rename.


http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS2, Line 1067: const scoped_refptr&
> yeah, but the caller might then think that this method increases the refcou
I don't see a problem with that. This method only takes a constref and returns 
when it's done so the caller can drop the ref after this method returns. From 
my perspective it would be significantly more confusing if we passed a bare ptr 
here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Create ConsensusMetadataManager

2017-06-28 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: Create ConsensusMetadataManager
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

Line 44
> > Isn't interleaving on Create() already a disallowed operation?
in that case, if a load races with a create, can it see partially written data? 
same with a delete. it seems like concurrent creates/deletes can't happen, but 
what if a load happens while data is being mutated on disk?


PS7, Line 51: 
nit: can't you just lock in CancelOpInProgress?


http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

PS2, Line 169: meta
> I did consider it but I think if we want to do that it should be a separate
maybe just on the public method then.


http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS2, Line 1067: const scoped_refptr&
> Why? Passing it this way provides useful information, i.e. that this is ref
yeah, but the caller might then think that this method increases the refcount, 
which it doesn't, and that it no longer need to keep a refcount itself.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Rename MiniClusterOptions to InternalMiniClusterOptions

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Rename MiniClusterOptions to InternalMiniClusterOptions
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb1a87d916fdc0a248b5b6174306cc544333685
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Rename MiniClusterOptions to InternalMiniClusterOptions

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: Rename MiniClusterOptions to InternalMiniClusterOptions
..


Rename MiniClusterOptions to InternalMiniClusterOptions

Change-Id: I5bb1a87d916fdc0a248b5b6174306cc544333685
Reviewed-on: http://gerrit.cloudera.org:8080/7287
Tested-by: Mike Percy 
Reviewed-by: Todd Lipcon 
---
M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc
M src/kudu/benchmarks/tpch/tpch1.cc
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/internal_mini_cluster-itest-base.h
M src/kudu/integration-tests/internal_mini_cluster.cc
M src/kudu/integration-tests/internal_mini_cluster.h
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/token_signer-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/update_scan_delta_compact-test.cc
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/kudu-tool-test.cc
21 files changed, 25 insertions(+), 25 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5bb1a87d916fdc0a248b5b6174306cc544333685
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] ConsensusMetadata::Create() should not overwrite an existing file

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: ConsensusMetadata::Create() should not overwrite an existing 
file
..


ConsensusMetadata::Create() should not overwrite an existing file

We want to be able to control whether clobbering files is allowed. For
example, when there is an existing ConsensusMetadata file on disk, it is
dangerous for Create() to overwrite that file. The safe procedure to
make changes in that situation is to first Load() the file, make
changes, then Flush().

* Stop allowing ConsensusMetadata::Create() to overwrite files.
* Add unit test ConsensusMetadataTest.TestCreateNoOverwrite for this.
* Fix test BootstrapTest.TestOrphanCommit that violated this rule.

Change-Id: I0bfef1ae86a7d2c162095a76bb184a9c18dc69a7
Reviewed-on: http://gerrit.cloudera.org:8080/7190
Tested-by: Mike Percy 
Reviewed-by: Mike Percy 
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/tablet/tablet_bootstrap-test.cc
4 files changed, 48 insertions(+), 18 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0bfef1ae86a7d2c162095a76bb184a9c18dc69a7
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] ConsensusMetadata::Create() should not overwrite an existing file

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: ConsensusMetadata::Create() should not overwrite an existing 
file
..


Patch Set 6: Code-Review+2

Porting forward +2s from several others before a rebase.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bfef1ae86a7d2c162095a76bb184a9c18dc69a7
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] ConsensusMetadata::Create() should not overwrite an existing file

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: ConsensusMetadata::Create() should not overwrite an existing 
file
..


Patch Set 6: Verified+1

overriding flaky test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bfef1ae86a7d2c162095a76bb184a9c18dc69a7
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [java] separating Connection

2017-06-28 Thread Alexey Serbin (Code Review)
Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: [java] separating Connection
..

[java] separating Connection

This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The updated TabletClient has been renamed into RpcProxy.
Also, this patch contains other micro-updates on the related code.

In addition, this patch addresses KUDU-1878.

This work is done in the context of KUDU-2013.

Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
21 files changed, 1,401 insertions(+), 1,264 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired

2017-06-28 Thread Alexey Serbin (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java] KUDU-2013 re-acquire authn token if expired
..

[java] KUDU-2013 re-acquire authn token if expired

This patch introduces automatic authn token re-acquisition when the
current authn token expires.  The client automatically retries the RPC
that hits the token expiration error (the error to re-try is seen as
FATAL_INVALID_AUTHENTICATION_TOKEN sent by the server during connection
negotiation).

Added a few of tests to exercise the new retry logic for automatic
token re-acquisition in case of master-only operations, a bare minimum
workload scenario, and one special case of a connection to the master
opened with secondary credentials.

Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquireOpen.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
9 files changed, 656 insertions(+), 66 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [java] excluded slf4j from async dependency

2017-06-28 Thread Grant Henke (Code Review)
Grant Henke has uploaded a new change for review.

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

Change subject: [java] excluded slf4j from async dependency
..

[java] excluded slf4j from async dependency

The slf4j-api dependency defined by async uses a range-based version. This 
change excludes that dependency in order to ensure re-producible builds 
regardless of what slf4j-api versions are available.

Change-Id: Ia557729ef50b3736a53224bae64663652aeb0722
---
M java/kudu-client/pom.xml
M java/kudu-mapreduce/pom.xml
2 files changed, 16 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia557729ef50b3736a53224bae64663652aeb0722
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 


[kudu-CR] [rpc] use C++11 move semantics for selected methods

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [rpc] use C++11 move semantics for selected methods
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id7ee8d69a620db597ed9d27c4284656a4169bb09
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] re-acquire authn token if expired

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [java] re-acquire authn token if expired
..


Abandoned

This is superseded by https://gerrit.cloudera.org/#/c/7250/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [util] use emplace() instead of insert()

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [util] use emplace() instead of insert()
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1927 [master] always send IPKI CA certificate

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: KUDU-1927 [master] always send IPKI CA certificate
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib9a20e6da6531091fcc597af58a7bbf01a8847f5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [mini {master,tablet server}] webserver binding change

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [mini_{master,tablet_server}] webserver binding change
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I7a655754687badb6870747c18fb44488c68373b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [raft consensus-itest] CHECK OK --> ASSERT OK where possible

2017-06-28 Thread Alexey Serbin (Code Review)
Alexey Serbin has abandoned this change.

Change subject: [raft_consensus-itest] CHECK_OK --> ASSERT_OK where possible
..


Abandoned

not needed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I1bf7e5743a93619dd08db2519e2f5d9aeef24d86
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] mini tablet server: support multiple data dirs

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: mini_tablet_server: support multiple data dirs
..


Patch Set 6:

(3 comments)

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

Line 7: mini_tablet_server: support multiple data dirs
> I think this would be better defined in MiniClusterOptions and then plumbed
Agreed


PS6, Line 18: /test_dir/test_tserver_wal_dir
: /test_dir/test_tserver_data_dir-0
: /test_dir/test_tserver_data_dir-1
: /test_dir/test_tserver_data_dir-2
> Hmm, what do you think about:
+1 on this


http://gerrit.cloudera.org:8080/#/c/7211/6/src/kudu/tserver/tablet_server-test-base.h
File src/kudu/tserver/tablet_server-test-base.h:

PS6, Line 100:  = 1
Don't use default arguments with virtual functions. They don't really work.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52c9352f1a3565d58149cf2c63d37246c6b39c23
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes


[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
Reviewed-on: http://gerrit.cloudera.org:8080/7318
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] log block manager: do not fail block creation if truncate fails

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: log block manager: do not fail block creation if truncate fails
..


log block manager: do not fail block creation if truncate fails

RETURN_NOT_OK here means WritableBlock::Close will fail unnecessarily. I
don't _think_ that could lead to on-disk corruption (the container has to be
full, so no blocks will be added to it after this one), but it'll cause some
inconsistency in metrics.

Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd
Reviewed-on: http://gerrit.cloudera.org:8080/7312
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/fs/log_block_manager.cc
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Use dynamic bind address for internal + external mini clusters

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: Use dynamic bind address for internal + external mini clusters
..


Use dynamic bind address for internal + external mini clusters

This technique helps avoid port conflicts. It assigns a unique loopback
IP, based on pid and per-cluster numbering, to every InternalMiniCluster
or ExternalMiniCluster component (including masters and tablet servers).

Changes:

1. Apply the "dynamic bind address" technique to internal mini clusters
   (previously it was only used for external mini clusters).
2. Apply it to masters (previously it was only used for tablet servers).

I will post a follow-up patch to remove some of the CMakeLists.txt
limitations related to port conflicts, although we may need to put some
back due to lack of macOS support for unique loopback addresses.

I had to refactor a few host-related APIs to do this in a relatively
non-hacky manner. All tests still pass (and should more often).

Additional changes:

* Move BindMode to MiniCluster base class.
* Fix a few brittle tests that started failing because of this change.

Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0
Reviewed-on: http://gerrit.cloudera.org:8080/7274
Tested-by: Mike Percy 
Reviewed-by: Todd Lipcon 
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster-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/internal_mini_cluster.cc
M src/kudu/integration-tests/internal_mini_cluster.h
M src/kudu/integration-tests/master_migration-itest.cc
M src/kudu/integration-tests/master_replication-itest.cc
A src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/master/master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/master/mini_master.h
M src/kudu/master/sys_catalog-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager-test.cc
20 files changed, 316 insertions(+), 192 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Use dynamic bind address for internal + external mini clusters

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Use dynamic bind address for internal + external mini clusters
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR](branch-1.4.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
Reviewed-on: http://gerrit.cloudera.org:8080/7319
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [mini {master,tablet server}] webserver binding change

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: [mini_{master,tablet_server}] webserver binding change
..


Patch Set 2:

Instead of RUN_SERIAL we should probably mark them with a RESOURCE_LOCK 
"bind_wildcard" or something. That way at least they can run with other 
non-conflicting tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a655754687badb6870747c18fb44488c68373b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [mini {master,tablet server}] webserver binding change

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: [mini_{master,tablet_server}] webserver binding change
..


Patch Set 2:

I think once we merge https://gerrit.cloudera.org/7274 then we just need to 
mark any tests binding to WILDCARD (0.0.0.0) as RUN_SERIAL and this will be 
solved.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a655754687badb6870747c18fb44488c68373b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No


[kudu-CR] Use dynamic bind address for internal + external mini clusters

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Use dynamic bind address for internal + external mini clusters
..


Patch Set 6: Verified+1

overriding flaky test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

2017-06-28 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/7252/2/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

Line 89: 
::testing::Values("--block_manager=log","--block_manager=file"));
You will need to avoid --block_manager=log on macOS. You can do that with 
something like this:

#if defined(__APPLE__)
static const char* block_mgr_flags[] = {"--block_manager=file"};
#else
static const char* block_mgr_flags[] = {"--block_manager=log",
   "--block_manager=file"};
#endif

INSTANTIATE_TEST_CASE_P(BlockManagerType,
TsRecoveryITest,
::testing::ValuesIn(block_mgr_flags));


Line 96:   // Add the block manager type
Nit: period at end of sentence


Line 137: 
Nit: spurious extra line


PS2, Line 142:   vector extra_tserver_flags = {};
 :   extra_tserver_flags.emplace_back(GetParam());
instead of copy / pasting this in each test can we do this in the test 
constructor and have a vector with tserver flags ready to use here?


Line 145: 
Nit: spurious extra line


Line 161:   cluster_->SetFlag(cluster_->tablet_server(0),
If this is a test fix, please split it into its own commit instead of including 
it with this unrelated patch.


Line 221:   
cluster_->tablet_server(0)->mutable_flags()->push_back(parameter_flag);
Are you sure this is required? I think this is only for the dynamic flags we 
set using SetFlag().


PS2, Line 540: {
nit: space before opening brace


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes


[kudu-CR](branch-1.2.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
Reviewed-on: http://gerrit.cloudera.org:8080/7317
Reviewed-by: Todd Lipcon 
Tested-by: Will Berkeley 
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)

Approvals:
  Will Berkeley: Verified
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR](branch-1.2.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


Patch Set 1:

> Build Failed
 > 
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/8773/ : FAILURE

Internal maven problem- C++ tests were good so I'm considering this good.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR](branch-1.2.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR](branch-1.4.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR](branch-1.2.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [java] separating Connection

2017-06-28 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: [java] separating Connection
..


Patch Set 17: Code-Review+1

LGTM, does Todd or JD want to take another look?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [docs] Add an upgrade note about MM threads

2017-06-28 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: [docs] Add an upgrade note about MM threads
..


[docs] Add an upgrade note about MM threads

Some users configure MM threads to high values to work around the fact
that the MM was slow at scheduling tasks. This is now fixed in 1.4, so
this patch adds a note about this.

Change-Id: I5b9c36e04d24d0bc9991f19da35b29b474dd6022
Reviewed-on: http://gerrit.cloudera.org:8080/7283
Reviewed-by: Adar Dembo 
Reviewed-by: Todd Lipcon 
Tested-by: Jean-Daniel Cryans 
---
M docs/release_notes.adoc
1 file changed, 9 insertions(+), 0 deletions(-)

Approvals:
  Jean-Daniel Cryans: Verified
  Adar Dembo: Looks good to me, but someone else must approve
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5b9c36e04d24d0bc9991f19da35b29b474dd6022
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: Todd Lipcon 


[kudu-CR] [docs] Add an upgrade note about MM threads

2017-06-28 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: [docs] Add an upgrade note about MM threads
..


Patch Set 2: Verified+1

Overriding Jenkins since this is a docs change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b9c36e04d24d0bc9991f19da35b29b474dd6022
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..

KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley 


[kudu-CR](branch-1.4.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..

KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Will Berkeley 


[kudu-CR](branch-1.2.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

2017-06-28 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-2049 Fix too-strict CHECK in 
RleIntBlockDecoder::SeekToPositionInBlock
..

KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock

In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.

Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.

Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.

Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Will Berkeley 


[kudu-CR] [java] update dependencies and clean up some deprecate method usage

2017-06-28 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: [java] update dependencies and clean up some deprecate method 
usage
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No