date:20160928

Alexey Serbin has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 16:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4412/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 180: TEST_F(ToolTest, TestTopLevelHelp) {
> Can you also add a new section to TestModeHelp for the new action?
Done


PS16, Line 788: vector(
> Sorry, I meant why do you need a vector() wrapper?
Ah, sorry for the misunderstanding.  That's not needed: good catch!


http://gerrit.cloudera.org:8080/#/c/4412/17/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

PS17, Line 79: using std::unique_ptr;
 : using std::vector;
> Nit: back_inserter should go first.
Sounds like a pun :)

Fixed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] added insert-generated-rows into kudu tools
..

[tools] added insert-generated-rows into kudu tools

The insert-generated-rows tool has been merged into the 'kudu'
umbrella toolset.

This addresses KUDU-1628.

Besides, added ability to run multiple inserter threads and
specify additional parameters on batching behavior of the generated
write operations.  It's possible to run data generating sessions
both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes.  Introduced
sequential and random modes for the data generator.  Overall, these
changes allow to use the tool to measure performance of the Kudu C++
client library in simplistic 'push-as-much-as-you-can' scenario:
the client generates and sends data as fast as it can.

Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/insert-generated-rows.cc
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_local_replica.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
17 files changed, 714 insertions(+), 183 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Adar Dembo has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 17:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4412/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 180:   string tool_path_;
> Done
Can you also add a new section to TestModeHelp for the new action?


PS16, Line 788: ema(client::Kud
> Because otherwise it will be an error like
Sorry, I meant why do you need a vector() wrapper?


http://gerrit.cloudera.org:8080/#/c/4412/17/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

PS17, Line 79: using std::copy;
 : using std::back_inserter;
Nit: back_inserter should go first.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds

Adar Dembo has posted comments on this change.

Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds
..


Patch Set 2: Verified+1

Overriding Jenkins, flakiness in ITClient.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895
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: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

Alexey Serbin has posted comments on this change.

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Alexey Serbin has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 16:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4412/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 180: TEST_F(ToolTest, TestTopLevelHelp) {
> Can you update this test with the new mode and action?
Done


PS16, Line 781: client::sp::
> Nit: you added a "using" for this at the top of the file.
Done


PS16, Line 788: vector(
> Why is this needed?
Because otherwise it will be an error like

Bad status: Invalid argument: Table partitioning must be specified using 
add_hash_partitions or set_range_partition_columns


PS16, Line 789: wait(true)
> Can omit, this is the default behavior.
Done


PS16, Line 798: ASSERT_OK(cluster->Start());
> You can move this into LoadgenCreateMiniCluster if you want. You could even
Good idea.  For some reason, I thought keeping separate functions would be 
better, but apparently there isn't any demand for that.  Will update.


PS16, Line 819: string("-table_name=") + kTableName
> Nit: Substitute("-table_name=%s", kTableName) is more idiomatic.
Done


Line 820: "-buffer_flush_watermark_pct=0.125",
> Nit: can you use -- to prefix gflags, for consistency with existing tests?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] added insert-generated-rows into kudu tools
..

[tools] added insert-generated-rows into kudu tools

The insert-generated-rows tool has been merged into the 'kudu'
umbrella toolset.

This addresses KUDU-1628.

Besides, added ability to run multiple inserter threads and
specify additional parameters on batching behavior of the generated
write operations.  It's possible to run data generating sessions
both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes.  Introduced
sequential and random modes for the data generator.  Overall, these
changes allow to use the tool to measure performance of the Kudu C++
client library in simplistic 'push-as-much-as-you-can' scenario:
the client generates and sends data as fast as it can.

Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/insert-generated-rows.cc
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_local_replica.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
17 files changed, 708 insertions(+), 182 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] thirdparty: fix up libtool scripts if needed

Dan Burkert has posted comments on this change.

Change subject: thirdparty: fix up libtool scripts if needed
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1654 - [python] Python 3 Client Test Failure: test table column

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1654 - [python] Python 3 Client Test Failure: 
test_table_column
..

KUDU-1654 - [python] Python 3 Client Test Failure: test_table_column

Python 3 fails the test_table_column test because it is is attempting
to cast a string to bytes without an encoding argument. In Python 3,
this is a required parameter as bytes and strings are no longer
synonymous. This patch utilizes the kudu.compat.frombytes method
which was written for this purpose.

Change-Id: I7184639426b7f2528523209152dafd5fa39fecf9
---
M python/kudu/client.pyx
M python/kudu/tests/test_client.py
2 files changed, 7 insertions(+), 6 deletions(-)


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

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

[kudu-CR] KUDU-1654 - [python] Python 3 Client Test Failure: test table column

Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1654 - [python] Python 3 Client Test Failure: 
test_table_column
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4543/1/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:

Line 45: assert col.name == name
> given that column names should typically be human-readable (and are Strings
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7184639426b7f2528523209152dafd5fa39fecf9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1654 - [python] Python 3 Client Test Failure: test table column

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1654 - [python] Python 3 Client Test Failure: 
test_table_column
..

KUDU-1654 - [python] Python 3 Client Test Failure: test_table_column

Python 3 fails the test_table_column test because it is is attempting
to cast a string to bytes without an encoding argument. In Python 3,
this is a required parameter as bytes and strings are no longer
synonymous. This patch utilizes the kudu.compat.frombytes method
which was written for this purpose.

Change-Id: I7184639426b7f2528523209152dafd5fa39fecf9
---
M python/kudu/client.pyx
M python/kudu/tests/test_client.py
2 files changed, 8 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7184639426b7f2528523209152dafd5fa39fecf9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Adar Dembo has posted comments on this change.

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..


Patch Set 4: Code-Review+2

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

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

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Jean-Daniel Cryans has posted comments on this change.

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4560/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 50: emphasis
> You missed "emphasize" for "emphasis".
oops


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Hello Kudu Jenkins,

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

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

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

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..

[docs] Release note for OperationResponse.getWriteTimestamp

See https://gerrit.cloudera.org/#/c/4487/

Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
---
M docs/release_notes.adoc
1 file changed, 3 insertions(+), 1 deletion(-)


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

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

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Adar Dembo has posted comments on this change.

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4560/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 50: emphasis
> Done
You missed "emphasize" for "emphasis".


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

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

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Jean-Daniel Cryans has posted comments on this change.

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4560/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 49: renamed
> Nit: renamed to
Done


PS1, Line 50: emphasis
> Nit: emphasize that it doesn't return milliseconds.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Hello Kudu Jenkins,

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

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

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

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..

[docs] Release note for OperationResponse.getWriteTimestamp

See https://gerrit.cloudera.org/#/c/4487/

Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
---
M docs/release_notes.adoc
1 file changed, 3 insertions(+), 1 deletion(-)


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

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

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Adar Dembo has posted comments on this change.

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4560/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 49: renamed
Nit: renamed to


PS1, Line 50: emphasis
Nit: emphasize that it doesn't return milliseconds.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [WIP]KUDU-1640 - [python] Add IN-list predicate support

Hello Kudu Jenkins,

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

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

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

Change subject: [WIP]KUDU-1640 - [python] Add IN-list predicate support
..

[WIP]KUDU-1640 - [python] Add IN-list predicate support

This patch adds IN list predicate support for the python client.
This patch includes a test.

Change-Id: I932dfded62e162cf85e0e12432cf6716311957de
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/test_scanner.py
3 files changed, 27 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I932dfded62e162cf85e0e12432cf6716311957de
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp

Jean-Daniel Cryans has uploaded a new change for review.

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

Change subject: [docs] Release note for OperationResponse.getWriteTimestamp
..

[docs] Release note for OperationResponse.getWriteTimestamp

See https://gerrit.cloudera.org/#/c/4487/

Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
---
M docs/release_notes.adoc
1 file changed, 3 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans

[kudu-CR] KUDU-1612 - [python] Enable setting of read mode for scanning

Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1612 - [python] Enable setting of read mode for scanning
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4520/2/python/kudu/tests/util.py
File python/kudu/tests/util.py:

Line 122: self.snapshot_timestamp = 
self.client.latest_observed_timestamp()
> because the server's clock might not have the same time as the system's clo
I follow, done. I took an approach that converts hybridtime to datetimes (via 
micros) to keep it a bit cleaner on the interface side if someone wanted to do 
anything with the latest observed timestamp value.


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

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

[kudu-CR] KUDU-1612 - [python] Enable setting of read mode for scanning

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1612 - [python] Enable setting of read mode for scanning
..

KUDU-1612 - [python] Enable setting of read mode for scanning

Currently the python client is unable to set the read mode for
scanning, so all scans are done as READ_LATEST.  This patch enables
the ability to set the read mode so that the python client can read
at snapshots. This patch includes multiple tests.

Change-Id: I2c61ef09f6e15bad2c44d9caf85b2cc2582b8a49
---
M python/kudu/__init__.py
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/util.py
M python/kudu/util.py
7 files changed, 275 insertions(+), 7 deletions(-)


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

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

[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds

Dan Burkert has posted comments on this change.

Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895
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: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] c++ client: stop requiring the old gcc ABI

Dan Burkert has posted comments on this change.

Change subject: c++ client: stop requiring the old gcc ABI
..


Patch Set 3: Code-Review+2

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

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

[kudu-CR] thirdparty: fix up libtool scripts if needed

Dan Burkert has posted comments on this change.

Change subject: thirdparty: fix up libtool scripts if needed
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

Alexey Serbin has posted comments on this change.

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..


Patch Set 6:

(1 comment)

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

PS6, Line 191: this
> This lambda function is accessing ConsensusStatePBToHtml method. Not the ri
Ah, I see -- I didn't spot this during the review.  That's OK.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: reorganize tree

Dan Burkert has posted comments on this change.

Change subject: thirdparty: reorganize tree
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383
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: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1363: Add IN-list predicate type

Dan Burkert has submitted this change and it was merged.

Change subject: KUDU-1363: Add IN-list predicate type
..


KUDU-1363: Add IN-list predicate type

Adds support in the C++ client for providing a set of equalities for a
given column. Support for using IN list predicates from the Java client
will be in a follow-up commit.

Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Reviewed-on: http://gerrit.cloudera.org:8080/2986
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_predicate-internal.h
M src/kudu/client/scan_predicate.cc
M src/kudu/client/scan_predicate.h
M src/kudu/client/value.h
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/common.proto
M src/kudu/common/key_util.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
16 files changed, 1,000 insertions(+), 37 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] thirdparty: build C dependencies with TSAN instrumentation too

Hello Dan Burkert, Todd Lipcon,

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

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

to review the following change.

Change subject: thirdparty: build C dependencies with TSAN instrumentation too
..

thirdparty: build C dependencies with TSAN instrumentation too

Our C library dependencies are pretty small so this isn't a huge hit, and
it makes the sanitizer-based dependency stack much more consistent. Now,
'common' is only for tools and header-only dependencies, while
uninstrumented/tsan/... include every library.

With additional sanitization come new suppressions. I didn't spend a whole
lot of time trying to figure out which were legitimate.

Change-Id: Ie8274ff7bc57016733cdd3c26858483f74e68faf
---
M build-support/tsan-suppressions.txt
M cmake_modules/FindPmem.cmake
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
4 files changed, 144 insertions(+), 121 deletions(-)


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

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

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Adar Dembo has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 16:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4412/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 180: TEST_F(ToolTest, TestTopLevelHelp) {
Can you update this test with the new mode and action?


PS16, Line 781: client::sp::
Nit: you added a "using" for this at the top of the file.


PS16, Line 788: vector(
Why is this needed?


PS16, Line 789: wait(true)
Can omit, this is the default behavior.


PS16, Line 798: ASSERT_OK(cluster->Start());
You can move this into LoadgenCreateMiniCluster if you want. You could even 
move LoadgenCreateTable() (enabled via an argument) and Subprocess::Call(), 
then call it RunLoadgenTest() or something. Basically make the TEST_F() 
functions themselves just thin wrappers that provide some additional arguments.


PS16, Line 819: string("-table_name=") + kTableName
Nit: Substitute("-table_name=%s", kTableName) is more idiomatic.


Line 820: "-buffer_flush_watermark_pct=0.125",
Nit: can you use -- to prefix gflags, for consistency with existing tests?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] added insert-generated-rows into kudu tools
..

[tools] added insert-generated-rows into kudu tools

The insert-generated-rows tool has been merged into the 'kudu'
umbrella toolset.

This addresses KUDU-1628.

Besides, added ability to run multiple inserter threads and
specify additional parameters on batching behavior of the generated
write operations.  It's possible to run data generating sessions
both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes.  Introduced
sequential and random modes for the data generator.  Overall, these
changes allow to use the tool to measure performance of the Kudu C++
client library in simplistic 'push-as-much-as-you-can' scenario:
the client generates and sends data as fast as it can.

Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/insert-generated-rows.cc
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_local_replica.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
17 files changed, 720 insertions(+), 179 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] thirdparty: split into dependency groups

Hello Kudu Jenkins,

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

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

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

Change subject: thirdparty: split into dependency groups
..

thirdparty: split into dependency groups

The monolithic thirdparty build is now quite a bit larger than it used to be
on account of the extra LLVM build. Let's see if we can't speed it up. The
idea is simple: carve it up into disjoint sections so that individual
sections can be rebuilt as needed.

This patch separates the various portions of the thirdparty build into
"dependency groups". Conceptually, a dependency group is a set of
dependencies built a certain way, but the implementation is really just a
set of non-overlapping code fragments in build-thirdparty.sh.

The initial set of groups are:
- common: dependencies that are never instrumented.
- uninstrumented: dependencies that may be instrumented, but aren't in this
  build.
- tsan: dependencies that may be instrumented, and are indeed in this build
  (with -fsanitize=thread).

These three generally map to the existing "common", "uninstrumented", and
"tsan" thirdparty subdirectories. There's an obvious pattern here for future
sanitizer builds (e.g. MSAN would provide an "msan" dependency group).

The new build-if-necessary.sh can accept an argument that maps to a set of
dependency groups representing a particular build. Every dependency group
has its own hash/stamp file so that it is only rebuilt when needed.

This also fixes a bug in the stamp file approach that prevented it from
actually rebuilding anything.

Change-Id: I549262858f98b5ce6c78e786e8c8d8134ba2ed36
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
M thirdparty/.gitignore
M thirdparty/build-if-necessary.sh
M thirdparty/build-thirdparty.sh
5 files changed, 271 insertions(+), 202 deletions(-)


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

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

[kudu-CR] thirdparty: fix up libtool scripts if needed

Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: thirdparty: fix up libtool scripts if needed
..

thirdparty: fix up libtool scripts if needed

Older versions of libtool (such as the version found on el6) do not pass
-stdlib=libc++ found in CXXFLAGS down to the libtool link command line, and
as a result, the shared object is linked against the system libstdc++. The
net effect is that some thirdparty shared objects sprout @@GLIBCXX
dependencies. Mysteriously, Kudu libraries and binaries themselves do not,
and the @@GLIBCXX dependencies appear to be satisfied via libc++ somehow.

Nevertheless, this is confusing and could prove problematic down the road,
so here's a quick hack to "fix up" libtool scripts after they are generated
via configure.

Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
---
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
A thirdparty/postflight.py
3 files changed, 131 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon

[kudu-CR] KUDU-1363: Add IN-list predicate type

Alexey Serbin has posted comments on this change.

Change subject: KUDU-1363: Add IN-list predicate type
..


Patch Set 19: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds

Adar Dembo has posted comments on this change.

Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4511/2/CMakeLists.txt
File CMakeLists.txt:

Line 106: COMMAND ${THIRDPARTY_DIR}/build-if-necessary.sh
> shouldn't this only be building the uninstrumented group by default, and th
You're looking at the patches out-of-order; the dependency group patch comes 
_after_ this one. Sorry for not making that clear.


http://gerrit.cloudera.org:8080/#/c/4511/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 345:   # Enable debug symbols so that stacktraces and linenumbers are 
available at
> Are the platform standard libraries built with debug symbols?  If so we may
libstdc++ on my machine doesn't have debug symbols in it, no.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds

Dan Burkert has posted comments on this change.

Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4511/2/CMakeLists.txt
File CMakeLists.txt:

Line 106: COMMAND ${THIRDPARTY_DIR}/build-if-necessary.sh
shouldn't this only be building the uninstrumented group by default, and the 
tsan group if KUDU_USER_TSAN is set?


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

Line 82:   local BUILD_TYPE=$1
(mindblown)


http://gerrit.cloudera.org:8080/#/c/4511/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 345:   # Enable debug symbols so that stacktraces and linenumbers are 
available at
Are the platform standard libraries built with debug symbols?  If so we may 
want to include them in the libc++ build.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: fix up libtool scripts if needed

Adar Dembo has posted comments on this change.

Change subject: thirdparty: fix up libtool scripts if needed
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4512/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 383: $TP_DIR/postflight.py
> This should be in the above if block, I think.  It will certainly always fa
Well, the goal was to be generic enough to run all the time. I think we can 
achieve that if the TSAN dependency check is reordered so that the check for 
the installed/tsan/lib precedes the check for ldd. I'll do that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: fix up libtool scripts if needed

Dan Burkert has posted comments on this change.

Change subject: thirdparty: fix up libtool scripts if needed
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4512/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 383: $TP_DIR/postflight.py
This should be in the above if block, I think.  It will certainly always fail 
on OS X, since there is no ldd or tsan.  I think it may also fail if you do a 
"./build-thirdparty.sh uninstrumented" on a clean checkout.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: split into dependency groups

Adar Dembo has posted comments on this change.

Change subject: thirdparty: split into dependency groups
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4513/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 141: echo TSAN does not work on OS X
> This triggers with a plain "./build-thirdparty.sh" invocation
Oops, will fix.


Line 395: echo "Thirdparty dependencies '$*' built and installed successfully"
> This isn't great with no arguments.
Done


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

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

[kudu-CR] thirdparty: split into dependency groups

Dan Burkert has posted comments on this change.

Change subject: thirdparty: split into dependency groups
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4513/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

Line 141: echo TSAN does not work on OS X
This triggers with a plain "./build-thirdparty.sh" invocation


Line 395: echo "Thirdparty dependencies '$*' built and installed successfully"
This isn't great with no arguments.


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

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

[kudu-CR] thirdparty: stifle unused argument warnings when building with clang

Dan Burkert has posted comments on this change.

Change subject: thirdparty: stifle unused argument warnings when building with 
clang
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

2016-09-28 Thread Ninad Shringarpure (Code Review)

Ninad Shringarpure has posted comments on this change.

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..


Patch Set 6:

(6 comments)

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

Line 219:   // TODO: would be nice to include some other stuff like memory 
usage
> Nit: please fix Tidy Bot's warning as well.  You can take a look at the nex
Fixed


PS6, Line 238:  
> Nit: an extra space; clang compiler is smart enough not to take it as '>>' 
Fixed


PS6, Line 239:  
> ditto
Fixed


Line 249: generate_table("Live Tablets",live_peers, output);
> Nit: a space is missing.
Fixed


Line 252: *output << "Tombstoned tablets are tablets that 
previously "
> Why not to put that after the table?  If this precedes the table, then this
Fixed


Line 254: generate_table("Tombstoned Tablets",tombstoned_peers, output);
> Nit: a space is missing.
Fixed


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

2016-09-28 Thread Ninad Shringarpure (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..

[web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a 
separate section

- Separated tombstoned and live tablet tables in /tablets ui page.
- Added one sentence explaining tombstoned tablets as:
   "Tombstoned tablets are tablets that previously stored a replica on this 
server."
- Tombstoned tablets table is displayed only when they are found.

Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
---
M src/kudu/tserver/tserver-path-handlers.cc
1 file changed, 64 insertions(+), 40 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] c++ client: stop requiring the old gcc ABI

Dan Burkert has posted comments on this change.

Change subject: c++ client: stop requiring the old gcc ABI
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] thirdparty: split into dependency groups

Hello Kudu Jenkins,

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

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

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

Change subject: thirdparty: split into dependency groups
..

thirdparty: split into dependency groups

The monolithic thirdparty build is now quite a bit larger than it used to be
on account of the extra LLVM build. Let's see if we can't speed it up. The
idea is simple: carve it up into disjoint sections so that individual
sections can be rebuilt as needed.

This patch separates the various portions of the thirdparty build into
"dependency groups". Conceptually, a dependency group is a set of
dependencies built a certain way, but the implementation is really just a
set of non-overlapping code fragments in build-thirdparty.sh.

The initial set of groups are:
- common: dependencies that are never instrumented.
- uninstrumented: dependencies that may be instrumented, but aren't in this
  build.
- tsan: dependencies that may be instrumented, and are indeed in this build
  (with -fsanitize=thread).

These three generally map to the existing "common", "uninstrumented", and
"tsan" thirdparty subdirectories. There's an obvious pattern here for future
sanitizer builds (e.g. MSAN would provide an "msan" dependency group).

The new build-if-necessary.sh can accept an argument that maps to a set of
dependency groups representing a particular build. Every dependency group
has its own hash/stamp file so that it is only rebuilt when needed.

This also fixes a bug in the stamp file approach that prevented it from
actually rebuilding anything.

Change-Id: I549262858f98b5ce6c78e786e8c8d8134ba2ed36
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
M thirdparty/.gitignore
M thirdparty/build-if-necessary.sh
M thirdparty/build-thirdparty.sh
5 files changed, 260 insertions(+), 202 deletions(-)


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

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

[kudu-CR] c++ client: stop requiring the old gcc ABI

Hello Kudu Jenkins,

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

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

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

Change subject: c++ client: stop requiring the old gcc ABI
..

c++ client: stop requiring the old gcc ABI

With the upgrade to clang 3.9 and the transition to libc++ for TSAN, we can
now safely remove the old gcc ABI guard rail without breaking TSAN builds.

The impact on backwards compatibility is immaterial. At least one Kudu
vendor has shipped a client package for Ubuntu 16.04 built against the old
ABI; that package will be built against the new ABI going forward. Any
application built against the old ABI will be incompatible with said
package once the ABI changes. But, c++ client consumers are few and far
between, and the major consumer of record (Apache Impala) does not yet
build on Ubuntu 16.04.

Change-Id: Ia06edb8a4699fc842a2072e1edab5cfaa14c4381
---
M .ycm_extra_conf.py
M CMakeLists.txt
M docs/release_notes.adoc
M python/setup.py
M src/kudu/client/client.h
M src/kudu/client/client_samples-test.sh
M src/kudu/client/samples/CMakeLists.txt
M thirdparty/build-thirdparty.sh
8 files changed, 9 insertions(+), 51 deletions(-)


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

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

[kudu-CR] thirdparty: stifle unused argument warnings when building with clang

Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: thirdparty: stifle unused argument warnings when building with 
clang
..

thirdparty: stifle unused argument warnings when building with clang

Dan tested my various thirdparty patches on macOS and reported that clang
emits thousands of unused argument warnings when building llvm, gflags, and
gtest. The first is due to the use of EXTRA_LDFLAGS in the llvm build; the
others have always been there.

We can tackle this in one of two ways:
1. Add -Qunused-arguments to EXTRA_CXXFLAGS. This isn't as easy as it
   sounds, because we need to restrict this to clang-based builds (gcc
   doesn't recognize the parameter), and sometimes that happens via CC/CXX
   environment variables and other times implicitly.
2. Narrow our prolific additions of -L... and -Wl,-rpath,... such that
   they're only added when needed.

I chose approach #2, which also meant remembering all of our
interdependencies. As best I can tell, here is the complete list:
- glog depends on gflags
- glog depends on libunwind
- pmemobj depends on pmem
- bitshuffle depends on lz4

With such a short list, approach #2 isn't actually that bad. I tested it by
building thirdparty with my system's clang. It worked once I disabled
-Werror in the nvml build.

I tried very hard not to regress commit 2567ed0. My system should be using
gold, though I noticed that only the shared objects in
installed/tsan had RUNPATH entries; the ones in installed/uninstrumented and
installed/common used RPATH. Nevertheless, I ran both debug and tsan tests,
which should have exercised all of them.

Change-Id: I41db7fa29c6823c2914881e3fe91844b0c315779
---
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
2 files changed, 36 insertions(+), 22 deletions(-)


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

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

[kudu-CR] thirdparty: fix up libtool scripts if needed

Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: thirdparty: fix up libtool scripts if needed
..

thirdparty: fix up libtool scripts if needed

Older versions of libtool (such as the version found on el6) do not pass
-stdlib=libc++ found in CXXFLAGS down to the libtool link command line, and
as a result, the shared object is linked against the system libstdc++. The
net effect is that some thirdparty shared objects sprout @@GLIBCXX
dependencies. Mysteriously, Kudu libraries and binaries themselves do not,
and the @@GLIBCXX dependencies appear to be satisfied via libc++ somehow.

Nevertheless, this is confusing and could prove problematic down the road,
so here's a quick hack to "fix up" libtool scripts after they are generated
via configure.

Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
---
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
A thirdparty/postflight.py
3 files changed, 131 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce
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: Todd Lipcon

[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds

Hello Kudu Jenkins,

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

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

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

Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds
..

thirdparty: use libc++ instead libstdc++ for TSAN builds

This all began because I wanted two things:
1. To use the new gcc 5 ABI on platforms that default to it (such as Ubuntu
   Xenial). Other applications compiled on these platforms will use the new
   ABI, and the fact that the Kudu client forces them to use the old ABI is
   quite unfriendly.
2. To have working local TSAN builds again, which broke following the gcc 5
   ABI transition in Xenial.

There are a number of interconnected issues at play:
A. Until 3.9, LLVM did not recognize gcc's new ABI tags, which prevented
   Kudu's codegen module from building properly against the new ABI.
B. For TSAN builds, we rebuild some thirdparty dependencies against the
   libstdc++ from thirdparty, but the LLVM libraries are not one of them.
   This may work when the system libstdc++ and the thirdparty libstdc++ are
   of the same version, but becomes increasingly untenable as the versions
   differ. Why? Because libstdc++ only guarantees forward compatibility;
   that is, a binary linked against libstdc++ can only be used with a
   libstdc++ of the same version or newer. Attempts to run with an older
   libstdc++ can lead to run time errors about missing GLIBCXX symbols. As
   a point of reference, on Xenial the two libraries are more than a major
   version apart.
C. Continuing B, libstdc++ from gcc 5 actually breaks backward compatibility
   for certain C++11-only symbols by moving them to an inline namespace
   (e.g. std::error_category is now std::_V2::error_category). The LLVM
   libraries use these symbols, which means LLVM built against a gcc 5
   libstdc++ cannot link against the older libstdc++ in thirdparty.
D. As the libstdc++ in thirdparty is from gcc 4, it is not multilib and does
   not provide new ABI symbols (e.g. std::__cxx11::string). Meaning, if the
   rest of Kudu tried to use the new ABI, TSAN builds would fail because the
   libstdc++ in use lacks new ABI symbols.

After upgrading LLVM, the path of least resistance was to upgrade libstdc++
in thirdparty, but what a saga that turned out to be. After much trial and
error, I gave up; I could not build libstdc++ from gcc 5 with clang, and we
must use clang to realize the latest -fsanitize=thread support.

Are there any alternatives? Well, we can follow Chromium's lead and use
libc++ for TSAN instead of libstdc++. I think this makes sense for several
reasons:
- The LLVM build, such as it is, is much more friendly than gcc's build.
  Building libstdc++ out of all of gcc was always a little hacky.
- There's at least one large open-source project (Chromium) that's
  successfully gone down this path.

That brings us to this patch, which is largely about replacing libstdc++
with libc++. Here are additional interesting details:
o We now build entire set of TSAN-duplicated dependencies with
  -fsanitize=thread, not just protobuf. It doesn't affect correctness much
  either way, but it's simpler and an easier concept to extend to future
  sanitizers that DO care (e.g. MSAN).
o We now build LLVM twice: once against the system libstdc++ for build tools
  and the regular LLVM libraries, and a second time against libc++ for
  instrumented LLVM libraries. The first build is a little hokey: it'd be
  more "pure" to build LLVM three times: once for build tools, once for LLVM
  libraries, and once for instrumented LLVM libraries. But these builds are
  super long so we optimize by combining the first two. The downside is that
  the first build now places build tools in 'installed-deps' instead of
  'installed'. I played around with placing build tools in 'installed' while
  placing the libraries in 'installed-deps', but found that to be too hacky.
o The full thirdparty build is now quite a bit longer on account of the
  second LLVM library build. I tried to mitigate this by reducing the number
  of extra cruft built each time. An upcoming patch will address this
  further by splitting thirdparty into separate modules.
o libc++ depends on libc++abi, so we build that first.
o The libc++ and libc++abi builds are done standalone rather than with the
  LLVM library build, because it isn't possible to do them together AND have
  the LLVM libraries depend on libc++.
o build_python may now be invoked more than once, so I've changed it to be
  idempotent within the same run of build-thirdparty.sh.

Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run-test.sh
M build-support/run_dist_test.py
M cmake_modules/FindPmem.cmake
M src/kudu/codegen/CMakeLists.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
D

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

2016-09-28 Thread Ninad Shringarpure (Code Review)

Ninad Shringarpure has posted comments on this change.

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..


Patch Set 6:

(1 comment)

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

PS6, Line 191: this
> Why is 'this' capture necessary for this functor?
This lambda function is accessing ConsensusStatePBToHtml method. Not the right 
way to do it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: reorganize tree

Hello Kudu Jenkins,

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

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

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

Change subject: thirdparty: reorganize tree
..

thirdparty: reorganize tree

This patch changes the organization of the thirdparty tree. The new layout
looks like this:
- installed: _all_ installed dependencies, with 'common',
 'uninstrumented', and 'tsan' subdirectories.
- build: build directories for all dependencies.
- src: source directories for all dependencies.

Additionally, the patch changes the build logic for each dependency so that
its build output is fully isolated from its source directory, and from other
build output (if the dependency is built multiple times).

Why do this?
1. Build isolation simplifies building dependencies multiple times (i.e. for
   different sanitizers) and makes it much safer.
2. It also means cleaning up build output doesn't mean redownloading all of
   the sources (i.e. no need to 'git clean -xdf thirdparty').
3. The grouping of all installed locations under the shared 'installed'
   subdirectory makes it a tad easier to blow it all away.
4. It also eases the transition for the remaining LLVM and thirdparty
   patches, as the conflicting build output will be copied to a different
   set of directories.
5. It slims down thirdparty/.gitignore; adding a new dependency no longer
   requires updating .gitignore.

Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383
---
M .ycm_extra_conf.py
M CMakeLists.txt
M README.adoc
M build-support/dist_test.py
M build-support/jenkins/build-and-test.sh
M build-support/lint.sh
M build-support/run_dist_test.py
M docs/installation.adoc
M docs/support/scripts/make_site.sh
M java/kudu-client/pom.xml
M src/kudu/client/client_samples-test.sh
M src/kudu/scripts/benchmarks.sh
M src/kudu/scripts/tpch.sh
M thirdparty/.gitignore
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 334 insertions(+), 217 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383
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: Todd Lipcon

[kudu-CR] thirdparty: reorganize tree

Adar Dembo has posted comments on this change.

Change subject: thirdparty: reorganize tree
..


Patch Set 1:

(3 comments)

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

Line 1: #!/bin/bash
> Is this necessary?
Every other shell script in the Kudu repo uses it, so I figure it's best to be 
consistent. Plus, I might be introducing bash-isms with these changes, so 
better safe than sorry.


Line 43: restore_env() {
> Should this now include MODE_SUFFIX?  I think that might be good so that we
Ahh, didn't realize PREFIX was also saved/restored. Yeah, I'll add MODE_SUFFIX 
too.


Line 439:   rsync -a $BOOST_SOURCE/boost $PREFIX/include
> Not your change, but this rsync incantation is using different flags than t
You referring to the lack of -v, or --delete?

The former is because boost tree is stupendously huge, so logging every file 
(-v) is kind of a drag.

But --delete should probably go in there, otherwise we may not notice header 
removals during upgrades. I'll add it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: clean up llvm cmake files too

Adar Dembo has submitted this change and it was merged.

Change subject: thirdparty: clean up llvm cmake files too
..


thirdparty: clean up llvm cmake files too

The upgrade to LLVM 3.9 changed the list of targets provided in LLVM's cmake
files. If the cmake files are left behind and make it into an LLVM 3.8-based
build, the subsequent Kudu build will fail because it can't find all of the
targets listed.

It's too late to help with the LLVM 3.9 upgrade, but hopefully it'll ease
future upgrades.

Change-Id: I98393cf1f8afc2ced78245cad5e2e24ee9410214
Reviewed-on: http://gerrit.cloudera.org:8080/4549
Reviewed-by: Dan Burkert 
Tested-by: Kudu Jenkins
---
M thirdparty/build-definitions.sh
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

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

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Adar Dembo has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/kudu-test.cc
File src/kudu/tools/kudu-test.cc:

Line 37: 
> The exact row count is verified by the utility itself if adding '-run_scan'
Ah, forgot about -run_scan. No, then this is fine.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Alexey Serbin has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 15:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:

PS15, Line 108: ADD_KUDU_TEST(kudu-test)
  : ADD_KUDU_TEST_DEPENDENCIES(kudu-test
  :   kudu)
> Why not reuse kudu-tool-test, which is where all of the new CLI tests (apar
Good idea, will do.  I was not sure where to put it; probably I should have 
asked for an advice.


http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/kudu-test.cc
File src/kudu/tools/kudu-test.cc:

Line 37: 
> These are nice, but can we at least verify that something was written with 
The exact row count is verified by the utility itself if adding '-run_scan' 
flag (this is so for all tests besides the very first one which runs with 
default parameters).

Do you mean we want to verify the exact data which has been written?  I.e., 
read it back and compare with the data which was inserted into the table?


http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

Line 110: #include "kudu/util/random.h"
> Nit: should precee stopwatch (surprised Tidy Bot didn't mention it?)
Good catch, will update.

So, it seems there is a hope -- not all our jobs are going to disappear due to 
automation and AI :)

Probably, there is a bug in Tidy Bot.  I'm just speculating here, but it might 
be related to the fact that this header comes last in the list (it might be 
off-by-one mistake or alike).


PS15, Line 152: options allows "
  : "to keep
> Nit: option retains
Done


PS15, Line 156: is not in effect
> Nit: has no effect
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Adar Dembo has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2:

> that being said, maybe you could just insert the block that causes
 > the test somewhere else.  How about FullStackInsertScanTest, would
 > it be easier to cause it there?

I too have a (weak) opinion that the test should be merged but I'll defer to 
Dan. That said, we use FullStackInsertScanTest in performance dashboards, so I 
don't think we should add a FsManager.Open() looping thread to it.

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

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

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2:

(1 comment)

that test is going to bit rot super quickly and this seems like a thorny enough 
problem that we'd want to run it frequently in case we break it somewhere else.
60 seconds isn't much when running tests in parallel.
that being said, maybe you could just insert the block that causes the test 
somewhere else.  How about FullStackInsertScanTest, would it be easier to cause 
it there?

http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 496: if (data_file_size < record.offset() + record.length()) {
PREDICT_FALSE to annotate that this is a rare race


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Adar Dembo has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 15:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:

PS15, Line 108: ADD_KUDU_TEST(kudu-test)
  : ADD_KUDU_TEST_DEPENDENCIES(kudu-test
  :   kudu)
Why not reuse kudu-tool-test, which is where all of the new CLI tests (apart 
from the ported ones in kudu-ts-cli-test and kudu-admin-test) live?


http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/kudu-test.cc
File src/kudu/tools/kudu-test.cc:

Line 37: 
These are nice, but can we at least verify that something was written with a 
scan? Can we verify an exact row count too?


http://gerrit.cloudera.org:8080/#/c/4412/15/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

Line 110: #include "kudu/util/random.h"
Nit: should precee stopwatch (surprised Tidy Bot didn't mention it?)


PS15, Line 152: options allows "
  : "to keep
Nit: option retains


PS15, Line 156: is not in effect
Nit: has no effect


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS2, Line 499: in order to account for the latest appends
Good find Dan, this is not a review but a curiosity Qn: Looking at the 
description/comments it seems like a racy situation between reader thread and a 
writer thread(I may have understood this wrong). Is there a guarantee here that 
the data_file_->Size is not changed after we grabbed the local_record.back() to 
inspect with CheckBlockRecord?


PS2, Line 504: local_records.back()
Nit: replace with record ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [web-ui] KUDU-1588 /tablets page should separate out tombstoned tablets to a separate section

Alexey Serbin has posted comments on this change.

Change subject: [web-ui] KUDU-1588 /tablets page should separate out tombstoned 
tablets to a separate section
..


Patch Set 6:

(7 comments)

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

PS6, Line 191: this
Why is 'this' capture necessary for this functor?


Line 219:   // TODO: would be nice to include some other stuff like memory 
usage
> warning: missing username/bug in TODO [google-readability-todo]
Nit: please fix Tidy Bot's warning as well.  You can take a look at the next 
line for the hint if you don't know who added this TODO.


PS6, Line 238:  
Nit: an extra space; clang compiler is smart enough not to take it as '>>' 
operator.


PS6, Line 239:  
ditto


Line 249: generate_table("Live Tablets",live_peers, output);
Nit: a space is missing.


Line 252: *output << "Tombstoned tablets are tablets that 
previously "
Why not to put that after the table?  If this precedes the table, then this 
paragraph is attributed to the 'Live Tablets':

Live Tablets
...
...

Tombstoned Tablets
...


Line 254: generate_table("Tombstoned Tablets",tombstoned_peers, output);
Nit: a space is missing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0aa9b67749234c5d68899a82d6b1493633bb78c5
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ninad Shringarpure 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] Implement a manual leader step down for a tablet

Adar Dembo has posted comments on this change.

Change subject: [tools] Implement a manual leader_step_down for a tablet
..


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

PS1, Line 204: }
> Sure, could you tell me what's the relationship between this being a main t
The CHECK_* macros come from glog, and they all do the same thing regardless of 
the context in which they're called:
1. Evaluate an expression.
2. If evaluation fails, log something.
3. Dump a stack trace.
4. Call abort() to exit the process.

The DCHECK_* macros (also from glog) are debug-only variants that are otherwise 
equivalent, but compiled out in non-debug builds.

The ASSERT_* macros come from gtest, so they're specific to how gtest works. 
They too evaluate an expression, but on failure, they do some gtest stuff to 
capture the failure, end the run of the current test, and move to the next 
test. They have some foibles, and one of them is that ASSERT failures in the 
thread that isn't the main process thread don't seem to get noticed  by gtest. 
Maybe there's something we need to do for that to happen; if there is, I don't 
know what it is.

Oh, and EXPECT_* is like ASSERT_* except that the test keeps running. So 
ASSERT_* will only ever reveal the "first" failure, while EXPECT_* will reveal 
all of them.

So to summarize:
- Use CHECK_*/DCHECK_* macros in production code. Use DCHECK_* if you expect to 
get sufficient test coverage of the callsite, CHECK_* otherwise.
- Use ASSERT_*/EXPECT_* macros in test-only code, unless that code is expected 
to run on a separate test thread, in which case use CHECK_* too. Default to 
EXPECT_*, unless the operation in question must succeed for the rest of the 
test to function properly, in which case use ASSERT_*.

Hope this helps.


Line 218: ASSERT_OK(Subprocess::Call({
> In reality yes, but given that we are operating in a deterministic test env
Unfortunately a test environment with leader failure detection isn't 
deterministic. The test can run on a machine with arbitrary load (perhaps from 
other tests), in which case one of the tserver processes can pause for 
arbitrary periods of time, causing a new leader to be elected. This is one of 
the leading causes of (low-grade) test flakiness for us.


PS1, Line 230: CHECK_OK(GetInfoFromConsensus(tservers, tablet_id_,
 :   _leader, _term));
> I tried this with an unsuccessful result, later realized AssertEventually()
Couple things:

1. I don't understand why AssertEventually() doesn't work here. AFAICT the goal 
of this snippet is "tell the leader to step down, wait for an election to 
happen, then assert that the new term is higher than the one before". If you 
wrap L229-L233 in AssertEventually(), doesn't that mean you don't need the 3 
second sleep (which could be flaky to begin with, since a cluster running with 
ASAN/TSAN could take a lot longer to run an election)? I don't see what the 
issue is; can you help me understand? Look at some of the existing usages of 
AssertEventually() if it's not clear.
2. I'm not seeing the bug in the code you pasted. What is it?


Line 264:   s = GetInfoFromConsensus(tservers, tablet_id_,
> Done
The actual check (not in the CHECK sense of the word, but the comparison and 
assertion) is still here, though.


http://gerrit.cloudera.org:8080/#/c/4533/2/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

PS2, Line 251: CHECK_OK
Still got some leftover CHECK_* statements that should be converted to their 
ASSERT_* equivalents.


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

PS1, Line 194: 
> Done, although not sure if we need a toggle given that the routine anyways 
On second thought, the ResolveAddresses output isn't even used, is it? So let's 
drop it altogether. Looks like address resolution happens within BuildProxy() 
so that extra call was redundant from day one (in ConfigChange() as well).


PS1, Line 274: 
 : 
> Nit: reformat as "Force the tablet's leader replica (if present) to step do
You missed these comments down here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1363: Add IN-list predicate type

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1363: Add IN-list predicate type
..

KUDU-1363: Add IN-list predicate type

Adds support in the C++ client for providing a set of equalities for a
given column. Support for using IN list predicates from the Java client
will be in a follow-up commit.

Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_predicate-internal.h
M src/kudu/client/scan_predicate.cc
M src/kudu/client/scan_predicate.h
M src/kudu/client/value.h
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/common.proto
M src/kudu/common/key_util.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
16 files changed, 1,000 insertions(+), 37 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1363: Add IN-list predicate type

Dan Burkert has posted comments on this change.

Change subject: KUDU-1363: Add IN-list predicate type
..


Patch Set 18:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/2986/18/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:

Line 464: // = |  |
> nit: is this supposed to be 
Done


Line 472: // <) AND
> Just in case, consider adding tests for boundary conditions like
The equality ones are already tested in the equality + range section.  I've 
added some additional tests to make sure single element lists get simplified to 
an equality.   Added the rest.


http://gerrit.cloudera.org:8080/#/c/2986/18/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:

PS18, Line 357: ASSERT_EQ
> Here and below for the ASSERT_EQ() macro:
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] added insert-generated-rows into kudu tools
..

[tools] added insert-generated-rows into kudu tools

The insert-generated-rows tool has been merged into the 'kudu'
umbrella toolset.

This addresses KUDU-1628.

Besides, added ability to run multiple inserter threads and
specify additional parameters on batching behavior of the generated
write operations.  It's possible to run data generating sessions
both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes.  Introduced
sequential and random modes for the data generator.  Overall, these
changes allow to use the tool to measure performance of the Kudu C++
client library in simplistic 'push-as-much-as-you-can' scenario:
the client generates and sends data as fast as it can.

Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/insert-generated-rows.cc
A src/kudu/tools/kudu-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_local_replica.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
17 files changed, 758 insertions(+), 179 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Adar Dembo has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] docs: add master permanent failure recovery workflow

Adar Dembo has submitted this change and it was merged.

Change subject: docs: add master permanent failure recovery workflow
..


docs: add master permanent failure recovery workflow

While testing this I filed KUDU-1620; this wasn't an issue in
master_failover-itest because it (obviously) can't do any DNS aliasing.

Change-Id: I49d63efa76166bc548db75b0e43ae317c49f9e95
Reviewed-on: http://gerrit.cloudera.org:8080/4436
Reviewed-by: David Ribeiro Alves 
Tested-by: Adar Dembo 
---
M docs/administration.adoc
1 file changed, 165 insertions(+), 14 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Adar Dembo: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I49d63efa76166bc548db75b0e43ae317c49f9e95
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves

[kudu-CR] docs: add master permanent failure recovery workflow

Adar Dembo has posted comments on this change.

Change subject: docs: add master permanent failure recovery workflow
..


Patch Set 2: Verified+1

Overriding Jenkins, the build failures were due to other thirdparty patches in 
flight and not to do with this doc change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I49d63efa76166bc548db75b0e43ae317c49f9e95
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] added insert-generated-rows into kudu tools
..

[tools] added insert-generated-rows into kudu tools

The insert-generated-rows tool has been merged into the 'kudu'
umbrella toolset.

This addresses KUDU-1628.

Besides, added ability to run multiple inserter threads and
specify additional parameters on batching behavior of the generated
write operations.  It's possible to run data generating sessions
both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes.  Introduced
sequential and random modes for the data generator.  Overall, these
changes allow to use the tool to measure performance of the Kudu C++
client library in simplistic 'push-as-much-as-you-can' scenario:
the client generates and sends data as fast as it can.

Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/insert-generated-rows.cc
A src/kudu/tools/kudu-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_local_replica.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
A src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/tools/tool_action_wal.cc
M src/kudu/tools/tool_main.cc
17 files changed, 769 insertions(+), 179 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Dan Burkert has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2:

Two reasons:

It's typically about 35 seconds to repro on ve0158; when we fix it we'll have 
to run the test for perhaps 60 seconds to really make sure it doesn't happen.

The issue already has pretty good coverage through alter_table-randomized-itest.

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

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

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Dinesh Bhat has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 13:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4412/12/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

PS12, Line 452: t of master a
> The original approach was to be able to use any table structure, not only s
Yeah, my comment at L457 was kinda related to this. i.e, I am not sure if we 
need to be flexible wrt schemas - we can have one static schema table and load 
generate and delete the table at the end of the test with an option to keep it 
if we want to append rows later. Basically when it comes to tests, one default 
approach could be 'leave_no_trace' philosophy. Imagine if I run this tool 
continuously for 100 times without exposing a button to delete the data 
generated from these tests, which means we are growing the data in unbounded 
fashion.


PS12, Line 455: et
> Yes, here and elsewhere in the description for parameters: this is intentio
Other tools seem to have followed just one space, so was trying to be 
consistent with them.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] docs: add master permanent failure recovery workflow

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: docs: add master permanent failure recovery workflow
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I49d63efa76166bc548db75b0e43ae317c49f9e95
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] WIP: consensus: refactor tracking of received OpIds out of ReplicaState

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: consensus: refactor tracking of received OpIds out of 
ReplicaState
..


Patch Set 1:

I like the consolidation and, as I had said on the mailing list, I'm +1 on the 
removal of the test, so, in general, I think this patch is a good idea.

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

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

[kudu-CR] KUDU-1612 - [python] Enable setting of read mode for scanning

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1612 - [python] Enable setting of read mode for scanning
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4520/2/python/kudu/tests/util.py
File python/kudu/tests/util.py:

Line 122: self.snapshot_timestamp = datetime.datetime.utcnow()
> Since this op is being flushed why would it not be guaranteed? Since its bl
because the server's clock might not have the same time as the system's clock 
(it's not a pure physical clock). i.e. the time returned by the system clock 
might behind or ahead of the time the server assigned to the write. it's 
unlikely but it can happen and thus it would make this test flaky.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c61ef09f6e15bad2c44d9caf85b2cc2582b8a49
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1363: Add IN-list predicate type

Alexey Serbin has posted comments on this change.

Change subject: KUDU-1363: Add IN-list predicate type
..


Patch Set 18:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/2986/18/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:

Line 464: // = |  |
nit: is this supposed to be 

[--->
  | | |
=   | |

?

I hope you are reading this comment in a monospace font :)


Line 472: // <) AND
Just in case, consider adding tests for boundary conditions like

[--) AND
   |


[--) AND
  ||


[--) AND
|  |


Also, I didn't notice that the following cases are covered:

  [) AND
|


  [) AND
|  |


<-) AND
|

<-> AND
 |

   MAX_INT
<-) AND
  |

Consider adding those as well.

Besides, a tiny nit: consider re-grouping the test cases so the range-related 
ones come in one block (if, of course, there isn't any other grouping criterion 
in effect here which I might be missing).


http://gerrit.cloudera.org:8080/#/c/2986/18/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:

PS18, Line 357: ASSERT_EQ
Here and below for the ASSERT_EQ() macro:
if I'm not mistaken, the expected value is the first parameter, the actual is 
the second.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 2:

why not include the test and only run it in slow mode? jenkins runs tests with 
dist-test so I don't think that 30 secs is going to be a problem there.

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

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

[kudu-CR] KUDU-861 Support changing default, storage attributes

Dan Burkert has posted comments on this change.

Change subject: KUDU-861 Support changing default, storage attributes
..

Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4310/6/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:

Line 147: ByteString defaultByteString =
ProtobufHelper.objectToByteStringNoType(name, newDefault);
> Yeah, but that's not a good excuse for me not to at least try and improve i
After thinking about this a bit more (and hitting a similar issue with the IN
list stuff), I'm not sure how it could be realistically improved. Adding a
KuduValue type interface doesn't really make the situation any better; there is
still the opportunity for a type mismatch with the column. Probably best to
leave as-is for now.

http://gerrit.cloudera.org:8080/#/c/4310/6/src/kudu/client/table_alterer-internal.cc
File src/kudu/client/table_alterer-internal.cc:

Line 106: RETURN_NOT_OK(s.spec->ToColumnSchemaDelta(_delta));
This does some of the checks above, right? Maybe it would be best to
consolidate all of them in ToColumnSchemaDelta.

http://gerrit.cloudera.org:8080/#/c/4310/6/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

PS6, Line 353: gscoped_ptr
> Done. Is there a goal to remove gscoped_ptr in favor of unique_ptr througho
Yes, I think long term. It's hairy to update that much code at once, though.
For now we are just trying to not introduce more usage of gscoped_ptr.

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

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

[kudu-CR] KUDU-1563. Add support for INSERT IGNORE

2016-09-28 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1563. Add support for INSERT IGNORE
..


Patch Set 7:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/4491/7/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:

Line 80: using client::KuduColumnSchema;
> warning: using decl 'KuduColumnSchema' is unused [misc-unused-using-decls]
not your fault, but could you address these warnings? hard to keep track of the 
new ones versus the old ones. you can leave out the TODO(username) ones that 
are not introduced by this patch.


Line 411: LOG(FATAL) << "r = " << r;
can you add more info to the output? i.e. what is "r". the row key might also 
be helpful


http://gerrit.cloudera.org:8080/#/c/4491/7/src/kudu/tablet/tablet-test-base.h
File src/kudu/tablet/tablet-test-base.h:

Line 326:   // Inserts "count" rows.
add: "... , ignoring errors in the case of duplicate keys." or something like 
that.


PS7, Line 354:   if (i % 2 == 0) {
 :   CHECK_OK(writer.Insert(row));
 : } else {
 :   CHECK_OK(writer.InsertIgnore(row));
 : }
 :   }
not sure about this block. What are you trying to do here. You've clearly 
already added a new op type. Are you trying to make it so that most tests, 
which use regular inserts, also test the insert ignore code path? I like the 
idea, but not sure this is the best way to go. doesn't this make some tests 
fail (and if not shouldn't it?).


http://gerrit.cloudera.org:8080/#/c/4491/7/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

Line 401: if (present) {
would still rather have a switch here (this was one of the intentions of the 
original comment, though I now notice I didn't mention it at all, guess you 
didn't read my mind :) ).


Line 407: op->SetInsertIgnoreSucceeded();
this method name here reads even weirder. How about: 
op->SetInsertErrorIgnored() or something


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

Line 365: // refer to INSERT IGNORE. It refers to inserts ignored during 
log replay
add: "... purposefully ignored due to duplicate key errors during log replay" 
or something like that.


http://gerrit.cloudera.org:8080/#/c/4491/2/src/kudu/tablet/tablet_metrics.h
File src/kudu/tablet/tablet_metrics.h:

PS2, Line 50: scoped_refptr rows_insert_ignored;
> Yeah I can see folding this into rows_Inserted. At the same time, I like mo
how about renaming the var to something like: insert_errors_ignored. or 
something of the kind


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bfc35e9d27bd5e2d3375b68e6e4716ed671f36c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Brock Noland 
Gerrit-Reviewer: Brock Noland 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Dan Burkert has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4551/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS1, Line 498:   // This codepath can be used in read-only mode as part of 
a tool running
 :   // against a live tablet server. In this case, the tablet 
server may be
 :   // actively appending entries to the container, so the 
data_file_size may
 :   // need to be occasionally updated to account for new data 
entries.
 :   // CheckBlockRecord will crash the process when 
data_file_size is less
 :   // than what the metadata record indicates.
> Nit: can you reword this comment to reduce its scope? We're deep inside a b
Done


Line 504: 
> Nit: remove this empty line?
Done


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

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

[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
..

KUDU-1657: read-only FsManager::Open on active tablet can crash

See the comment and JIRA for explanation. No test is included, since the
only reproducible case takes approximately 30 seconds to run.
alter_table-randomized-test is made significantly flaky by this issue,
so it has some amount of coverage already.

The repro creates a single tablet with many columns and no replication,
and writes data to it as fast as possible. Another thread creates a file
system manager in a loop, which opens a log block manager on the local
tablet. The repro can be found at:
https://github.com/danburkert/kudu/commit/e919ca410941b268993824da7668ee69c5a7b31c

Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770
---
M src/kudu/fs/log_block_manager.cc
1 file changed, 11 insertions(+), 2 deletions(-)


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

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

[kudu-CR] thirdparty: reorganize tree

Dan Burkert has posted comments on this change.

Change subject: thirdparty: reorganize tree
..


Patch Set 1:

(3 comments)

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

Line 1: #!/bin/bash
Is this necessary?


Line 43: restore_env() {
Should this now include MODE_SUFFIX?  I think that might be good so that we 
will fail fast if we try to use it without setting it.

Although now that I'm looking, I don't think we set -u, so it may not make a 
difference.


Line 439:   rsync -a $BOOST_SOURCE/boost $PREFIX/include
Not your change, but this rsync incantation is using different flags than the 
others, is that intentional?


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

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

[kudu-CR] [java-client] IN-list predicate

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] IN-list predicate
..


Patch Set 6: Code-Review+2

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

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

[kudu-CR] [tools] added insert-generated-rows into kudu tools

Alexey Serbin has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools
..


Patch Set 12:

(20 comments)

http://gerrit.cloudera.org:8080/#/c/4412/11/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

PS11, Line 358:   }
  :   if (total_row_count != nullptr) {
  : *total_row_count = accumulate(row_count.begin(), 
row_count.end(), 0UL);
  :   }
> Looks like it's still here?
Oops, my bad.  I'm surprised myself -- probably, I got distracted.  Let me 
remove it, though.


http://gerrit.cloudera.org:8080/#/c/4412/12/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

PS12, Line 21: options
> s/options/option ?
Done


Line 33: // --num_threads=1 \
> don't you need master_addresses here as required params ? also applicable i
Good catch -- these examples became stale once I switched to the mandatory 
master(s) location.  Will update


PS12, Line 75: master server(s) location
> as per the code below there is no default_params for master server location
Yes, correct -- since changing to use master addresses as mandatory parameter, 
this no longer has default value.  Will update.


Line 80: // bench_c_00_03
> Same as above. Also here and above, we could keep this name intuitive to re
Done


Line 119: using kudu::Schema;
> I think Schema is not being used anywhere.
Done


Line 131: using kudu::client::sp::shared_ptr;
> Actually a basic Qn here: why do we use this special shared_ptr  as opposed
It's just a typedef: it might be either std::shared_ptr or 
std::tr1::shared_ptr, check $REPO_ROOT/src/kudu/client/shared_ptr.h  That's 
because Kudu C++ client API uses this notation to be compatible with C++03.


PS12, Line 142: 100
> Nit: Not sure if you want to insert 1 million by default, if I am a curious
1M rows are inserted really fast: at my laptop it's just 5 seconds.  Also, the 
idea was to create some _load_ :)  With 1K rows there is no load at all.  
Besides, take into the consideration that the original insert-generated-rows 
tools was running with no limit at generated rows at all.

So, what do you think would be the right number here?


Line 194:   int64_t NextImpl() {
> Do we want to use uint64_t here ? Although Random.Next64() seems to be fill
OK, that makes sense.  Will update to uint64_t.


PS12, Line 215: != 0
> this check is prolly redundant ?
As I remember, Tidy Bot thinks it's better for readability: that's the reason I 
left it here.  I'll try to get rid of it -- let's see what it will say.


PS12, Line 285: case UNKNOWN_DATA:  // fall-through
> redundant ?
Yes, it seems so.  I copied the switch enumerators from some place and left 
this UNKNOWN_DATA as is.  Will remove.


Line 302:  FLAGS_buffer_flush_watermark_pct));
> I vaguely recall cpp guideline saying next spill being after 4 spaces I gue
QtCreator's editor does this sometimes.  Will fix.


PS12, Line 314: FLAGS_num_rows_per_thread
> consider using variable FLAGS_ directly to be consistent with other places.
I wanted to have a local variable since I access it in multiple places.


PS12, Line 315: num_rows_per_thread == 0
> given it's default value being 1M, what's this check for ?
Take a look at the comment for the '--num_rows_per_thread': 0 means no limit.


Line 324:   if (row_count != nullptr) {
> DCHECK or CHECK for this, unless we have a use case where it could actually
I don't see why DCHECK or CHECK is needed here.  One can pass nullptr as 
rowcount: that means there isn't any interest in getting the count.  It's a 
ubiquitous notation for pointer out parameters, which I want to have here, even 
if nobody uses it right now.


Line 403:   Stopwatch sw(Stopwatch::ALL_THREADS);
> Stupid Qn: Does ALL_THREADS here mean all threads on CPU or all threads gen
Threads in this process, so the monitoring/main thread will also be counted in.

I don't want LOG_TIMING, since I want to output that information in my own 
format, not what LOG_TIMING provides.


PS12, Line 440: optional scan afterwards
> I think this need not be made explicit since this is listed in optional par
I want this to be present to emphasize that there are two parts: the write and 
the read one, even if the latter is optional.  That's an essence of this test 
in a terse phrasing, otherwise it would be necessary to read through all the 
list of options to understand about the read part.


PS12, Line 452: kTableNameArg
> Can we not make this table_name optional and create one if the user doesn't
The original approach was to be able to use any table structure, not only some 
fixed pre-generated one which the test would create.  So, this test retrieves 
the table schema out of existing table it's pointed at.  That's because 
specifying custom table structure via parameters of this test would be 
cumbersome.
  
We can add this an option, with pairing

[kudu-CR] [tools] Implement a manual leader step down for a tablet

Dinesh Bhat has posted comments on this change.

Change subject: [tools] Implement a manual leader_step_down for a tablet
..


Patch Set 1:

(13 comments)

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

PS1, Line 12:   
> Nit: got an extra space here.
Done


PS1, Line 18: tablet
> You mean 'table' here, right?
yeah, thanks for catching.


http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

PS1, Line 178: Status s = itest::GetConsensusState(ts,
 : tablet_id,
 : 
consensus::CONSENSUS_CONFIG_COMMITTED,
 : 
MonoDelta::FromSeconds(10), );
 : if (!s.ok()) {
 :   return s;
 : }
> Just use RETURN_NOT_OK(...).
Done


Line 198:   int num_retries = 0;
> Why is this defined way up here instead of closer to where it's used?
Done, was thinking of keeping the *retries* variables together initially, but I 
agree about moving it down just before the use. Also just made it one variable 
now.


PS1, Line 204: CHECK_EQ
> Use ASSERT_* instead of CHECK_* since all of these are on the main test thr
Sure, could you tell me what's the relationship between this being a main 
thread and these macros here ? I remember asking this to you earlier in one of 
earlier rev comments but forgot to follow up in-person. 
I am just curious if there are debug-only versions of these macros ?


Line 218:   CHECK_EQ(master_reported_leader, current_leader);
> The leader can change between the GetInfoFromConsensus calls and L213, righ
In reality yes, but given that we are operating in a deterministic test 
environment where the server failures are manually triggered by code, I thought 
this check is safe to have. However, I am not entirely sure what this check is 
buying us, I just happen to notice that there are more than one way we could 
find the leader replica, and I thought it would be good to compare them when we 
know that they don't change on the fly during the test. I can remove this if 
you think this might be a room for any flakiness.


PS1, Line 230: // Wait for re-election to happen again.
 : // 3 seconds picked to accommodate consensus heartbeat 
timeout(1.5s)
> Use AssertEventually() to make this more robust, and to minimize the amount
I tried this with an unsuccessful result, later realized AssertEventually() 
kinda doesn't seem to fit this scenario here; Reason being: a) We really want 
to check the consensus state after the heartbeat timeout as opposed to 
executing f() and then sleeping;  AssertEventually() aggressively(every ms) 
keeps executing the f() until the timeout hits. b) I won't be able to use 
assert from inside the lambda argument when we know that Consensus state is 
still under flux.
Side note: I found one minor bug in AssertEventually() here which I can fix in 
another patch:
int attempts = 0;
int sleep_ms = (attempts < 10) ? (1 << attempts) : 1000;
SleepFor(sleep_ms);

Perhaps we can devise another routine which sleeps for N secs and then executes 
the function and returns the status ?


Line 264:   CHECK_EQ(master_reported_leader, "");
> Let's not check this. It's a common calling convention that if a function f
Done


PS1, Line 271:   CHECK_EQ(current_leader, "");
 :   CHECK_EQ(current_term, 0);
> Likewise here, which means you needn't initialize current_term.
Done


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

PS1, Line 194:   Status s = GetTabletLeader(client, tablet_id, _uuid, 
_hp);
> Consider consolidating from here to ResolveAddresses in a helper function, 
Done, although not sure if we need a toggle given that the routine anyways 
returns the Status and caller can decide if NotFound is FATAL or not ?


PS1, Line 198: else if (!s.ok()) {
 : return s;
 :   }
> To pile on with Tidy Bot, just do RETURN_NOT_OK(s) after the not found chec
Done


PS1, Line 209:   ServerStatusPB status;
 :   RETURN_NOT_OK(GetServerStatus(leader_hp.host(), 
leader_hp.port(), ));
> What's this for? I don't see status used anywhere.
I kinda misunderstood the combination of RETURN_NOT_OK(GetServerStatus... 
thinking that this will return failure if leader is not up; My goal here was to 
check if leader is up & running so that we need not send the RPC if the server 
is already down. It looks like this doesn't buy us anything since RPC will fail 
anyways if the leader is down. Hence removed.


PS1, Line 224:  return Status::RemoteError(resp.ShortDebugString());
> Why not return StatusFromPB(resp.error().status()) like ConfigChange?
Sounds good, just didn't know about this routine, thanks.


-- 
To view, visit

[kudu-CR] [tools] Implement a manual leader step down for a tablet

Hello David Ribeiro Alves, Adar Dembo, Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: [tools] Implement a manual leader_step_down for a tablet
..

[tools] Implement a manual leader_step_down for a tablet

This change introduces a leader_step_down functionality
under 'kudu tablet'. This tool may be handy to recover from
situations when a single tablet server is overloaded and we want
to kick off a new election to balance the load across the clusters.
Although it is not guaranteed that a different replica will be elected
as the leader, this is an optimistic effort to elect a new tablet
server as the leader for the given tablet in the cluster.

Also snuck in a small change to display host:port details with
'kudu table list  --list_tablets' command.

Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_table.cc
M src/kudu/tools/tool_action_tablet.cc
3 files changed, 180 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [tools] added insert-generated-rows into kudu tools