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

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

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


Patch Set 2:

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

Also needed a little bit of cleanup so it would compile and pass the tests. New 
patch uploaded.

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

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

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

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

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


Patch Set 1:

> BTW, I tried this on a cluster with a bad table:
 > WARNING: Unable to connect to Tablet Server 5fb7d6c7083943059521e03d6ece2863
 > (10.20.132.112:7050) because Network error: Client connection
 > negotiation failed: client connection to 10.20.132.112:7050:
 > connect: Connection refused (error 111)
 > WARNING: Unable to connect to Tablet Server acd8306f95334ec1bfce8cb30d7ca36d
 > (10.20.126.115:7050) because Network error: Client connection
 > negotiation failed: client connection to 10.20.126.115:7050:
 > connect: Connection refused (error 111)
 > WARNING: Unable to connect to Tablet Server dff78a5acdbb4a47ba2c7a62d1bcc5ee
 > (10.20.132.107:7050) because Network error: Client connection
 > negotiation failed: client connection to 10.20.132.107:7050:
 > connect: Connection refused (error 111)
 > WARNING: Connected to 69 Tablet Servers, 3 weren't reachable
 > WARNING: Tablet 3bf432551c5d4c529616f8e7ce829424 of table
 > 'usertable' does not have a majority of replicas in RUNNING state
 > WARNING: Tablet 2f652871b74b4d0f9bf99e730486a451 of table
 > 'usertable' does not have a majority of replicas in RUNNING state
 > WARNING: Tablet b009973af71842cf99e10d25254b5557 of table
 > 'usertable' does not have a majority of replicas in RUNNING state
 > WARNING: Tablet 71ca44eebda44903868014175e02862a of table
 > 'usertable' does not have a majority of replicas in RUNNING state
 > WARNING: Table usertable has 4 bad tablets
 > INFO: Table IntegrationTestBigLinkedListHeads is HEALTHY
 > INFO: Table IntegrationTestBigLinkedList is HEALTHY
 > WARNING: 1 out of 3 tables are not in a healthy state
 > ==
 > Errors:
 > ==
 > Tablet server aliveness check error: Network error: Not all Tablet
 > Servers are reachable
 > Table consistency check error: Corruption: 1 tables are bad
 > 
 > FAILED
 > 
 > From this output you can see how it would be useful to give
 > slightly more info on why the bad tablets are bad. Let me know if
 > you'll have time to keep working on this - otherwise I might try to
 > take it from where you left off.

Go for it, Unfortunately I don't think I'll have much time over the next couple 
of weeks.

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

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

[kudu-CR] Make block manager-test work on systems without hole-punching

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Make block_manager-test work on systems without hole-punching
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3636/2/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 85: template 
> How about just add a RETURN_NOT_LOG_BLOCK_MANAGER() here at the top of SetU
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56af74ad6b973c90057680be719b257109924fca
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Make block manager-test work on systems without hole-punching

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Make block_manager-test work on systems without hole-punching
..

Make block_manager-test work on systems without hole-punching

Commit 47c023f5eba708a184666e53d4a3000177a32fbc changed
block_manager-test so that it used a macro to return from tests that
weren't using the log block manager. This macro also needs to be
called in the Setup() method for BlockManagerTest.

Change-Id: I56af74ad6b973c90057680be719b257109924fca
---
M src/kudu/fs/block_manager-test.cc
1 file changed, 7 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56af74ad6b973c90057680be719b257109924fca
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3640/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

Line 80:   string *val = new string(StringPrintf(fmt_str, i));
> This is leaking the string since it's never deallocated.  I think the fix h
Yeah my bad...that was pretty obvious. Thanks for the quick comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Fix encoding-test on OS X
..

Fix encoding-test on OS X

A recent change caused encoding-test to fail on OS X. This reverts
the part of the change that causes the test to fail (and nothing
else).

I'm not sure why it fails on OS X and not in, e.g., Jenkins builds.

Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
---
M src/kudu/cfile/encoding-test.cc
1 file changed, 5 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: Fix encoding-test on OS X
..

Fix encoding-test on OS X

A recent change caused encoding-test to fail on OS X. This reverts
the part of the change that causes the test to fail (and nothing
else).

I'm not sure why it fails on OS X and not in, e.g., Jenkins builds.

Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
---
M src/kudu/cfile/encoding-test.cc
1 file changed, 3 insertions(+), 2 deletions(-)


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

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

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

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

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

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

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

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

There's a flag to revert to the old behavior that only uses master
metadata.

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

I'm planning to add canaries and make more improvements to ksck in
follow-up patches.

Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/master/master.proto
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/kudu-ksck.cc
9 files changed, 191 insertions(+), 26 deletions(-)


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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3501/5/src/kudu/consensus/consensus_queue-test.cc
File src/kudu/consensus/consensus_queue-test.cc:

PS5, Line 822: as a followe
> this seems to conflict with the 'SetLeaderMode' call below?
See next comment.


Line 827:   queue_->SetNonLeaderMode();
> oh, I see, you change it to non-leader. perhaps just update the comment abo
Done. Is this a reasonable way to do this? I just frankenstein'd the test from 
other tests.


Line 851:   // Committed index should be the same.
> I'm not sure why this patch is changing the committed index metric to not a
This patch doesn't change how followers handle the committed index. It changes 
how followers update metrics. The situation before this patch (and after) is 
that followers don't update the committed index-- I tested this a little bit to 
try to confirm it was what was leading to the bad metrics. I'm checking it's 
not updated in the test because if the committed index ever starts to be 
updated I'd like this test to fail, so somebody will take a look at how the 
change to the committed index code affects how metrics should work (e.g. maybe 
the follower could have the same metrics as leader at that point).


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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..

KUDU-763 consensus queue metrics on followers are messed up

On non-leader tablet replicas, the majority_done_ops and
in_progress_ops metrics are wrong. The reason is that non-leaders do
not update their consensus queue's committed index
(queue_state_.committed_index). This patch hides these incorrect
numbers in the web ui, where they were previously exposed (I think
unintentionally, the code having printed the queue when it meant to
print the state), documents that the numbers are not meaningful
except in the leader case, and forces them to always be zero
whenever the tablet replica is not a leader.

Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
---
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.cc
4 files changed, 68 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..

KUDU-763 consensus queue metrics on followers are messed up

On non-leader tablet replicas, the majority_done_ops and
in_progress_ops metrics are wrong. The reason is that non-leaders do
not update their consensus queue's committed index
(queue_state_.committed_index). This patch hides these incorrect
numbers in the web ui, where they were previously exposed (I think
unintentionally, the code having printed the queue when it meant to
print the state), documents that the numbers are not meaningful
except in the leader case, and forces them to always be zero
whenever the tablet replica is not a leader.

Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
---
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.cc
4 files changed, 68 insertions(+), 10 deletions(-)


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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 3:

(2 comments)

> did you see my note about a test?

Whoops. I'll add one soon.

http://gerrit.cloudera.org:8080/#/c/3501/2/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

Line 77:   "not all peers. Not meaningful for 
non-leader tablet replicas.");
> I do think we should actually make the data at least be "0" in the case tha
Done


http://gerrit.cloudera.org:8080/#/c/3501/2/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

Line 251: peer.has_last_known_addr() ? peer.last_known_addr().host() : 
peer.permanent_uuid();
> would be good to move these web ui improvements to a separate patch
Done


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

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

[kudu-CR] Add port in web ui tables, add role and table name to /tablet page

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: Add port in web ui tables, add role and table name to /tablet 
page
..

Add port in web ui tables, add role and table name to /tablet page

Couple of improvements to web ui:
1. The RaftConfig column in the tablet table on master and tserver now
displays the port as well as the hostname for the tablet.
2. The /tablet page displays the tablet role and the table name.

Change-Id: Ia80b74346cae1a75d66b400521e07aa1994d1d65
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.cc
2 files changed, 11 insertions(+), 4 deletions(-)


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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..

KUDU-763 consensus queue metrics on followers are messed up

On non-leader tablet replicas, the majority_done_ops and in_progress_ops
metrics are wrong. The reason is that non-leaders do not update their
consensus queue's committed index (queue_state_.committed_index). This patch
hides these incorrect numbers in the web ui, where they were previously
exposed (I think unintentionally, the code having printed the queue when it
meant to print the state), documents that the numbers are not meaningful
except in the leader case, and forces them to always be zero whenever the
tablet replica is not a leader.

Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/raft_consensus.cc
2 files changed, 10 insertions(+), 10 deletions(-)


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

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

[kudu-CR] KUDU-1386 NaN float and double values are not handled correctly

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1386 NaN float and double values are not handled correctly
..

KUDU-1386 NaN float and double values are not handled correctly

Previously, TypeInfo::Compare always returned 0 for comparisons with NaN.
This meant that an equality predicate on a floating point column would
always return all NaN values for the column. This patch changes Compare
so that NaN compares > all non-NaN floating point values, and == to itself
(contrary to IEEE754 standard), based on the suggestion from Todd Lipcon
that we follow Postgres's example.

Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae
---
M src/kudu/client/predicate-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/types-test.cc
M src/kudu/common/types.h
4 files changed, 101 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Peter Ebert
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1398 CFile index blocks can store shortest separating 
prefix
..

KUDU-1398 CFile index blocks can store shortest separating prefix

(No changes: resubmitting to trigger Jenkins build)

This changes the values stored as index keys to be a shortest key
between the first key of the data block and the last key of the previous
data block. However, this change does not apply to deltafiles, because
deltafiles expect to be able to decode an index key into a DeltaKey.

The way the change works is illustrated with the example from the JIRA,
extended:

Block 1: apple, banana, cardamom
Block 2: carrot, epazote, fennel
Block 3: fig, guava, kiwi

Before: ['apple' -> block 1, 'carrot' -> block 2, 'fig' -> block 3]

After: ['a' -> block 1, 'carr' -> block 2, 'fi' -> block 3]

Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
---
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/binary_prefix_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/bloomfile.h
M src/kudu/cfile/bshuf_block.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/gvint_block.cc
M src/kudu/cfile/gvint_block.h
M src/kudu/cfile/index-test.cc
M src/kudu/cfile/index_block.cc
M src/kudu/cfile/plain_bitmap_block.h
M src/kudu/cfile/plain_block.h
M src/kudu/cfile/rle_block.h
M src/kudu/tablet/deltafile.cc
23 files changed, 234 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
Gerrit-PatchSet: 10
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-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1398 CFile index blocks can store shortest separating 
prefix
..

KUDU-1398 CFile index blocks can store shortest separating prefix

(No changes: resubmitting to trigger Jenkins build)

This changes the values stored as index keys to be a shortest key
between the first key of the data block and the last key of the previous
data block. However, this change does not apply to deltafiles, because
deltafiles expect to be able to decode an index key into a DeltaKey.

The way the change works is illustrated with the example from the JIRA,
extended:

Block 1: apple, banana, cardamom
Block 2: carrot, epazote, fennel
Block 3: fig, guava, kiwi

Before: ['apple' -> block 1, 'carrot' -> block 2, 'fig' -> block 3]

After: ['a' -> block 1, 'carr' -> block 2, 'fi' -> block 3]

Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
---
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/binary_prefix_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/bloomfile.h
M src/kudu/cfile/bshuf_block.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/gvint_block.cc
M src/kudu/cfile/gvint_block.h
M src/kudu/cfile/index-test.cc
M src/kudu/cfile/index_block.cc
M src/kudu/cfile/plain_bitmap_block.h
M src/kudu/cfile/plain_block.h
M src/kudu/cfile/rle_block.h
M src/kudu/tablet/deltafile.cc
23 files changed, 234 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
Gerrit-PatchSet: 9
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-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-1398 CFile index blocks can store shortest separating 
prefix
..


Patch Set 6:

> Sorry for the delay on this, Will. I was on PTO the last couple of
 > weeks. Hope to get back to review this week. I noticed the last
 > build failed but the build results have been purged by this point.
 > Is the current rev passing tests, etc?

No worries Mr Lipcon. I hope you enjoyed your vacation. IIRC it passes 
everything, but on the last build Jenkins choked somehow and failed the build 
through no fault on my own. I'll verify it passes the basic stuff locally and 
repush so Jenkins can (hopefully) verify.

Unrelatedly, I have a patch in the works for KUDU-1227 (update-merge). It works 
and passes the tests + the tests I wrote for it, but definitely needs some 
review love. Is there interest in that for 1.0? I mostly did it for personal 
enrichment but obv happy to contribute it if it's wanted.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
Gerrit-PatchSet: 6
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-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1398 CFile index blocks can store shortest separating 
prefix
..

KUDU-1398 CFile index blocks can store shortest separating prefix

This changes the values stored as index keys to be a shortest key
between the first key of the data block and the last key of the previous
data block. However, this change does not apply to deltafiles, because
deltafiles expect to be able to decode an index key into a DeltaKey.

The way the change works is illustrated with the example from the JIRA,
extended:

Block 1: apple, banana, cardamom
Block 2: carrot, epazote, fennel
Block 3: fig, guava, kiwi

Before: ['apple' -> block 1, 'carrot' -> block 2, 'fig' -> block 3]

After: ['a' -> block 1, 'carr' -> block 2, 'fi' -> block 3]

Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
---
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/binary_prefix_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/bshuf_block.h
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/gvint_block.cc
M src/kudu/cfile/gvint_block.h
M src/kudu/cfile/index-test.cc
M src/kudu/cfile/index_block.cc
M src/kudu/cfile/plain_bitmap_block.h
M src/kudu/cfile/plain_block.h
M src/kudu/cfile/rle_block.h
M src/kudu/tablet/deltafile.cc
20 files changed, 206 insertions(+), 51 deletions(-)


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

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

[kudu-CR] [WIP] KUDU-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [WIP] KUDU-1398 CFile index blocks can store shortest 
separating prefix
..

[WIP] KUDU-1398 CFile index blocks can store shortest separating prefix

This changes the values stored as index keys to be a shortest key
between the first key of the data block and the last key of the previous
data block. This change does not apply to deltafiles or bloomfiles.
Deltafiles expect to be able to decode an index key into a DeltaKey.
Bloom files don't work with this change for reasons I haven't tracked
down yet.

Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
---
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/binary_prefix_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/bshuf_block.h
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/gvint_block.cc
M src/kudu/cfile/gvint_block.h
M src/kudu/cfile/plain_bitmap_block.h
M src/kudu/cfile/plain_block.h
M src/kudu/cfile/rle_block.h
M src/kudu/tablet/deltafile.cc
19 files changed, 197 insertions(+), 45 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
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] [WIP] KUDU-1398 CFile index blocks can store shortest separating prefix

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [WIP] KUDU-1398 CFile index blocks can store shortest 
separating prefix
..

[WIP] KUDU-1398 CFile index blocks can store shortest separating prefix

This changes the values stored as index keys to be a shortest key
between the first key of the data block and the last key of the previous
data block. This change does not apply to deltafiles or bloomfiles.
Deltafiles expect to be able to decode an index key into a DeltaKey.
Bloom files don't work with this change for reasons I haven't tracked
down yet.

Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
---
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/binary_prefix_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/bshuf_block.h
M src/kudu/cfile/cfile_util.cc
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/gvint_block.cc
M src/kudu/cfile/gvint_block.h
M src/kudu/cfile/plain_bitmap_block.h
M src/kudu/cfile/plain_block.h
M src/kudu/cfile/rle_block.h
M src/kudu/tablet/deltafile.cc
18 files changed, 196 insertions(+), 45 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68ae9146fabd4a19b17d103d118d2d60e28bb315
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1446 Consider data types in predicate evaluation order

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1446 Consider data types in predicate evaluation order
..

KUDU-1446 Consider data types in predicate evaluation order

This patch enhances SelectivityRank() in column_predicate.cc to account for
column type. Predicate type dominates column type for the sort, and a predicate
on a column of a small type will sort before a predicate on a column of larger
(or variable-size) type.

It also adds a small test for selectivity comparison.

Change-Id: I9c3388876035cb494fdb2c0e1f17698f089fdf72
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
2 files changed, 49 insertions(+), 5 deletions(-)


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

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

[kudu-CR] KUDU-1446 Consider data types in predicate evaluation order

[Will Berkeley (Code Review)]

Will Berkeley has abandoned this change.

Change subject: KUDU-1446 Consider data types in predicate evaluation order
..


Abandoned

unintentional dupe

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic20cf9d574b2e5f80c7ab3b1a5c4569415dc1d4d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1446 Consider data types in predicate evaluation order

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-1446 Consider data types in predicate evaluation order
..

KUDU-1446 Consider data types in predicate evaluation order

This patch enhances SelectivityRank() in column_predicate.cc to account for
column type. Predicate type dominates column type for the sort, and a predicate
on a column of a small type will sort before a predicate on a column of larger
(or variable-size) type.

Change-Id: I9c3388876035cb494fdb2c0e1f17698f089fdf72
---
M src/kudu/common/column_predicate.cc
1 file changed, 31 insertions(+), 5 deletions(-)


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

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

[kudu-CR] Dependency on Hadoop test classes causes impertinent Javadoc warnings

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: Dependency on Hadoop test classes causes impertinent Javadoc 
warnings
..

Dependency on Hadoop test classes causes impertinent Javadoc warnings

Initial patch to see what Jenkins says since the tests don't run well
locally for me...will update and clean up again with another patch set
later.

Change-Id: I297f46d930f274666db9d503cf8897fe509470da
---
M java/kudu-mapreduce/pom.xml
A java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/JarFinder.java
M 
java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/KuduTableMapReduceUtil.java
3 files changed, 185 insertions(+), 4 deletions(-)


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

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

[kudu-CR] KUDU-1386 NaN float and double values are not handled correctly

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-1386 NaN float and double values are not handled correctly
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3142/2/src/kudu/common/types.h
File src/kudu/common/types.h:

Line 303: } else if (isnan(*lhs_n)) {
> given that nans are less common than anything else, I think it would be bes
Done.


Line 334: const double *lhs_n = reinterpret_cast(lhs);
> can you extract this code into a FloatCompare templated method?
Done.


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

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