[kudu-CR] client-test: remove an unnecessary manual leader election

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: client-test: remove an unnecessary manual leader election
..


client-test: remove an unnecessary manual leader election

One test case predates the addition of automatic leader election to
Kudu, and was manually triggering a leader election after killing a
node. This is no longer necessary, so the test can be simplified.

Change-Id: I943d331d4d8afed9d7d929df1df05aa2f3cf454e
Reviewed-on: http://gerrit.cloudera.org:8080/4141
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/client/client-test.cc
1 file changed, 0 insertions(+), 40 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

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


[kudu-CR] raft consensus-itest: workaround flakiness due to KUDU-1580

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: raft_consensus-itest: workaround flakiness due to KUDU-1580
..


raft_consensus-itest: workaround flakiness due to KUDU-1580

KUDU-1580 is a bug in which the client doesn't properly fail over in the
case that the RPC connection negotiation times out. In the
MultiThreadedInsertWithFailovers test case, the client is frequently
timing out with such errors causing flakiness.

This workaround just bumps the connection negotiation timeout.

Change-Id: If8f375654b954701ab8d23c07bd133ba393e222d
Reviewed-on: http://gerrit.cloudera.org:8080/4142
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/raft_consensus-itest.cc
1 file changed, 5 insertions(+), 0 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

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


[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: raft_consensus-itest: inserter thread should FATAL instead of 
FAIL
..


raft_consensus-itest: inserter thread should FATAL instead of FAIL

This test has a thread which inserts rows and is supposed to fail the
test case if it sees any row errors. Failing the test using FAIL() from
a non-main thread is not thread-safe in gtest. Furthermore, FAIL() acts
as a 'return' and thus the latch used to communicate the thread
completion never gets fired. So, when this assertion failed, the test
would hang forever.

This was a regression caused by d0cff255f84e75b70c0c39ccd34a35f348e3c722
which changed the code from a CHECK(...) to a FAIL().

In order to correct this behavior, this patch switches to using the
utility method FlushSessionOrDie() which has an identical
implementation.

Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b
Reviewed-on: http://gerrit.cloudera.org:8080/4140
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/raft_consensus-itest.cc
1 file changed, 1 insertion(+), 21 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] raft consensus-itest: workaround flakiness due to KUDU-1580

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: raft_consensus-itest: workaround flakiness due to KUDU-1580
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] client-test: remove an unnecessary manual leader election

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: client-test: remove an unnecessary manual leader election
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: raft_consensus-itest: inserter thread should FATAL instead of 
FAIL
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] KUDU-1586. consensus: always send at least one op

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Dan Burkert,

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

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

to review the following change.

Change subject: KUDU-1586. consensus: always send at least one op
..

KUDU-1586. consensus: always send at least one op

This fixes a bug in which the LogCache would not return any operations
in the case of a cache miss on an operation larger than the configured
batch size.

This would cause consensus to get "stuck": the leader would repeatedly
send status-only messages (i.e. no operations) to the follower in a
tight loop, never making any progress.

The new unit test reproduces the problem.

Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697
---
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_cache.cc
2 files changed, 7 insertions(+), 1 deletion(-)


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

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


[kudu-CR] KUDU-1586. consensus: always send at least one op

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1586. consensus: always send at least one op
..


Patch Set 1:

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

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

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


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

PS15, Line 313:   DCHECK_EQ(table_id_, superblock.table_id());
  :   PartitionSchemaPB partition_schema_pb;
  :   partition_schema_.ToPB(&partition_schema_pb);
  :   
DCHECK_EQ(superblock.partition_schema().SerializeAsString(),
  : partition_schema_pb.SerializeAsString());
  :   PartitionPB partition_pb;
  :   partition_.ToPB(&partition_pb);
  :   DCHECK_EQ(superblock.partition().SerializeAsString(),
  : partition_pb.SerializeAsString());
> I liked this idea, but it still doesn't prevent compat break if the wire fo
I suppose we could change this to CHECK and just rely on upgrade / downgrade 
tests for coverage on this stuff, if we don't want release and debug builds to 
act differently. After all, this isn't a hot path.

Dinesh, to your point: providing an Equals() method for the non-PB objects 
doesn't prevent forward-compatibility problems, you are right. These are just 
inherent issues we have to deal with and as far as I can tell, having upgrade 
tests are really the only way to avoid unforeseen compatibility problems.


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

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


[kudu-CR] [docs] added Kudu version into the doxygen footer

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [docs] added Kudu version into the doxygen footer
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4165/1/CMakeLists.txt
File CMakeLists.txt:

PS1, Line 79: execute_process(COMMAND git rev-parse HEAD
:   WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
:   OUTPUT_VARIABLE KUDU_SRC_GIT_HASH)
> What happens if this runs outside of a git repo, or if git isn't installed?
I didn't know that: was assuming all builds work against git-cloned repo.  Will 
re-use the generated info in version_defines.h


http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.doxy.in
File docs/support/doxygen/client_api.doxy.in:

PS1, Line 55: share/doc/kuduClient/samples/sample.cc
> Can we substitute this in as a list? Better yet if we can somehow set it fr
Certainly, good idea!  Not sure whether it's possible to set it via 
src/kudu/client/CMakeLists.txt, but as for the list from the top-level 
CMakeLists.txt -- for sure.  Will change.


http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.footer.in
File docs/support/doxygen/client_api.footer.in:

PS1, Line 6: 
   : 
> Presumably these were opened in the header?
Yes, this is how doxygen wants it:

http://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_html_footer


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

PS1, Line 21: /// @file shared_ptr.h
> Hmm, I don't recall seeing @file tags in the other files you doxygenized. W
The @file command/directive is needed here because this file contains only 
'global' entities, so without the @file they are not included into the 
generated documentation.  For details, take a look at the 'Important' note:

http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdfile


PS1, Line 22: pointers
> Nit: pointer
Done


PS1, Line 39: defined(__clang__)
> You can build Kudu with gcc on macOS? Didn't know that was possible.
Ah, it seems that's from my attempts to do that.  I worked some asm issues, but 
I stopped with glog libraries.  I'll remove this -- thanks.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] compaction policy: fix bound calculation

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: compaction_policy: fix bound calculation
..


Patch Set 1:

(2 comments)

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

PS1, Line 160: fractional_solution_
Nit: why not to use priority_queue for that as a handy wrapper?


PS1, Line 161: >=
So, if the resulting weight is equal to the specified max_weight, then it's not 
a solution?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #174
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Cleanup/refactor tracking of consensus watermarks

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Cleanup/refactor tracking of consensus watermarks
..

Cleanup/refactor tracking of consensus watermarks

This is a fairly invasive cleanup/refactor to consensus in preparation
for propagating replication watermarks from the leader to the followers.

The key item here is to address a long-standing TODO that the
PeerMessageQueue should itself calculate the commit index, rather than
delegate that job to ReplicaState. The flow used to be something like
this on the leader upon receiving a response from a peer that it has
replicated some new operations:

 - PeerMessageQueue::ResponseFromPeer
   - updates peer last_received
   - calls PeerMessageQueue::AdvanceQueueWatermark
 - updates PeerMessageQueue::majority_replicated to the new majority
   watermark
   - calls NotifyObserversOfMajorityReplOp
   --> submits an async task to the raft threadpool
- for each observer (best I can tell there is currently always
  only one, but that's a separate issue)
-- observer->UpdateMajorityReplicated() to grab committed index
   (the observer is always the RaftConsensus instance)
--- RaftConsensus::UpdateMajorityReplicated
 ReplicaState::UpdateMajorityReplicatedUnlocked
- this checks the condition that the majority-replicated
  watermark is within the leader's term before advancing the
  commit index.
- calls AdvanceCommittedIndexUnlocked
--- commits stuff, etc
- sets *committed_index out-param
-- PeerMessageQueue picks up this out-param and sets the new
   value within the PeerMessageQueue

This was very difficult to follow, given that the "observer" in fact was
passing data back to the "notifier" through an out-parameter. Moreover,
it wasn't obvious to see which class was "authoritative" for the
tracking and advancement of watermarks.

The new design makes the PeerMessageQueue itself fully responsible for
calculating when the commit index can advance. This required a fairly
invasive surgery because the PeerMessageQueue itself doesn't remember
the terms of pending operations, and thus the watermarks became indexes
instead of OpIds.

This is itself also a simplification -- we previously were very messy
about using the word "index" when in fact we had an OpId type. In almost
every case, we only used the index of those OpIds. In the Raft paper
itself, these watermarks are just indexes and not OpIds, so this change
brings us closer to the implementation described in the original design.

Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8
---
M docs/release_notes.adoc
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus-test.cc
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
M src/kudu/integration-tests/raft_consensus-itest.cc
15 files changed, 400 insertions(+), 569 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Cleanup/refactor tracking of consensus watermarks

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Cleanup/refactor tracking of consensus watermarks
..


Patch Set 4:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: port log-dump

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: port log-dump
..


Patch Set 1:

Code lgtm. Can you add this to the release note with a pointer to 'kudu wal 
dump' and 'kudu tablet dump_wals'?

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

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


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without flags unlocked:
  E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and 
unsupported.
  E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use.
  E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to 
proceed at your own risk.
  E0824 14:04:57.264104 14821 flags.cc:296] Flag 
--local_ip_for_outbound_sockets is experimental and unsupported.
  E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use.
  E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to 
proceed at your own risk.
  

Example error output with flags unlocked:
  W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: 
--never_fsync=true
  W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: 
--local_ip_for_outbound_sockets=127.0.0.1
  

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Reviewed-on: http://gerrit.cloudera.org:8080/4100
Reviewed-by: Adar Dembo 
Tested-by: Todd Lipcon 
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/flag_tags.h
M src/kudu/util/flags.cc
8 files changed, 136 insertions(+), 5 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 9: Verified+1

Hit a jenkins bug in the lint build (but previous lint builds passed)

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

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


[kudu-CR] Predicate evaluation pushdown

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Predicate evaluation pushdown
..


Patch Set 7:

(31 comments)

http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/binary_plain_block.cc
File src/kudu/cfile/binary_plain_block.cc:

Line 306: Status BinaryPlainBlockDecoder::CopyNextValues(size_t *n, 
ColumnDataView *dst) {
I wonder whether this function could share code with the new one using some 
templating (to force that they're separately instantiated)


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/binary_prefix_block.h
File src/kudu/cfile/binary_prefix_block.h:

Line 99: cur_idx_ += *n;
surprised this doesn't need to update cur_val_ and next_ptr_


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

Line 488:   ctx->SetDecoderEvalNotSupported();
shouldn't this be done in PrepareEvalContext?


PS7, Line 489: ctx->block()
given the number of times you use ctx->block() in this function I think it's 
worth just doing: ColumnBlock* dst = ctx->block()


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/cfile_reader.h
File src/kudu/cfile/cfile_reader.h:

Line 355:   // Sets context if needed. This must happen before a call to
no need for this line (since it's already described in the interface above)


PS7, Line 390: ;
nit: space after ;


PS7, Line 463: or>cod
nit: space after >


Line 480:   // Predicate and selection vector associated with the iterator
would this ever be null? worth a comment explaining when it might be


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/rle_block.h
File src/kudu/cfile/rle_block.h:

Line 190:   void SeekForward(size_t *n) override {
hrm, I'm skeptical of this function, I'd think it would need to update the 
rle_decoder_ as well.

Maybe update encoding-test so that this is covered?


Line 413:   void SeekForward(size_t *n) override {
same


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/common/column_eval_context.h
File src/kudu/common/column_eval_context.h:

Line 26: // get passed down to the decoders during predicate push-down.
this is now more generally the set of objects passed down to the column 
iterator and decoder during column materialization, right?


Line 31: public:
nit: indent one


Line 43:   // Column index in the parent CFileSet.
I think this is incorrect and it's actually the index within the projection 
schema, rather than within the CFileSet. Perhaps we should call it 
projection_idx or something to clarify?


Line 51:   // I.e. BinaryDictBlockDecoder.CopyNextAndEval(), 
CFileIterator.Scan()
not quite following this comment. when is this used outside of scans?


Line 54:   // Selection vector reflecting the result of the predicate 
evaluation.
should clarify whether this is an "out" or an "in-out" type parameter.

In other words, if the scanner sees that a column is already non-selected in 
here, is it free to skip reading that row? I'm guessing so, but good to clarify.


Line 57:   // Must be greater than size 0 and must be large enough to cover the 
number
isn't "greater than size 0" redundant with "large enough to cover the number of 
rows"?


Line 63:   // materializing_iterator_decoder_eval to false, forcing evaluation 
to fall
I think this comment needs an update


Line 74: if (decoder_eval_status_ == kNotSet) {
it seems weird that this function is a silent no-op in the case that it's 
already been set. If it's set in a "conflicting" manner, what happens? seems 
like it should either be a DCHECK, or by a 'TrySetDecoderEvalSupported' type 
function that returns a bool about whether it successfully changed modes or not.


Line 79:   void SetDecoderEvalNotSupported() {
particularly seems like here it should be a CHECK that it's kNotSet. Otherwise 
you might try to switch from supported to non-supported, and only partially 
have filled in the selection vector


http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:

Line 26: #include "kudu/common/column_eval_context.h"
nit: sort the includes


PS7, Line 522:  = ColumnEvalContext
can just be:

ColumnEvalContext ctx(...)

instead of the extra temporary construction and assignment


Also, given that we now use ColumnEvalContext for all materialization, and not 
just the one with predicate evaluation, we should rename to 
ColumnMaterializationContext


Line 526: if (ctx.pred()->predicate_type() == PredicateType::None) {
does a None predicate ever make it this far into the code? I would think that 
the None would be handled way up above this layer (and this could be a DCHECK)


Line 549: ColumnEvalContext ctx = ColumnEvalContext(col_idx,
same


Line 553: ctx.SetDecoderEvalNotSupported();
why's this necessary? shouldn't the nullptr predicate be sufficient for the 
code to know it doesn't need to evaluate any predicate?


http://gerrit.cloudera

[kudu-CR] tool: port log-dump

2016-08-29 Thread Adar Dembo (Code Review)
Hello Todd Lipcon,

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

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

to review the following change.

Change subject: tool: port log-dump
..

tool: port log-dump

This one was more complicated, because log-dump can run against a single
file or an entire tablet. So I put all the common functionality in one place
and referenced it from both modes.

I consolidated similar gflags where it made sense to do so, and I tweaked
the endline handling for help generation so that each parameter is separated
from the next with an empty line.

Semantic changes from log-dump:
- If called with print_entries=no, will also print the footer.
- The print_headers flag is now print_meta, to be more consistent with 'kudu
  cfile dump'.

Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9
---
M src/kudu/consensus/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
R src/kudu/tools/tool_action_common.cc
A src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
M src/kudu/util/test_macros.h
12 files changed, 434 insertions(+), 160 deletions(-)


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

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


[kudu-CR] tool: port log-dump

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: port log-dump
..


Patch Set 1:

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

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

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


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 9:

arg, one more conflict resolution!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 9:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Mike Percy, Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without flags unlocked:
  E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and 
unsupported.
  E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use.
  E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to 
proceed at your own risk.
  E0824 14:04:57.264104 14821 flags.cc:296] Flag 
--local_ip_for_outbound_sockets is experimental and unsupported.
  E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use.
  E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to 
proceed at your own risk.
  

Example error output with flags unlocked:
  W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: 
--never_fsync=true
  W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: 
--local_ip_for_outbound_sockets=127.0.0.1
  

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/flag_tags.h
M src/kudu/util/flags.cc
8 files changed, 136 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Inlined dispatch for predicate evaluation

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Inlined dispatch for predicate evaluation
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] Inlined dispatch for predicate evaluation

2016-08-29 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Inlined dispatch for predicate evaluation
..

Inlined dispatch for predicate evaluation

In order to evaluate a predicate, we must first determine the correct
comparator to handle the physical type of interest. Currently, this occurs once
per batch when ColumnPredicate::Evaluate() is called. A call to this will
dispatch the comparator as a lambda to be applied in a loop over all rows in
the batch.

For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch
occurs for each cell. This particularly slows down decoder-level evaluation,
which falls back on evaluating cell-by-cell rather than in batches. To
remediate this, the dispatch has been templatized here to remediate the cost of
repeated dispatch.

This figure shows the performance of dictionary encoding for decoder-level
evaluation without this templating adjustment. The query selects a single value
out of a dictionary-encoded column containing 1M unique strings.
https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png

Compare the above with the plot on the right, which is the result of the same
query, but with inlined dispatch.
https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png

Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
---
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
4 files changed, 83 insertions(+), 44 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: raft_consensus-itest: inserter thread should FATAL instead of 
FAIL
..


Patch Set 1: Code-Review+1

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

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


[kudu-CR] transaction tracker: back-off when logging in-flight transactions

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: transaction_tracker: back-off when logging in-flight 
transactions
..


Patch Set 1: Code-Review+1

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

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


[kudu-CR] [docs] added Kudu version into the doxygen footer

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

Change subject: [docs] added Kudu version into the doxygen footer
..


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4165/1/CMakeLists.txt
File CMakeLists.txt:

PS1, Line 79: execute_process(COMMAND git rev-parse HEAD
:   WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
:   OUTPUT_VARIABLE KUDU_SRC_GIT_HASH)
What happens if this runs outside of a git repo, or if git isn't installed? 
Some of our builds don't work directly against git.

Also, I think it would be better if we could rely on version_defines.h (see 
util/CMakeLists.txt) for this information instead of generating it again.


http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.doxy.in
File docs/support/doxygen/client_api.doxy.in:

PS1, Line 55: share/doc/kuduClient/samples/sample.cc
Can we substitute this in as a list? Better yet if we can somehow set it from 
within src/kudu/client/CMakeLists.txt, which is where sample.cc is placed in 
the installation output.


http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.footer.in
File docs/support/doxygen/client_api.footer.in:

PS1, Line 6: 
   : 
Presumably these were opened in the header?


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

PS1, Line 21: /// @file shared_ptr.h
Hmm, I don't recall seeing @file tags in the other files you doxygenized. What 
is its purpose here?


PS1, Line 22: pointers
Nit: pointer


PS1, Line 39: defined(__clang__)
You can build Kudu with gcc on macOS? Didn't know that was possible.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [docs] added Kudu version into the doxygen footer

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: [docs] added Kudu version into the doxygen footer
..


Patch Set 2:

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

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

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


[kudu-CR] [docs] added Kudu version into the doxygen footer

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new patch set (#2).

Change subject: [docs] added Kudu version into the doxygen footer
..

[docs] added Kudu version into the doxygen footer

Added information on the Kudu source version into the HTML footer
of the auto-generated doxygen docs of the Kudu C++ API.

Removed sample.cc file from the list of files to process with doxygen.

Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f
---
M CMakeLists.txt
M docs/support/doxygen/client_api.doxy.in
A docs/support/doxygen/client_api.footer.in
M src/kudu/client/shared_ptr.h
4 files changed, 42 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [docs] added Kudu version into the doxygen footer

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: [docs] added Kudu version into the doxygen footer
..


Patch Set 1:

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

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

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


[kudu-CR] [docs] added Kudu version into the doxygen footer

2016-08-29 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: [docs] added Kudu version into the doxygen footer
..

[docs] added Kudu version into the doxygen footer

Added information on the Kudu source version into the HTML footer
of the auto-generated doxygen docs of the Kudu C++ API.

Removed sample.cc file from the list of files to process with doxygen.

Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f
---
M CMakeLists.txt
M docs/support/doxygen/client_api.doxy.in
A docs/support/doxygen/client_api.footer.in
M src/kudu/client/shared_ptr.h
4 files changed, 46 insertions(+), 7 deletions(-)


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

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


[kudu-CR] Inlined dispatch for predicate evaluation

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Inlined dispatch for predicate evaluation
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] Inlined dispatch for predicate evaluation

2016-08-29 Thread Andrew Wong (Code Review)
Andrew Wong has uploaded a new change for review.

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

Change subject: Inlined dispatch for predicate evaluation
..

Inlined dispatch for predicate evaluation

In order to evaluate a predicate, we must first determine the correct
comparator to handle the physical type of interest. Currently, this occurs once
per batch when ColumnPredicate::Evaluate() is called. A call to this will
dispatch the comparator as a lambda to be applied in a loop over all rows in
the batch.

For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch
occurs for each cell. This particularly slows down decoder-level evaluation,
which falls back on evaluating cell-by-cell rather than in batches. To
remediate this, the dispatch has been templatized here to remediate the cost of
repeated dispatch.

This figure shows the performance of dictionary encoding for decoder-level
evaluation without this templating adjustment. The query selects a single value
out of a dictionary-encoded column containing 1M unique strings.
https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png

Compare the above with the plot on the right, which is the result of the same
query, but with inlined dispatch.
https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png

Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
---
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
4 files changed, 86 insertions(+), 48 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 


[kudu-CR] compaction policy: fix bound calculation

2016-08-29 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: compaction_policy: fix bound calculation
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Anonymous Coward #174
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


KUDU-1048 master should show versions of tservers, version summary

This patch adds a version summary table and a total count of
registered tablet servers to /tablet-servers.

It also fixes the display of the registration, which was printing in
red font.

Sample: 
https://raw.githubusercontent.com/wdberkeley/kudu-cr/master/KUDU-1048-improved.png

Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Reviewed-on: http://gerrit.cloudera.org:8080/4104
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_descriptor.h
M src/kudu/master/ts_manager.cc
4 files changed, 46 insertions(+), 14 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


Patch Set 3: Code-Review+2

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

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


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 8: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'

2016-08-29 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: tool: port cfile-dump to 'kudu fs dump_cfile'
..


tool: port cfile-dump to 'kudu fs dump_cfile'

Some non-cosmetic changes:
- I changed the block_id conversion into something nicer than a CHECK.
- The block_id parameter is expected in base 10, not base 16. To be honest,
  cfile-dump should have used base 10 for quite some time, because that's
  how they're printed in dumped PBs.
- I dropped the num_iterations parameter because it didn't seem useful.

Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e
Reviewed-on: http://gerrit.cloudera.org:8080/4151
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/cfile/CMakeLists.txt
D src/kudu/cfile/cfile-dump.cc
M src/kudu/cfile/cfile-test-base.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
5 files changed, 131 insertions(+), 106 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: port cfile-dump to 'kudu fs dump_cfile'
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Mike Percy, Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without flags unlocked:
  E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and 
unsupported.
  E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use.
  E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to 
proceed at your own risk.
  E0824 14:04:57.264104 14821 flags.cc:296] Flag 
--local_ip_for_outbound_sockets is experimental and unsupported.
  E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use.
  E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to 
proceed at your own risk.
  

Example error output with flags unlocked:
  W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: 
--never_fsync=true
  W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: 
--local_ip_for_outbound_sockets=127.0.0.1
  

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/flag_tags.h
M src/kudu/util/flags.cc
8 files changed, 136 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Mike Percy, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without flags unlocked:
  E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and 
unsupported.
  E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use.
  E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to 
proceed at your own risk.
  E0824 14:04:57.264104 14821 flags.cc:296] Flag 
--local_ip_for_outbound_sockets is experimental and unsupported.
  E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use.
  E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to 
proceed at your own risk.
  

Example error output with flags unlocked:
  W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: 
--never_fsync=true
  W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: 
--local_ip_for_outbound_sockets=127.0.0.1
  

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/flag_tags.h
M src/kudu/util/flags.cc
8 files changed, 136 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: port cfile-dump to 'kudu fs dump_cfile'
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..


Patch Set 23:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

2016-08-29 Thread Alexey Serbin (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..

KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode

Implemented AUTO_FLUSH_BACKGROUND for the Kudu C++ client library.
In AUTO_FLUSH_BACKGROUND mode,
the KuduSession::Apply() method blocks if total amount of data
for pending operations reaches the buffer size limit.  The limit
on the buffer size can be set by the
KuduSession::SetMutationBufferSpace() method.

The background flush logic checks whether at least one of the
following two conditions is satisfied to determine whether it's time
to flush the accumulated write operations:
  * The over-the-watermark criterion: check whether the total size of
the freshly submitted (i.e. not-yet-scheduled-for-flush) write
operations is over the threshold.  The threshold can be set as
the percentage of the total buffer size using the
KuduSession::SetMutationBufferFlushWatermark() method.
  * The maximum wait time criterion: check whether the current batch
of operations has been accumulating for more than the maximum
wait time.  The maximum wait time can be specified in milliseconds
using the KuduSession::SetMutationBufferFlushInterval() method.
A KuduSession object uses RPC messenger's thread pool to monitor
batches' maximum wait time.

Added functionality to control the maximum number of batchers
per session.  If number of batchers is at the limit already,
KuduSession::Apply() blocks until it's possible to add a new batcher
to accommodate the incoming operation.

Modified behavior of the KuduSession::Flush(): now it waits until all
batchers are flushed before returning.

This change also addresses the following JIRA issue:
  KUDU-1376 KuduSession::SetMutationBufferSpace is not defined

Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
---
M python/kudu/tests/test_client.py
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/error_collector.cc
M src/kudu/client/error_collector.h
M src/kudu/client/session-internal.cc
M src/kudu/client/session-internal.h
M src/kudu/client/write_op.cc
M src/kudu/client/write_op.h
13 files changed, 1,440 insertions(+), 209 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tool: split up action descriptions

2016-08-29 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: tool: split up action descriptions
..


tool: split up action descriptions

With ksck we have a use case for "short" and "long" action descriptions: the
former appears in mode help and action help, while the latter is only in
action help. This patch splits action descriptions into "short" and "extra",
refactoring all actions accordingly. While I was there I got rid of the
label abstraction, which I felt was producing too much unfluent code.

I also added some more tests to kudu-tool-test.

Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Reviewed-on: http://gerrit.cloudera.org:8080/4148
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_tablet.cc
M src/kudu/tools/tool_main.cc
8 files changed, 261 insertions(+), 97 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4100/6/python/kudu/tests/common.py
File python/kudu/tests/common.py:

PS6, Line 56: "--unlock_unsafe_flags",
: "--unlock_experimental_flags",
> Nit: use single dash for consistency with the other flags. Below too.
Done


http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flag_tags-test.cc
File src/kudu/util/flag_tags-test.cc:

Line 36: 
> Nit: there's an extra empty line here.
Done


Line 75: ASSERT_DEATH({ HandleCommonFlags();},
> Nit: { HandleCommonFlags(); }
Done


PS6, Line 82: StringVectorSink sink;
: ScopedRegisterSink reg(&sink);
> You know you've made it as a project when you've got unit tests that check 
\_o_/


http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:

Line 312: void CheckFlagsUnlocked() {
> Nit: perhaps adjust the function name slightly? When I saw FooUnlocked() I 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..


Patch Set 6:

Code change looks good, but how about a test in kudu-ts-cli-test?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: port cfile-dump to 'kudu fs dump_cfile'
..


Patch Set 2: Code-Review+2

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

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


[kudu-CR] tool: split up action descriptions

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: split up action descriptions
..


Patch Set 4:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] cfile: replace DumpIteratorOptions with number of rows

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: cfile: replace DumpIteratorOptions with number of rows
..


cfile: replace DumpIteratorOptions with number of rows

As of commit 9884fab, DumpIterator() doesn't print anything at all when
print_rows is false. That makes the option rather useless, so I'm replacing
it here with the raw number of rows.

Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063
Reviewed-on: http://gerrit.cloudera.org:8080/4150
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile-dump.cc
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/tools/fs_tool.cc
4 files changed, 27 insertions(+), 52 deletions(-)

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



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

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


[kudu-CR] cfile: replace DumpIteratorOptions with number of rows

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: cfile: replace DumpIteratorOptions with number of rows
..


Patch Set 2: Code-Review+2

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

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


[kudu-CR] tool: split up action descriptions

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: split up action descriptions
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-687: expose additional tablet metadata in C++ client
..


KUDU-687: expose additional tablet metadata in C++ client

This patch adds new data-only KuduReplica and KuduTablet classes.
Along with KuduTabletServer, the C++ client now has more parity with the
Java client w.r.t. exposing tablet metadata.

For now, this is only exposed via KuduScanToken, because it's already doing
the work to figure out which replicas exist where. That's an odd fit for
ksck (which doesn't scan, at least not using the client), but it should
stave off any controversy stemming from adding dubious public APIs. In the
future, it wouldn't be unreasonable for these classes to be exposed via
KuduTable in some way.

There are four public signature changes:
- Addition of KuduScanToken::tablet(): this is backwards compatible.
- Addition of KuduTabletServer::port(): this is backwards compatible.
- Change to the KuduScanToken constructor: this should be backwards
  compatible, because it's private and so shouldn't used by applications.
- Removal of KuduScanToken::TabletServers(): this is an incompatible change.
  I think it's OK because Impala is the only significant C++ client user and
  it's not even using scan tokens yet.

Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff
Reviewed-on: http://gerrit.cloudera.org:8080/4146
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M docs/release_notes.adoc
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
A src/kudu/client/replica-internal.cc
A src/kudu/client/replica-internal.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-internal.h
M src/kudu/client/scan_token-test.cc
A src/kudu/client/tablet-internal.cc
A src/kudu/client/tablet-internal.h
M src/kudu/client/tablet_server-internal.cc
M src/kudu/client/tablet_server-internal.h
15 files changed, 445 insertions(+), 95 deletions(-)

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



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

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


[kudu-CR] KUDU-687: use client in ksck for master operations

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-687: use client in ksck for master operations
..


Patch Set 4: Code-Review+2

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

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


[kudu-CR] KUDU-687: use client in ksck for master operations

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-687: use client in ksck for master operations
..


KUDU-687: use client in ksck for master operations

This patch modifies ksck to use the client for all master operations,
restricting direct access only for tserver operations. In doing so, ksck can
now work with multi-master clusters, retrying operations when the leader
master dies.

Interesting things of note:
- I went back and forth on what the semantics of KsckMaster::Connect() ought
  to be. At first I thought we should ping every master, but in the end I
  settled on building the client, which just verifies the existence of a
  leader master. My justification: today's ksck isn't really concerned with
  master health; the master merely provides information that is consumed
  during the real checks: of table and tablet integrity.
- I relented on commit cf009d4 and restored a MiniCluster method to find the
  leader master. It's got the same signature as the ExternalMiniCluster
  variant so that Mike's common cluster interface (a work in progress) can
  expose it.

Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3
Reviewed-on: http://gerrit.cloudera.org:8080/4147
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/client/schema.h
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/tool_action_cluster.cc
11 files changed, 163 insertions(+), 166 deletions(-)

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



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

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


[kudu-CR] transaction tracker: back-off when logging in-flight transactions

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: transaction_tracker: back-off when logging in-flight 
transactions
..


Patch Set 1:

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

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

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


[kudu-CR] transaction tracker: back-off when logging in-flight transactions

2016-08-29 Thread Todd Lipcon (Code Review)
Hello Mike Percy,

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

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

to review the following change.

Change subject: transaction_tracker: back-off when logging in-flight 
transactions
..

transaction_tracker: back-off when logging in-flight transactions

On a test cluster pushed into overload, I ended up with thousands of
in-flight transactions which were only being drained at the rate of one
or two per second. Dumping the entire list of transactions once a second
ended up flooding the glog with many MB per second work of transaction
strings.

This changes the dumping to back-off exponentially.

Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de
---
M src/kudu/tablet/transactions/transaction_tracker.cc
1 file changed, 16 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-687: use client in ksck for master operations

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

Change subject: KUDU-687: use client in ksck for master operations
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

Line 284:   for (auto s : servers) {
> ah. I think 'const auto*' is nice, then. Makes it clearer it's a pointer.
OK, added * where appropriate.


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

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


[kudu-CR] KUDU-687: use client in ksck for master operations

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-687: use client in ksck for master operations
..


Patch Set 4:

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

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

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


[kudu-CR] KUDU-687: use client in ksck for master operations

2016-08-29 Thread Adar Dembo (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-687: use client in ksck for master operations
..

KUDU-687: use client in ksck for master operations

This patch modifies ksck to use the client for all master operations,
restricting direct access only for tserver operations. In doing so, ksck can
now work with multi-master clusters, retrying operations when the leader
master dies.

Interesting things of note:
- I went back and forth on what the semantics of KsckMaster::Connect() ought
  to be. At first I thought we should ping every master, but in the end I
  settled on building the client, which just verifies the existence of a
  leader master. My justification: today's ksck isn't really concerned with
  master health; the master merely provides information that is consumed
  during the real checks: of table and tablet integrity.
- I relented on commit cf009d4 and restored a MiniCluster method to find the
  leader master. It's got the same signature as the ExternalMiniCluster
  variant so that Mike's common cluster interface (a work in progress) can
  expose it.

Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3
---
M src/kudu/client/schema.h
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/tool_action_cluster.cc
11 files changed, 163 insertions(+), 166 deletions(-)


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

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


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Dinesh Bhat (Code Review)
Hello Dan Burkert, Todd Lipcon, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..

Fix kudu-ts-cli crash when there is no data in tablet

NULL pointer was being sent in as an argument to
RowwiseRowBlockPB::Swap routine, fixing the callsite here.

./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 
--server_address=127.108.26.1:33958
previously segfaulted to:

$0  0x009c9867 in std::swap (__a=@0x7ffc75c76af8, __b=@0x18) at 
/opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176
$1  0x00b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, 
other=0x0) at 
/data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916
$2  0x0085de72 in kudu::client::KuduScanBatch::Data::Reset 
(this=0x7ffc75c76a80, controller=0x7ffc75c76a20, projection=0x7ffc75c76b40, 
client_projection=0x7ffc75c76ca0, data=...) at 
/data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484
$3  0x0084d5d9 in kudu::tools::TsAdminClient::DumpTablet 
(this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd") at 
/data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278
$4  0x0084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) 
at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455
$5  0x00850079 in main (argc=4, argv=0x7ffc75c773e8) at 
/data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492

Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
---
M src/kudu/tools/ts-cli.cc
1 file changed, 17 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Dinesh Bhat (Code Review)
Dinesh Bhat has posted comments on this change.

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4134/2/src/kudu/tools/ts-cli.cc
File src/kudu/tools/ts-cli.cc:

Line 278:   req.set_scanner_id(resp.scanner_id());
> yea, even without filters, you could have a bunch of deleted rows, and it's
Got it, thanks, also updated the scan to 'continue' instead of break. I will  
pump some small data into tablet to check this patch doesn't break the regular 
usage of dump_tablet.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Dinesh Bhat (Code Review)
Hello Dan Burkert, Todd Lipcon, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..

Fix kudu-ts-cli crash when there is no data in tablet

NULL pointer was being sent in as an argument to
RowwiseRowBlockPB::Swap routine, fixing the callsite here.

./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 
--server_address=127.108.26.1:33958
previously segfaulted to:

$0  0x009c9867 in std::swap (__a=@0x7ffc75c76af8, __b=@0x18) at 
/opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176
$1  0x00b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, 
other=0x0) at 
/data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916
$2  0x0085de72 in kudu::client::KuduScanBatch::Data::Reset 
(this=0x7ffc75c76a80, controller=0x7ffc75c76a20, projection=0x7ffc75c76b40, 
client_projection=0x7ffc75c76ca0, data=...) at 
/data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484
$3  0x0084d5d9 in kudu::tools::TsAdminClient::DumpTablet 
(this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd") at 
/data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278
$4  0x0084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) 
at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455
$5  0x00850079 in main (argc=4, argv=0x7ffc75c773e8) at 
/data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492

Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
---
M src/kudu/tools/ts-cli.cc
1 file changed, 8 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: split up action descriptions

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: split up action descriptions
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-687: use client in ksck for master operations

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-687: use client in ksck for master operations
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

Line 284:   for (const auto s : servers) {
> I'll add const, but why ref? All of these containers are of raw pointers.
ah. I think 'const auto*' is nice, then. Makes it clearer it's a pointer.


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

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


[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client

2016-08-29 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-687: expose additional tablet metadata in C++ client
..


Patch Set 2: Code-Review+2

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

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


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-29 Thread Dinesh Bhat (Code Review)
Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

PS15, Line 313: } else {
  :   DCHECK_EQ(table_id_, superblock.table_id());
  :   PartitionSchemaPB partition_schema_pb;
  :   partition_schema_.ToPB(&partition_schema_pb);
  :   
DCHECK_EQ(superblock.partition_schema().SerializeAsString(),
  : partition_schema_pb.SerializeAsString());
  :   PartitionPB partition_pb;
  :   partition_.ToPB(&partition_pb);
  :   DCHECK_EQ(superblock.partition().SerializeAs
> Well, it looks like we already have PartitionSchema::Equals() so maybe we j
I liked this idea, but it still doesn't prevent compat break if the wire format 
differs from the local copy if the destination version is ahead of source 
during tablet-copy. In general, it seems this is a question of : if the node is 
accepting a remote copy, is it a good idea to accept a copy from a server with 
release (N-1) ?


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

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


[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


Patch Set 3:

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

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

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


[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

2016-08-29 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4104/2/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:

Line 103: if (ts->TimeSinceHeartbeat().ToMilliseconds() < 
FLAGS_tserver_unresponsive_timeout_ms) {
> can you use PresumedDead here now? then you shouldn't need the DECLARE in t
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

2016-08-29 Thread Will Berkeley (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..

KUDU-1048 master should show versions of tservers, version summary

This patch adds a version summary table and a total count of
registered tablet servers to /tablet-servers.

It also fixes the display of the registration, which was printing in
red font.

Sample: 
https://raw.githubusercontent.com/wdberkeley/kudu-cr/master/KUDU-1048-improved.png

Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_descriptor.h
M src/kudu/master/ts_manager.cc
4 files changed, 46 insertions(+), 14 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col

2016-08-29 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col

2016-08-29 Thread Ram Mettu (Code Review)
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col
..

KUDU-1581 Fix DataFrame read failure when table has Binary Col

For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects
to receive Array[Byte], so change is to return a copy of the byte array.
Modified testcase to add missing col types and verify the values read back.

Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
4 files changed, 57 insertions(+), 20 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col

2016-08-29 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return

2016-08-29 Thread Ram Mettu (Code Review)
Ram Mettu has posted comments on this change.

Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col 
For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects 
to receive Array[Byte], so change is to return a copy of the byte array. 
Modified testcase to add missing col ty
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4145/2//COMMIT_MSG
Commit Message:

PS2, Line 8: For Binary 
> Nit: empty line here to conform to usual style
Done


http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:

PS2, Line 57: 
: 
> Nit: extra line break introduced here-- remove.
Actually line break was removed in my change, it was reverted back to the 
original


http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
File 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala:

PS2, Line 54: "c9_Timestamp"
> Nit: c9_timestamp -- just to have consistent case
Done


PS2, Line 55: "c10_Byte"
> Ditto
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-29 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

PS15, Line 313:   DCHECK_EQ(table_id_, superblock.table_id());
  :   PartitionSchemaPB partition_schema_pb;
  :   partition_schema_.ToPB(&partition_schema_pb);
  :   
DCHECK_EQ(superblock.partition_schema().SerializeAsString(),
  : partition_schema_pb.SerializeAsString());
  :   PartitionPB partition_pb;
  :   partition_.ToPB(&partition_pb);
  :   DCHECK_EQ(superblock.partition().SerializeAsString(),
  : partition_pb.SerializeAsString());
> I understand that this code isn't executed in release mode, but I feel fair
Well, it looks like we already have PartitionSchema::Equals() so maybe we just 
need to implement Partition::Equals() to do this without comparing protobuf 
encodings. We could have something like:

 PartitionNSchema s;
 RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(), &s));
 DCHECK(partition_schema_.Equals(s));

And something similar for Partition.

In general I'm mostly concerned with forward and backward compatibility in 
release mode, although also maintaining it for DEBUG builds clearly would be 
nice. I think writing automated upgrade / downgrade tests would also help 
detect problems before release.


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

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