[kudu-CR] consensus: move more logic from ReplicaState to RaftConsensus

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: consensus: move more logic from ReplicaState to RaftConsensus
..

consensus: move more logic from ReplicaState to RaftConsensus

ReplicaState currently has two main responsibilities:
* manage the set of pending transactions, including committing
  and aborting them. The main state here is the pending_txns_
  map.
* act as a wrapper to ConsensusMeta (stored as cmeta_).

These two items are relatively distinct. I drew a graph of the
dependencies between methods and members of ReplicaState[1] and it
became clear that most methods either pertained to one or the
other of these responsibilities, but not both. So, this patch seeks to
try to clean up the methods which were straddling the two
responsibilities as follows:

1. AbortOpsAfterUnlocked, AddPendingOperation, AdvanceCommittedIndexUnlocked

  These three are mainly focused around managing pending transactions,
  but were also taking care of setting the pending and committed Raft
  configuration in the case of configuration change operations.

  The patch moves the handling of configuration changes into
  RaftConsensus itself by using the existing "replicated callback" to
  perform the necessary work when the operation finishes, and by
  checking for config changes before adding pending operations.

4. CheckHasCommittedOpInCurrentTermUnlocked

  This one is used to check whether the last committed operation is in
  the current term. Recently the PeerMessageQueue became responsible for
  tracking the committed OpId, and already has the necessary state to
  provide the same functionality.

After the refactor, the class clearly shows two separate groups of
methods and members[2]. This will make it easier to maintain/understand
the code as well as open more opportunities for finer-grained locking by
having smaller classes with better-defined responsibilities.

I looped raft_consensus-itest 100 times with this patch and passed
100%[3].

NOTE: for the sake of the dependency graphs, I removed the state_
variable, ToString methods, and locking methods and left only the
"important" bits.

[1] http://i.imgur.com/fpyWwyM.png
[2] http://i.imgur.com/7K9Dc5J.png
[3] http://dist-test.cloudera.org/job?job_id=todd.1476324853.3445

Change-Id: I0f377ebb132f3f58f984605197831f41198d78c5
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/raft_consensus-itest.cc
7 files changed, 122 insertions(+), 99 deletions(-)


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

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

[kudu-CR] consensus: split ReplicaState in twain[1]

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: consensus: split ReplicaState in twain[1]
..

consensus: split ReplicaState in twain[1]

Following on the previous refactor, this splits out the parts of the
ReplicaState class that deal with pending consensus rounds into a new
PendingRounds class.

The change is mostly mechanical. The new class is made non-thread-safe
and we still use the synchronization of the ReplicaState class to
protect it. Since all of the methods dealing with pending rounds were
already 'Unlocked', this should be safe. The goal is to later make the
synchronization more fine-grained.

To that end, all of the methods were renamed to remove the 'Unlocked'
suffix.

There were several places in RaftConsensus which were using the
committed index/term tracked by PendingRounds, but it seems plausible
that these would be better tracked elsewhere. I left TODOs to deal with
this rather than making non-mechanical changes in this patch.

[1] https://www.youtube.com/watch?v=ZXRb6nPcx10

Change-Id: I95308ae8a5d65ada016ae08e0e8cf06c54b35909
---
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/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
5 files changed, 290 insertions(+), 300 deletions(-)


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

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

[kudu-CR] consensus peers: a little cleanup of cruft

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: consensus_peers: a little cleanup of cruft
..

consensus_peers: a little cleanup of cruft

* removes a test method that no longer was necessary
* removes an unimplemented method
* cleans up comments
* makes PeerManager methods non-virtual now that we no longer try to
  mock them

In particular I removed a big "state diagram" that I didn't find very
helpful and replaced it with a bit more text.

This is in prep for some work on KUDU-699.

Change-Id: Ie01c24b840e456482c12f96f3e2bcb3ad4388f0b
---
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/peer_manager.h
4 files changed, 23 insertions(+), 67 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01c24b840e456482c12f96f3e2bcb3ad4388f0b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] consensus: remove bits of dead code

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: consensus: remove bits of dead code
..

consensus: remove bits of dead code

We had some old code path in which PeerMessageQueue::AppendOperations()
would previously return ServiceUnavailable indicating some kind of
memory limit. However, I traced through the code and could only find one
case in which it would return ServiceUnavailable: if the log itself is
shut down.

We expect that the lifecycle of the log is controlled such that this
will never happen. So, the code path that tried to handle this is now
just a CHECK, and I was able to remove some code that was only called in
this (dead) case.

Additionally removes some other unused typedefs and methods in
ReplicaState.

Change-Id: I8dc0a129f4f136256083a31e4b4b27e6a673dbee
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
3 files changed, 4 insertions(+), 47 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8dc0a129f4f136256083a31e4b4b27e6a673dbee
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] ITBLL: use a faster PRNG

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: ITBLL: use a faster PRNG
..

ITBLL: use a faster PRNG

The SecureRandom PRNG is very very slow. Since we don't need
cryptographic random numbers, we can use a simpler random. This switches
to Xoroshiro128+, which is much faster but still has a long period
(2^128) necessary to avoid collisions.

The implementation is from Squidlib[1] which is Apache licensed. The
file itself has a license header indicating it is public domain. The
original C implementation is also in the public domain[2]. Given that, I
didn't reference this in LICENSE.txt or NOTICE.

It's copy-pasted rather than introduced as a dependency because SquidLib
is a library for writing turn-based games in Swing -- it just happens to
have a good RNG in it.

[1] 
https://github.com/SquidPony/SquidLib/blob/master/squidlib-util/src/main/java/squidpony/squidmath/XoRoRNG.java
at revision b4fb31efe527d3298b8f37f3cc72e957579ad6e3
[2] http://xoroshiro.di.unimi.it/xoroshiro128plus.c

Change-Id: I2f51664af25b9fb4309dd78556e954bf483d22c0
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 45 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f51664af25b9fb4309dd78556e954bf483d22c0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 

[kudu-CR] KUDU-1365. Add leader pre-elections

[Todd Lipcon (Code Review)]

Hello Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-1365. Add leader pre-elections
..

KUDU-1365. Add leader pre-elections

This implements the "pre-election" extension to the Raft algorithm.
The idea is that, before calling a leader election, a candidate first
sends a pre-election vote request to all voters. The voters respond
as they would have in a real vote, except they don't actually record
their vote.

Cluster tests verify that this substantially reduces the election storms
after a node has a temporarily slow disk or otherwise freezes.

A new experimental flag 'raft_enable_pre_election' is introduced which
defaults the feature to on, but provides a safety valve to disable this
in case we find some bug after release.

Tested this patch (along with the following series of cleanups) with:
 [1] 1000 loops of RaftConsensusITest.MultiThreadedInsertWithFailovers
 [2] 500 loops of raft-consensus-itest overall
 [3] 1000 loops of exactly_once_writes-itest

The above tests have usually been pretty good about finding bugs in
consensus in the past.

[1] http://dist-test.cloudera.org//job?job_id=todd.1476502616.16080
[2] http://dist-test.cloudera.org//job?job_id=todd.1476503111.17428
[3] http://dist-test.cloudera.org//job?job_id=todd.1476503268.18454

Change-Id: Ifcfabd8c9ffe31f17ab768542a046426f656db43
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/leader_election-test.cc
M src/kudu/consensus/leader_election.cc
M src/kudu/consensus/leader_election.h
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/integration-tests/raft_consensus-itest.cc
M src/kudu/tserver/tablet_service.cc
10 files changed, 309 insertions(+), 144 deletions(-)


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

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

[kudu-CR] consensus: remove bits of dead code

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: consensus: remove bits of dead code
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4712/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

Line 596:   CHECK(!s.IsServiceUnavailable()) << LogPrefixUnlocked() << "Log is 
shut down: " << s.ToString();
> can we consolidate this check? seems like we're fataling here and below
Done. Actually decided that there are no errors here we need to try to handle.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8dc0a129f4f136256083a31e4b4b27e6a673dbee
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] consensus: move more logic from ReplicaState to RaftConsensus

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: consensus: move more logic from ReplicaState to RaftConsensus
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4709/1/src/kudu/consensus/consensus_queue.h
File src/kudu/consensus/consensus_queue.h:

Line 65: // TODO Right now this class is able to track one outstanding operation
> warning: missing username/bug in TODO [google-readability-todo]
Done


PS1, Line 257: bool IsCommittedIndexInCurrentTerm() const;
> Is there an opportunity for the caller and the queue to disagree on what is
I don't think so because the current term is advanced only under the lock held 
by RaftConsensus, and this is called by RaftConsensus.


http://gerrit.cloudera.org:8080/#/c/4709/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

Line 618: 
DCHECK(round->replicate_msg()->change_config_record().has_old_config());
> are these DCHECKs really necessary? old/new config are required fields of C
I just moved this code. I think the idea is that since this might be a proto 
that we created on our side, it hasn't necessarily been verified to be fully 
initialized yet (so the fact that the fields are required doesn't mean that 
they're necessarily set). I'm guessing this was added because of some bug where 
we were missing these fields in some code path.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f377ebb132f3f58f984605197831f41198d78c5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
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 peers: a little cleanup of cruft

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: consensus_peers: a little cleanup of cruft
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4704/2/src/kudu/consensus/peer_manager.h
File src/kudu/consensus/peer_manager.h:

PS2, Line 45: queue into the local log/remote machines.
> ".. from the local queue to replicate it to remote machines"?
Done


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

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

[kudu-CR] KUDU-1508: script for testing presence of bug and finding upper bounds

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1508: script for testing presence of bug and finding upper 
bounds
..


Patch Set 1:

(5 comments)

can you write in the commit message or in the test header something about what 
the finding here means? if I'm interpreting it correctly, we're trying to find 
the max number of extents in the tree before it becomes multi-level, and that's 
dependent on the file system's block size?

http://gerrit.cloudera.org:8080/#/c/4730/1/src/kudu/experiments/KUDU-1508/hole_punch_range.c
File src/kudu/experiments/KUDU-1508/hole_punch_range.c:

Line 32: fprintf(stderr, "usage: %s
\n", argv[0]);
clarify whether the file should exist or not, and how large it should be?


Line 40:   int fd = open(argv[1], O_EXCL | O_WRONLY, 0644);
O_EXCL without O_CREAT is undefined


Line 54:   for (block_num = start_block; block_num < end_block; block_num += 
stride) {
any reason not to use C99 here?


http://gerrit.cloudera.org:8080/#/c/4730/1/src/kudu/experiments/KUDU-1508/run_test.sh
File src/kudu/experiments/KUDU-1508/run_test.sh:

PS1, Line 70: 1
instead of '1' how about using a string like 'fail_ok' so it's clearer

alternatively what about just using something like 'sudo umount || :'


PS1, Line 77: -l 
can you use the longer form (--length) here and elsewhere for flags that aren't 
well-known (eg I think mkfs -t or mount -o are commonly used)


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

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

[kudu-CR] KUDU-1508: script for testing presence of bug and finding upper bounds

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1508: script for testing presence of bug and finding upper 
bounds
..

KUDU-1508: script for testing presence of bug and finding upper bounds

This patch introduces a simple script that tests for the presence of
KUDU-1508.

When run on my laptop (Ubuntu 16.04):

  This kernel is not vulnerable to KUDU-1508, skipping test

When run on ve0518 (an el6.6 box):

  This kernel is vulnerable to KUDU-1508, finding per-block size upper bounds
  Block size 1024: searching for block number upper bound (MIN=0,MAX=16384)
  Block size 1024: 8192 bad (MIN=0,MAX=16384)
  Block size 1024: 4095 bad (MIN=0,MAX=8191)
  Block size 1024: 2047 bad (MIN=0,MAX=4094)
  Block size 1024: 1023 bad (MIN=0,MAX=2046)
  Block size 1024: 511 good (MIN=0,MAX=1022)
  Block size 1024: 767 bad (MIN=512,MAX=1022)
  Block size 1024: 639 good (MIN=512,MAX=766)
  Block size 1024: 703 bad (MIN=640,MAX=766)
  Block size 1024: 671 good (MIN=640,MAX=702)
  Block size 1024: 687 bad (MIN=672,MAX=702)
  Block size 1024: 679 bad (MIN=672,MAX=686)
  Block size 1024: 675 bad (MIN=672,MAX=678)
  Block size 1024: 673 good (MIN=672,MAX=674)
  Block size 1024: upper bound found at 673
  Block size 2048: searching for block number upper bound (MIN=0,MAX=16384)
  Block size 2048: 8192 bad (MIN=0,MAX=16384)
  Block size 2048: 4095 bad (MIN=0,MAX=8191)
  Block size 2048: 2047 bad (MIN=0,MAX=4094)
  Block size 2048: 1023 good (MIN=0,MAX=2046)
  Block size 2048: 1535 bad (MIN=1024,MAX=2046)
  Block size 2048: 1279 good (MIN=1024,MAX=1534)
  Block size 2048: 1407 bad (MIN=1280,MAX=1534)
  Block size 2048: 1343 good (MIN=1280,MAX=1406)
  Block size 2048: 1375 bad (MIN=1344,MAX=1406)
  Block size 2048: 1359 bad (MIN=1344,MAX=1374)
  Block size 2048: 1351 good (MIN=1344,MAX=1358)
  Block size 2048: 1355 bad (MIN=1352,MAX=1358)
  Block size 2048: 1353 good (MIN=1352,MAX=1354)
  Block size 2048: upper bound found at 1353
  Block size 4096: searching for block number upper bound (MIN=0,MAX=16384)
  Block size 4096: 8192 bad (MIN=0,MAX=16384)
  Block size 4096: 4095 bad (MIN=0,MAX=8191)
  Block size 4096: 2047 good (MIN=0,MAX=4094)
  Block size 4096: 3071 bad (MIN=2048,MAX=4094)
  Block size 4096: 2559 good (MIN=2048,MAX=3070)
  Block size 4096: 2815 bad (MIN=2560,MAX=3070)
  Block size 4096: 2687 good (MIN=2560,MAX=2814)
  Block size 4096: 2751 bad (MIN=2688,MAX=2814)
  Block size 4096: 2719 good (MIN=2688,MAX=2750)
  Block size 4096: 2735 bad (MIN=2720,MAX=2750)
  Block size 4096: 2727 bad (MIN=2720,MAX=2734)
  Block size 4096: 2723 bad (MIN=2720,MAX=2726)
  Block size 4096: 2721 good (MIN=2720,MAX=2722)
  Block size 4096: upper bound found at 2721

Change-Id: I710918a153a9e8e05e989fe63281891c9ebc7178
---
A src/kudu/experiments/KUDU-1508/hole_punch_range.c
A src/kudu/experiments/KUDU-1508/run_test.sh
2 files changed, 227 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I710918a153a9e8e05e989fe63281891c9ebc7178
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add krb5 to thirdparty

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> As long as we avoid -Wl,-rpath=PREFIX_COMMON/lib (which we don't have today
There is also --libdir options for the krb5's configure.  But it seems it's no 
longer relevant anyway.


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

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

[kudu-CR] Add krb5 to thirdparty

[Dan Burkert (Code Review)]

Dan Burkert has abandoned this change.

Change subject: Add krb5 to thirdparty
..


Abandoned

We're going to require krb5kdc to be installed on test machines.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add krb5 to thirdparty

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> I guess the includes actually aren't as problematic (since we wouldn't be i
As long as we avoid -Wl,-rpath=PREFIX_COMMON/lib (which we don't have today) 
when building the thirdparty dependencies and Kudu itself, that shouldn't 
happen. But, I can understand wanting more certainty here, to avoid it silently 
slipping in.


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

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

[kudu-CR] Improve pmem cmake discovery failure message

[Dan Burkert (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.

Change subject: Improve pmem cmake discovery failure message
..

Improve pmem cmake discovery failure message

Change-Id: I51d447733b2dd20eb8fbc7e19dd17c44ead0fc82
---
M cmake_modules/FindPmem.cmake
1 file changed, 1 insertion(+), 1 deletion(-)


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

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

[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [java client] Extract RemoteTablet from AsyncKuduClient
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6
Gerrit-PatchSet: 4
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-HasComments: No

[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Extract RemoteTablet from AsyncKuduClient
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4719/3/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:

Line 55:  * Unlike the C++ client, we don't short-circuit the call to the 
master if it isn't available.
As we discussed, I'm also removing this.


Line 76:   private int leaderIndex = NO_LEADER_INDEX;
> Add @GuardedBy("tabletServers") to this.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6
Gerrit-PatchSet: 3
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-HasComments: Yes

[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [java client] Extract RemoteTablet from AsyncKuduClient
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4719/3/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:

Line 76:   private int leaderIndex = NO_LEADER_INDEX;
Add @GuardedBy("tabletServers") to this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6
Gerrit-PatchSet: 3
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-HasComments: Yes

[kudu-CR] Add krb5 to thirdparty

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> BTW, if we still have any concerns regarding this, it's possible to use --i
I guess the includes actually aren't as problematic (since we wouldn't be 
including them), but libraries are potentially problematic if they end up 
dynamically loaded. For example, SASL will probably dynamically load 
libkrb5.so, and we want it to pick up the system version, not the one that we 
installed.


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

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

[kudu-CR] [java client] Extract RemoteTablet from AsyncKuduClient

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java client] Extract RemoteTablet from AsyncKuduClient
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d06a573e4307c76a7aebab05cd5238fb0d9a2c6
Gerrit-PatchSet: 3
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-HasComments: No

[kudu-CR] Add krb5 to thirdparty

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> Dan and I discussed this: if Kudu doesn't include any krb5 headers nor link
BTW, if we still have any concerns regarding this, it's possible to use 
--includedir option for configure.  It's possible to point it to some 
non-standard subdir (or to a temporary directory and then remove it after 
build).

I remember Dan didn't like the idea of having manual pages and other stuff 
installed.  To get rid of that, it's possible to point --datarootdir 
configure's option to some temporary directory and then remove if after build 
is done.


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

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

[kudu-CR] Add krb5 to thirdparty

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> Do we need to enable/disable any of the default options while running confi
Probably, it's already known and read, but just in case: there is nice doc on 
the configuration options in doc/html/build/options2configure.html


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

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

[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: [java client] Extract ip2client out of AsyncKuduClient
..


[java client] Extract ip2client out of AsyncKuduClient

As part of making the Java client more modular and easier to test, this patch 
is moving
almost all of the connections management into a separate class. The change was 
rather
painless.

Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c
Reviewed-on: http://gerrit.cloudera.org:8080/4717
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
3 files changed, 417 insertions(+), 355 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c
Gerrit-PatchSet: 4
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

[kudu-CR] Add krb5 to thirdparty

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> Dan and I discussed this: if Kudu doesn't include any krb5 headers nor link
Do we need to enable/disable any of the default options while running 
configure?  E.g., --without-readline, --without-libedit, --without-tcl?

In other words, are we satisfied with the default set of configuration options 
and does it give us stable build regardless of the installed/present system 
packages?


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

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

[kudu-CR] Add krb5 to thirdparty

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
> does this end up putting the includes on our build's include path? I think 
Dan and I discussed this: if Kudu doesn't include any krb5 headers nor link 
against krb5 libraries, what difference does it make?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
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] Add krb5 to thirdparty

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 564:   $KRB5_SOURCE/src/configure --prefix=$PREFIX
does this end up putting the includes on our build's include path? I think we 
should avoid doing that (maybe install into a separate prefix entirely, or 
somehow pass a flag to avoid actually installing them)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
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] Add krb5 to thirdparty

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS1, Line 557: Kudu does not link
 :   # against the built libraries for security reasons.
> Well, Kudu doesn't actually need to build against it; Kudu's security depen
yea most likely we don't even need to build against GSSAPI, just against SASL 
(which we already do). The GSSAPI thing is a runtime sasl plugin


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
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] Add krb5 to thirdparty

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Add krb5 to thirdparty
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS1, Line 557: Kudu does not link
 :   # against the built libraries for security reasons.
Well, Kudu doesn't actually need to build against it; Kudu's security 
dependency list terminates with gssapi, right?


Line 560:   echo $KRB5_BDIR
Nit: drop this, debugging only right?


http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

PS1, Line 24: #   * /thirdparty/installed/common - prefix directory for 
libraries and binary tools
: #common to all build types, 
e.g. CMake,
: #krb5, C dependencies.
We should update this; C dependencies (and libraries) are no longer found here, 
just tools.


Line 105:   if [ "$PREFIX_DIR" != "$PREFIX_COMMON" ]; then
May need to remove this now, since we're actually putting stuff in 
PREFIX_COMMON/lib once again.


http://gerrit.cloudera.org:8080/#/c/4727/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

Line 256:   fetch_and_expand ${KRB5_NAME}.tar.gz
Huh, I wonder why I didn't use FOO_NAME in the rest of the entries here. Seems 
like an oversight. Would you mind fixing that?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
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] Commit CSD metrics for 1.0.0 and 1.0.1

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: Commit CSD metrics for 1.0.0 and 1.0.1
..


Commit CSD metrics for 1.0.0 and 1.0.1

Change-Id: If7e9867289522e0c0c04bcbffaf04fb249589287
Reviewed-on: http://gerrit.cloudera.org:8080/4695
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
A java/kudu-csd/old-version-metrics/1.0.0-master.json
A java/kudu-csd/old-version-metrics/1.0.0-tserver.json
A java/kudu-csd/old-version-metrics/1.0.1-master.json
A java/kudu-csd/old-version-metrics/1.0.1-tserver.json
4 files changed, 4,720 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If7e9867289522e0c0c04bcbffaf04fb249589287
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add krb5 to thirdparty

[Dan Burkert (Code Review)]

Hello Adar Dembo, Todd Lipcon,

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

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

to review the following change.

Change subject: Add krb5 to thirdparty
..

Add krb5 to thirdparty

Change-Id: I444c7da5a24fc9d554f542f300586b921cd65b6e
---
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
4 files changed, 29 insertions(+), 1 deletion(-)


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

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

[kudu-CR] [java client] Extract ip2client out of AsyncKuduClient

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java client] Extract ip2client out of AsyncKuduClient
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48b0f3f262abd5ee26869202f661b3c25158f39c
Gerrit-PatchSet: 3
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-HasComments: No

[kudu-CR] [client.h] CountBufferedOperations marked as deprecated

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [client.h] CountBufferedOperations marked as deprecated
..

[client.h] CountBufferedOperations marked as deprecated

KuduSession::CountBufferedOperations() is used only by tests now.
Also, the only flush mode to use this method consistently is
MANUAL_FLUSH mode, but in that mode it's easy to count for number of
buffered operations by a different means -- that's the count of Apply()
calls since last KuduSession::Flush() call (or invocation of
the callback passed into KuduSession::FlushAsync() method).

Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548
---
M src/kudu/client/client.h
1 file changed, 17 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [client.h] updated comment for CountBufferedOperations

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: [client.h] updated comment for CountBufferedOperations
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4723/1/src/kudu/client/client.h
File src/kudu/client/client.h:

PS1, Line 1493: This is different than HasPendingOperations() above
> Well that MJ was interested is... interesting since Impala will be using AU
Yes, I ultimately decided I will not end up using this, but thanks for 
clarifying the message. I can't imagine this being useful w/ 
AUTO_FLUSH_BACKGROUND given the semantics.

BTW, the reason I was interested in this is for us to keep track of the number 
of writes that actually went through. The idea was to compute:

total_num_rows_we_added - pending_in_buffer - total_num_errors

I'm just going to compute this at the end and then we don't worry about pending 
in buffer.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[kudu-CR] [client.h] updated comment for CountBufferedOperations

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [client.h] updated comment for CountBufferedOperations
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4723/1/src/kudu/client/client.h
File src/kudu/client/client.h:

PS1, Line 1493: This is different than HasPendingOperations() above
> Yep -- so far I can see, this method is used only in tests.  However, MJ fo
Well that MJ was interested is... interesting since Impala will be using 
AUTO_FLUSH_BACKGROUND, which this method is _not_ useful for, right?

To handle the change, I'd duplicate and mark this one deprecated.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6293c0a3058f7056d24e7cee0120d1852ded548
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1700: Debug build will not fail gracefully on Messenger::Init() failure

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1700: Debug build will not fail gracefully on 
Messenger::Init() failure
..


KUDU-1700: Debug build will not fail gracefully on Messenger::Init() failure

If Messenger::Init() fails in a debug build, a CHECK() will happen on
the destruction of the Messenger object. This happens beacuse, on an
error, the Messenger is not Shutdown().

This patch gets rid of the private function
Messenger::Build(Messenger**) and moves all its logic into the public
function Build(shared_ptr*). On an error, an explicit call
to AllExternalReferencesDropped() is made.

This case was probably never encountered as Messenger::Init()
currently has a very low chance of failing. However, in the future, as
more things may get added on to the function, this issue might show
up more often.

A more detailed explanation is given in the JIRA.

Change-Id: Id6021587c746af53305b3f601bb1bcc19f63eab0
Reviewed-on: http://gerrit.cloudera.org:8080/4724
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/messenger.h
2 files changed, 7 insertions(+), 11 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id6021587c746af53305b3f601bb1bcc19f63eab0
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon