[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread Anonymous Coward (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..

python-client: fix test_scan_rows_string_predicate_and_projection

Revert unneccesary change to test in test_scanner.py in commit
3bf125f469a2929f7fe20e4e31f073376f37575d. The test
test_scan_rows_string_predicate_and_projection was already working
and using column names is both easier to read and more reliable in
the even of a schema change than the column index numbers.

Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
---
M python/kudu/tests/test_scanner.py
1 file changed, 1 insertion(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 5
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: a...@phdata.io


[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread Anonymous Coward (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..

python-client: fix test_scan_rows_string_predicate_and_projection

Revert unneccesary change to test in test_scanner.py in commit
3bf125f469a2929f7fe20e4e31f073376f37575d. The test
test_scan_rows_string_predicate_and_projection was already working
and using column names is both easier to read and more reliable in
the even of a schema change than the column index numbers.

Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
---
M python/kudu/tests/test_scanner.py
1 file changed, 1 insertion(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 4
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: a...@phdata.io


[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread Anonymous Coward (Code Review)
a...@phdata.io has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10255 )

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10255/2/python/kudu/tests/test_scanner.py@74
PS2, Line 74: sv = self.table['string_val']
> nit: since you're here mind removing the extra blank lines here and below u
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 3
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Tue, 01 May 2018 00:47:05 +
Gerrit-HasComments: Yes


[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread Anonymous Coward (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..

python-client: fix test_scan_rows_string_predicate_and_projection

Revert unneccesary change to test in test_scanner.py in commit
3bf125f469a2929f7fe20e4e31f073376f37575d. The test
test_scan_rows_string_predicate_and_projection was already working
and using column names is both easier to read and more reliable in
the even of a schema change than the column index numbers.

Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
---
M python/kudu/tests/test_scanner.py
1 file changed, 1 insertion(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 3
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-2214 Update logging to differentiate voting while copying/tombstoned

2018-04-30 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10169 )

Change subject: KUDU-2214 Update logging to differentiate voting while 
copying/tombstoned
..


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc@1561
PS1, Line 1561:   LOG_WITH_PREFIX_UNLOCKED(INFO) << "voting based on 
last-logged opid "
> Sure. The reason is that the log message today is misleading -- it says tha
I like Todd's suggestion here and I think it's a good one. From a quick grep, 
it looks like the only places we call this function directly are 
tablet_service.cc and raft_consensus_quorum-test.cc. For 
raft_consensus_quorum-test.cc, we can simply pass { boost::none, 
TABLET_DATA_READY } to all call-sites, so I don't think it should be very 
disruptive to the callers. In tablet_service.cc we already have 'data_state' 
available at the call site.


http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/integration-tests/tablet_copy-itest.cc
File src/kudu/integration-tests/tablet_copy-itest.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/integration-tests/tablet_copy-itest.cc@1328
PS1, Line 1328: TEST_F(TabletCopyITest, TestTabletCopyVotingLog) {
I don't think this test actually tests the change in this patch. Maybe we could 
parse the logs generated from existing tombstoned voting tests to validate 
this. However most of our logging-only changes don't actually come with a test 
because as far as I'm aware we haven't built helper infrastructure for that. We 
often manually validate logging changes.


http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/integration-tests/tablet_copy-itest.cc@1378
PS1, Line 1378: RequestVote
nit: make this a complete sentence or maybe remove this comment?


http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/tserver/tablet_service.cc@1006
PS1, Line 1006: LOG(INFO) << "Attempting to vote while "
> yes actually.
I think it would be preferable to incorporate this into the code path in 
raft_consensus.cc instead of adding additional log lines, from a performance 
perspective as well as a usability perspective (we already have a log-spew 
problem)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07007601d0a86d6161065629ba167121a33635d6
Gerrit-Change-Number: 10169
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 01 May 2018 00:42:43 +
Gerrit-HasComments: Yes


[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10255 )

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10255/2/python/kudu/tests/test_scanner.py@74
PS2, Line 74:
nit: since you're here mind removing the extra blank lines here and below until 
scanner.open() ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 2
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 01 May 2018 00:42:00 +
Gerrit-HasComments: Yes


[kudu-CR] python-client: fix test scan rows string predicate and projection

2018-04-30 Thread Anonymous Coward (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: python-client: fix 
test_scan_rows_string_predicate_and_projection
..

python-client: fix test_scan_rows_string_predicate_and_projection

Revert unneccesary change to test in test_scanner.py in commit
3bf125f469a2929f7fe20e4e31f073376f37575d. The test
test_scan_rows_string_predicate_and_projection was already working
and using column names is both easier to read and more reliable in
the even of a schema change than the column index numbers.

Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
---
M python/kudu/tests/test_scanner.py
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 2
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Revert unneccesary change to test in test scanner.py in commit 3bf125f469a2929f7fe20e4e31f073376f37575d. The test test scan rows string predicate and projection was already working and using

2018-04-30 Thread Anonymous Coward (Code Review)
a...@phdata.io has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10255


Change subject: Revert unneccesary change to test in test_scanner.py in commit 
3bf125f469a2929f7fe20e4e31f073376f37575d. The test 
test_scan_rows_string_predicate_and_projection was already working and using 
column names is both easier to read and more reliable in the eve
..

Revert unneccesary change to test in test_scanner.py in commit
3bf125f469a2929f7fe20e4e31f073376f37575d. The test
test_scan_rows_string_predicate_and_projection was already working
and using column names is both easier to read and more reliable in
the even of a schema change than the column index numbers.

Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
---
M python/kudu/tests/test_scanner.py
1 file changed, 2 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I12c900c1366fad61f5c04d023d80fea1f488ef19
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 1
Gerrit-Owner: a...@phdata.io


[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

2018-04-30 Thread Hao Hao (Code Review)
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..

A new Jepsen checker for READ_YOUR_WRITES scan mode

This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.

Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 201 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10237 )

Change subject: [tests] fixed flake in consensus_peer_health_status
..

[tests] fixed flake in consensus_peer_health_status

Fixed flake in the TestPeerHealthStatusTransitions scenario of the
ConsensusPeerHealthStatusITest test.  Prior to the fix, the flakiness
happened when the target tablet server was shutdown during an on-going
tablet copy, where the tablet copy was initiated by AddServer() call
while preparing the mini-cluster for the peer health sequence of
"HEALTHY -> UNKNOWN -> FAILED -> FAILED_UNRECOVERABLE".
In that situation, the source tablet server had corresponding WAL
segments anchored, so they could not be GCed.  As a result, the tablet
replica would not get FAILED_UNRECOVERABLE health status in 30 seconds
because --tablet_copy_idle_timeout_sec is set to 600 seconds by default.

Test results using dist-test, before and after the fix (ASAN),
before:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525111507.120645

after:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525099526.63998

Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Reviewed-on: http://gerrit.cloudera.org:8080/10237
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/consensus_peer_health_status-itest.cc
1 file changed, 6 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Gerrit-Change-Number: 10237
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

2018-04-30 Thread Hao Hao (Code Review)
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..

A new Jepsen checker for READ_YOUR_WRITES scan mode

This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.

Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 201 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10237 )

Change subject: [tests] fixed flake in consensus_peer_health_status
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Gerrit-Change-Number: 10237
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 22:50:17 +
Gerrit-HasComments: No


[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

2018-04-30 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9526 )

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..


Patch Set 5:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/build.gradle
File java/kudu-jepsen/build.gradle:

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/build.gradle@55
PS5, Line 55:   def scanMode = propertyWithDefault("scanMode", 
"READ_AT_SNAPSHOT")
> hum, why is this a global property? don't we want to test both modes?
Updated to run both modes.


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/pom.xml@43
PS5, Line 43: READ_AT_SNAPSHOT
> same question
Done


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj:

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@19
PS5, Line 19:   "Set test"
> could you point the original cockroachdb impl?
Done


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@19
PS5, Line 19:   "Set test"
> Yep, that would be helpful.
Trying to do that, but not many comments were presented there and most of the 
comments do not apply to this variation anymore. So added more comments. Let me 
know if it is still not clear.


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@130
PS5, Line 130: (using READ_YOUR_WRITES
 : ;; mode)
> this doesn't need to be exclusive to the RYW mode right? we could potential
Done


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@132
PS5, Line 132: successfully writes performed by that client.
> didn't we establish that we also need to check that the count didn't go dow
Good point, updated.


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@133
PS5, Line 133: (defn sets-test
> thanks for adding the docs, it helps a lot.
Done


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@133
PS5, Line 133: (defn sets-test
> nit: another option would be adding Clojure-style comment, if it makes any
Done


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj:

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@113
PS5, Line 113: AsyncKuduScanner$ReadMode/READ_YOUR_WRITES
> this should be a param, no?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 22:50:43 +
Gerrit-HasComments: Yes


[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

2018-04-30 Thread Hao Hao (Code Review)
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..

A new Jepsen checker for READ_YOUR_WRITES scan mode

This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.

Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/build.gradle
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
M java/kudu-jepsen/src/utils/kudu_test_runner.clj
5 files changed, 202 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [tools] ksck improvements [6/n]: Refactor result handling

2018-04-30 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [tools] ksck improvements [6/n]: Refactor result handling
..


Patch Set 13:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a994
PS12, Line 994:
> This is redundant- if FLAGS_consensus = false, then the ContainsKey on L801
Gotcha, thanks for the clarification!


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@194
PS12, Line 194: return master->FetchConsensusState();
  : });
> I think this is actually consistent with the GSG, since this lambda is an a
Yeah, I agree it's readable so I'm fine with it, so long as it's consistent 
within the PS.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@284
PS12, Line 284: r (const auto& entry : cluster_->tablet_servers()) {
> I agree it's a lot but I think 4 argument continuation + 2 for inside block
Yeah, I can buy that.


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

http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc@a146
PS11, Line 146:
> Hmm don't see this in PS12
I think you missed this?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 13
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 21:29:05 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10199 )

Change subject: java: fix minor synchronization issues exposed by error-prone
..


Patch Set 3: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:56:48 +
Gerrit-HasComments: No


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10199 )

Change subject: java: fix minor synchronization issues exposed by error-prone
..

java: fix minor synchronization issues exposed by error-prone

* Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions().
  This isn't likely to cause problems since it only happens on 'close'
  and would only race if a new session was concurrently started.

* Add missing GuardedBy annotations in a few spots (non-functional)

* Fix synchronization in TableLocationsCache.toString() to properly
  acquire the lock. Unlikely to cause problems in practice since we only
  call toString() on this in some rare trace messages (if that).

* Fix synchronization in FakeDNS (test-only code)

Change-Id: I1c737f59928f393883d198872419e8832dfff006
Reviewed-on: http://gerrit.cloudera.org:8080/10199
Reviewed-by: Grant Henke 
Tested-by: Todd Lipcon 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
4 files changed, 16 insertions(+), 5 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has removed a vote on this change.

Change subject: java: fix minor synchronization issues exposed by error-prone
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10196 )

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..

java: fix error-prone 'StringSplit' pattern[1]

This should be a non-functional change.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/StringSplit.md

Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Reviewed-on: http://gerrit.cloudera.org:8080/10196
Reviewed-by: Grant Henke 
Tested-by: Todd Lipcon 
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M 
java/kudu-client-tools/src/test/java/org/apache/kudu/mapreduce/tools/ITExportCsv.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
5 files changed, 5 insertions(+), 5 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has removed a vote on this change.

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10196
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10196 )

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..


Patch Set 3: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:56:00 +
Gerrit-HasComments: No


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..

java: prohibit use of a KuduTable from an unassociated KuduClient

This fixes a request-tracking issue with the following code
anti-pattern in which a KuduTable associated with one client is used to
create operations applied to another client's session:

  KuduClient client1 = KuduClientBuildernewClient();
  KuduTable t = client1.openTable(...);
  KuduClient client2 = KuduClientBuildernewClient();
  KuduSession s = client2.newSession();
  s.apply(t.newUpdate());

This would cause sequence numbers to be generated out of the session's
client's RequestTracker, but then marked complete in the operation's
client's RequestTracker. This could cause any number of issues:

- a cache "hit" on the server side might cause an operation to get an
  unrelated operation's response
- a cache "miss" on the server side might result in an operation
  incorrectly being marked as already-responded and garbage collected,
  causing it to fail.

This patch adds a Preconditions check for this issue and fixes some tests
where it was triggered.

Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Reviewed-on: http://gerrit.cloudera.org:8080/7362
Reviewed-by: David Ribeiro Alves 
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRequestTracker.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
9 files changed, 104 insertions(+), 58 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error-prone DefaultCharset[1] issues

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10197 )

Change subject: java: fix error-prone DefaultCharset[1] issues
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10197/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java:

http://gerrit.cloudera.org:8080/#/c/10197/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java@938
PS3, Line 938:   KuduPredicate.newInListPredicate(binaryCol, 
ImmutableList.of(bB, bD)),
> What happened to "e".getBytes?
oops, good catch...



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
Gerrit-Change-Number: 10197
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:55:30 +
Gerrit-HasComments: Yes


[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9526 )

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj:

http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@19
PS5, Line 19:   "Set test"
> could you point the original cockroachdb impl?
Yep, that would be helpful.

Also, it would be nice to preserve the original comments in the code, if there 
were any.


http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@133
PS5, Line 133: (defn sets-test
> thanks for adding the docs, it helps a lot.
nit: another option would be adding Clojure-style comment, if it makes any 
sense.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:34:58 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10196 )

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:00:32 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2399: Support IS NULL / IS NOT NULL predicates in Python

2018-04-30 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10072 )

Change subject: KUDU-2399: Support IS NULL / IS NOT NULL predicates in Python
..

KUDU-2399: Support IS NULL / IS NOT NULL predicates in Python

Add is_null and is_not_null predicates in kudu-python. Had to change
the KuduTestBase and TestScanner to default to None and not replace
None with 'nothing' so I could write tests for null predicates.
I also fixed the test_scan_rows_string_predicate_and_projection test
since it was missing the projections and failing.

Change-Id: I52a22d6d8a8fe9a6049270596eb131799c37f82f
Reviewed-on: http://gerrit.cloudera.org:8080/10072
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/util.py
5 files changed, 93 insertions(+), 10 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  David Ribeiro Alves: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I52a22d6d8a8fe9a6049270596eb131799c37f82f
Gerrit-Change-Number: 10072
Gerrit-PatchSet: 8
Gerrit-Owner: a...@phdata.io
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: a...@phdata.io


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10199 )

Change subject: java: fix minor synchronization issues exposed by error-prone
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:57:57 +
Gerrit-HasComments: No


[kudu-CR] java: fix error-prone DefaultCharset[1] issues

2018-04-30 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10197 )

Change subject: java: fix error-prone DefaultCharset[1] issues
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10197/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java:

http://gerrit.cloudera.org:8080/#/c/10197/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java@938
PS3, Line 938:   KuduPredicate.newInListPredicate(binaryCol, 
ImmutableList.of(bB, bD)),
What happened to "e".getBytes?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
Gerrit-Change-Number: 10197
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:57:17 +
Gerrit-HasComments: Yes


[kudu-CR] [tools] ksck improvements [6/n]: Refactor result handling

2018-04-30 Thread Will Berkeley (Code Review)
Hello Alexey Serbin, Andrew Wong,

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

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

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

Change subject: [tools] ksck improvements [6/n]: Refactor result handling
..

[tools] ksck improvements [6/n]: Refactor result handling

This patch refactors ksck so the results of the checks are collected in
a new KsckResults struct. Printing results is now a matter of iterating
over the KsckResults struct and formatting the information within it.
Furthermore, the KsckResults struct also serves as a programmatic access
point to the results of ksck, which can be used for other things like
rebalancing, auto-repair, or machine-readable printing.

There's also a couple of bonus changes:
- Added Ksck::Run and Ksck::RunAndPrintResults methods, which simplify
  running all ksck checks and printing the results, as is done in the
  ksck CLI tool and in ClusterVerifier.
- Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were
  about to be committed but would've needed to be redone a bit to fit
  with this refactor.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
10 files changed, 1,195 insertions(+), 880 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 13
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [tools] ksck improvements [6/n]: Refactor result handling

2018-04-30 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [tools] ksck improvements [6/n]: Refactor result handling
..


Patch Set 12:

(9 comments)

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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a281
PS12, Line 281:
  :
  :
> Is this still no longer needed?
Done


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a994
PS12, Line 994:
> This too?
This is redundant- if FLAGS_consensus = false, then the ContainsKey on L801 of 
PS12 will return false and no consensus state will be recorded.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a1024
PS12, Line 1024:
> This too?
Same, if FLAGS_consensus = false, r.consensus_state will be boost::none.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@194
PS12, Line 194: return master->FetchConsensusState();
  : });
> nit: not 100% sure how to interpret the GSG wrt lambdas (I would've thought
I think this is actually consistent with the GSG, since this lambda is an 
argument to a function and arguments get indented 4. There's then 2 more spaces 
for regular indentation inside a block.

I'm not particular on it looking a certain way if there's rough consistency and 
it's readable. I'll make 286 consistent with this.

Languages need tools that do the formatting so we can't stop worrying about 
stuff like this.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@284
PS12, Line 284:   VLOG(1) << "Going to connect to tablet server: " << 
ts->uuid();
> nit: same here wrt spacing. Maybe I'm missing something but six spaces seem
I agree it's a lot but I think 4 argument continuation + 2 for inside block is 
right. Imagine if the lambda were on a line of its own-- then the 4 would 
definitely be right, right?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@297
PS12, Line 297: const char* const wrong_uuid_msg = "ID reported by tablet 
server "
  :"doesn't match 
the expected ID";
> Do we have test cases that would fail if this message were to change?
It's worth moving this to a separate patch.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@300
PS12, Line 300:
> nit: strange place for a space ?
Done


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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck_results.cc@182
PS12, Line 182: || master_consensus_conflict
> This should also be gated by FLAGS_consensus, right? Or in PrintConsensusMa
It'll always be false if there's no consensus info.


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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/tool_action_cluster.cc@a45
PS12, Line 45:
 :
 :
> This probably belongs in the commit message too (or in a separate patch). I
It just moved to ksck.cc.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 12
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:48:48 +
Gerrit-HasComments: Yes


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:41:06 +
Gerrit-HasComments: No


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-30 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..


Patch Set 6: Code-Review+2

thanks


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:29:24 +
Gerrit-HasComments: No


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10196 )

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..


Patch Set 1:

Fixed the spot in the integration test build


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:24:06 +
Gerrit-HasComments: No


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7362/5/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java:

http://gerrit.cloudera.org:8080/#/c/7362/5/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java@241
PS5, Line 241: so we shouldn't unregister it.
> oh, I guess the TODO is more what I added elsewhere that it doesn't really
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:24:14 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-30 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins, Grant Henke,

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

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

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

Change subject: java: fix error-prone 'StringSplit' pattern[1]
..

java: fix error-prone 'StringSplit' pattern[1]

This should be a non-functional change.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/StringSplit.md

Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M 
java/kudu-client-tools/src/test/java/org/apache/kudu/mapreduce/tools/ITExportCsv.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
5 files changed, 5 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10199 )

Change subject: java: fix minor synchronization issues exposed by error-prone
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java@237
PS1, Line 237: rwl.readLock();
> woops, yes! I'm surprised error-prone doesn't have a check for this pattern
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:23:46 +
Gerrit-HasComments: Yes


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-30 Thread Todd Lipcon (Code Review)
Hello Alexey Serbin, David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins, 
Anonymous Coward #314, Grant Henke,

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

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

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

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..

java: prohibit use of a KuduTable from an unassociated KuduClient

This fixes a request-tracking issue with the following code
anti-pattern in which a KuduTable associated with one client is used to
create operations applied to another client's session:

  KuduClient client1 = KuduClientBuildernewClient();
  KuduTable t = client1.openTable(...);
  KuduClient client2 = KuduClientBuildernewClient();
  KuduSession s = client2.newSession();
  s.apply(t.newUpdate());

This would cause sequence numbers to be generated out of the session's
client's RequestTracker, but then marked complete in the operation's
client's RequestTracker. This could cause any number of issues:

- a cache "hit" on the server side might cause an operation to get an
  unrelated operation's response
- a cache "miss" on the server side might result in an operation
  incorrectly being marked as already-responded and garbage collected,
  causing it to fail.

This patch adds a Preconditions check for this issue and fixes some tests
where it was triggered.

Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRequestTracker.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
9 files changed, 104 insertions(+), 58 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error-prone DefaultCharset[1] issues

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10197 )

Change subject: java: fix error-prone DefaultCharset[1] issues
..


Patch Set 1:

Switched to StandardCharsets


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
Gerrit-Change-Number: 10197
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 19:23:56 +
Gerrit-HasComments: No


[kudu-CR] java: fix error-prone DefaultCharset[1] issues

2018-04-30 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins, Grant Henke,

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

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

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

Change subject: java: fix error-prone DefaultCharset[1] issues
..

java: fix error-prone DefaultCharset[1] issues

This should be a non-functional change on all systems where UTF8 is the
default character set. It may fix issues on other systems which are
exceedingly rare these days.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/DefaultCharset.md

Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
---
M 
java/kudu-client-tools/src/test/java/org/apache/kudu/mapreduce/tools/ITImportCsv.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTestUtils.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
M 
java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/ITOutputFormatJob.java
M java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
18 files changed, 67 insertions(+), 47 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
Gerrit-Change-Number: 10197
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-30 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins, Grant Henke,

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

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

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

Change subject: java: fix minor synchronization issues exposed by error-prone
..

java: fix minor synchronization issues exposed by error-prone

* Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions().
  This isn't likely to cause problems since it only happens on 'close'
  and would only race if a new session was concurrently started.

* Add missing GuardedBy annotations in a few spots (non-functional)

* Fix synchronization in TableLocationsCache.toString() to properly
  acquire the lock. Unlikely to cause problems in practice since we only
  call toString() on this in some rare trace messages (if that).

* Fix synchronization in FakeDNS (test-only code)

Change-Id: I1c737f59928f393883d198872419e8832dfff006
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
4 files changed, 16 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] benchmarks: update parameters for mt-bloomfile benchmark

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10242 )

Change subject: benchmarks: update parameters for mt-bloomfile benchmark
..

benchmarks: update parameters for mt-bloomfile benchmark

The old parameters were somewhat unrealistic:
- The default bloom block size is 4kb, but it was using 32kb.
- The cache capacity was set to 1MB, which meant that each shard on a
  multicore system was only 50KB or less. With the new optimization that
  allows 1% slack in MemTracker accounting, that slack was only
  500bytes or less. With 4kb bloom blocks, that optimization was
  effectively disabled.

The new parameters are slightly more realistic while keeping the
performance reasonable: 100MB cache capacity (>1MB per shard even on 88
core system) and 4kb blocks. Runtime is still reasonable - 4-5 seconds
for the query phase. The number of unique keys is bumped up to 150M to
ensure cache churn.

Change-Id: I0a85fe85ee20d1e21ddf260cdb50c4994e96
Reviewed-on: http://gerrit.cloudera.org:8080/10242
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/scripts/benchmarks.sh
1 file changed, 5 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0a85fe85ee20d1e21ddf260cdb50c4994e96
Gerrit-Change-Number: 10242
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] benchmarks: update parameters for mt-bloomfile benchmark

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10242 )

Change subject: benchmarks: update parameters for mt-bloomfile benchmark
..


Patch Set 1:

The performance of the benchmark got slower when I pushed the improvement for 
memtracker contention in the LRU cache :)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a85fe85ee20d1e21ddf260cdb50c4994e96
Gerrit-Change-Number: 10242
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:58:18 +
Gerrit-HasComments: No


[kudu-CR] [tools] ksck improvements [6/n]: Refactor result handling

2018-04-30 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [tools] ksck improvements [6/n]: Refactor result handling
..


Patch Set 12:

(11 comments)

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

http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/ksck.cc@297
PS11, Line 297: t wrong_uuid_msg = "ID reported by tablet server "
  :"doesn't match 
the expected ID";
> :/ I hacked in a more precise but more brittle check. I'm not sure we can s
Hrmm.. Thanks for making this change, sorry I'm gonna be waffle here and say I 
think this sort of thing probably also belongs in a separate patch too, since 
it could probably use a test too. Maybe keep this as it was, open a JIRA, and 
point this at it?

Or add a test in this patch and scope creep a little bit (including a point in 
the commit message)


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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a281
PS12, Line 281:
  :
  :
Is this still no longer needed?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a994
PS12, Line 994:
This too?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a1024
PS12, Line 1024:
This too?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@194
PS12, Line 194: return master->FetchConsensusState();
  : });
nit: not 100% sure how to interpret the GSG wrt lambdas (I would've thought 
it'd be four spaces to the left), so I'll just point at this and look to you 
for confirmation that this is how you want it to be spaced.

See L286, I was expecting something more like that.


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@284
PS12, Line 284:   VLOG(1) << "Going to connect to tablet server: " << 
ts->uuid();
nit: same here wrt spacing. Maybe I'm missing something but six spaces seems 
like a lot?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@297
PS12, Line 297: const char* const wrong_uuid_msg = "ID reported by tablet 
server "
  :"doesn't match 
the expected ID";
Do we have test cases that would fail if this message were to change?


http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@300
PS12, Line 300:
nit: strange place for a space ?


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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck_results.cc@182
PS12, Line 182: || master_consensus_conflict
This should also be gated by FLAGS_consensus, right? Or in 
PrintConsensusMatrix() maybe


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

http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc@a146
PS11, Line 146:
> Done
Hmm don't see this in PS12


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

http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/tool_action_cluster.cc@a45
PS12, Line 45:
 :
 :
This probably belongs in the commit message too (or in a separate patch). Is 
this completely gone?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 12
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:45:15 +
Gerrit-HasComments: Yes


[kudu-CR] mini-cluster: support parallel multi-master clusters

2018-04-30 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8280 )

Change subject: mini-cluster: support parallel multi-master clusters
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8280/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8280/3//COMMIT_MSG@11
PS3, Line 11: This allows for a few
: simplifications:
There's only one simplification in this list now.


http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/external_mini_cluster.cc@317
PS3, Line 317:   vector> reserved_sockets;
Worth a comment explaining why it's OK for this to go out of scope after all of 
the masters have been started.

Actually, if we're going to close the 
different-process-grabs-our-port-during-restart race, shouldn't we keep this 
alive for the lifetime of the cluster?


http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/internal_mini_cluster.cc
File src/kudu/mini-cluster/internal_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/internal_mini_cluster.cc@119
PS3, Line 119:   vector> reserved_sockets;
Likewise.


http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/mini_cluster.cc
File src/kudu/mini-cluster/mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8280/3/src/kudu/mini-cluster/mini_cluster.cc@80
PS3, Line 80:   socket->reset(new Socket());
Style nit: prefer the calling convention where OUT params are unmodified in the 
event of an error. So, set up a local unique_ptr and swap to 'socket' 
when you can no longer fail.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b0ff7bfc179d8fdb1ed306d1bbd12acddeb060c
Gerrit-Change-Number: 8280
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:27:07 +
Gerrit-HasComments: Yes


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Alexey Serbin (Code Review)
Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: [tests] fixed flake in consensus_peer_health_status
..

[tests] fixed flake in consensus_peer_health_status

Fixed flake in the TestPeerHealthStatusTransitions scenario of the
ConsensusPeerHealthStatusITest test.  Prior to the fix, the flakiness
happened when the target tablet server was shutdown during an on-going
tablet copy, where the tablet copy was initiated by AddServer() call
while preparing the mini-cluster for the peer health sequence of
"HEALTHY -> UNKNOWN -> FAILED -> FAILED_UNRECOVERABLE".
In that situation, the source tablet server had corresponding WAL
segments anchored, so they could not be GCed.  As a result, the tablet
replica would not get FAILED_UNRECOVERABLE health status in 30 seconds
because --tablet_copy_idle_timeout_sec is set to 600 seconds by default.

Test results using dist-test, before and after the fix (ASAN),
before:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525111507.120645

after:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525099526.63998

Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
---
M src/kudu/integration-tests/consensus_peer_health_status-itest.cc
1 file changed, 6 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Gerrit-Change-Number: 10237
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] rpc: add experimental rpc reuseport flag

2018-04-30 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8279 )

Change subject: rpc: add experimental rpc_reuseport flag
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8279/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8279/3//COMMIT_MSG@11
PS3, Line 11: except RHEL 6.4 and below.
As we discussed, could you run an el6.6 binary on an el6.4 VM and see what the 
behavior is? Does setsockopt() fail? Does it no-op? What's the effect on our 
unit tests and/or minicluster CLI?


http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/rpc/messenger.h
File src/kudu/rpc/messenger.h:

http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/rpc/messenger.h@166
PS3, Line 166: SO_REUSEADDR
SO_REUSEPORT


http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:

http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/rpc/messenger.cc@182
PS3, Line 182:   reuseport_ = true;
reuseport_ needs a default value.


http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/util/net/socket.cc
File src/kudu/util/net/socket.cc:

http://gerrit.cloudera.org:8080/#/c/8279/3/src/kudu/util/net/socket.cc@263
PS3, Line 263: Status Socket::SetReusePort(bool flag) {
Seems like a fair amount of code could be consolidated if there was a single 
setsockopt() helper function that dealt with errno and generated a reasonable 
Status on failure.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d8ce9faa646fa2be554f5cfdf8b6ed0c48b496e
Gerrit-Change-Number: 8279
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:14:56 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2422. Fix TSAN hangs in Subprocess::Start

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10240 )

Change subject: KUDU-2422. Fix TSAN hangs in Subprocess::Start
..

KUDU-2422. Fix TSAN hangs in Subprocess::Start

This pulls in an upstream patch from LLVM to fix an occasional hang when
starting processes in TSAN builds[1]

Tested with:

KUDU_ALLOW_SLOW_TESTS=0 ./build-support/dist_test.py loop -n 1000 \
  ./build/latest/bin/subprocess-test --gtest_filter=\*CurrentDir \
  --gtest_repeat=1000 --stress-cpu-threads=3

Without the patch, two of the tasks hung in Subprocess::Start and
failed. With the patch, all succeeded.

[1] https://github.com/google/sanitizers/issues/945

Change-Id: I9426b28c8a925b6d59ec4241aa12007a07f6ec70
Reviewed-on: http://gerrit.cloudera.org:8080/10240
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M thirdparty/download-thirdparty.sh
A 
thirdparty/patches/llvm-tsan-disable-trace-switching-after-multithreaded-for.patch
2 files changed, 56 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9426b28c8a925b6d59ec4241aa12007a07f6ec70
Gerrit-Change-Number: 10240
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10237 )

Change subject: [tests] fixed flake in consensus_peer_health_status
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10237/1//COMMIT_MSG@11
PS1, Line 11: happened when the target tablet server was shutdown during an 
on-going
: tablet copy.  In that situation, the source tablet server had
: c
> curious why you took this approach instead of just waiting for the tablet c
Ah, it seems I omitted some useful information on where the flake happened, 
I'll add that.

The flake happened in the last line of the test scenario, and , as I 
understand, the tablet copy that hold anchor as triggered by AddServer() at 
line 197.

Yes, I tried ASSERT_OK(WaitUntilTabletRunning(follower_ts, tablet_id_, 
kTimeout)) instead of NO_FATALS(wait_for_health_state(leader_ts, follower_uuid, 
HEALTHY)) at line 198 and it also worked.  I thought maybe it's worth keeping 
this interesting situation with hung tablet copy (even if it's rare).

>From the other side, if we want to be explicit and just start from scratch, 
>then WaitUntilTabletRunning() after AddServer() is the best approach, I think. 
> All right, I'll change to that one.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Gerrit-Change-Number: 10237
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:09:01 +
Gerrit-HasComments: Yes


[kudu-CR] benchmarks: update parameters for mt-bloomfile benchmark

2018-04-30 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10242 )

Change subject: benchmarks: update parameters for mt-bloomfile benchmark
..


Patch Set 1: Code-Review+2

What tipped you off that the parameters needed to be tweaked?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a85fe85ee20d1e21ddf260cdb50c4994e96
Gerrit-Change-Number: 10242
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:08:46 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2422. Fix TSAN hangs in Subprocess::Start

2018-04-30 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10240 )

Change subject: KUDU-2422. Fix TSAN hangs in Subprocess::Start
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9426b28c8a925b6d59ec4241aa12007a07f6ec70
Gerrit-Change-Number: 10240
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 18:06:29 +
Gerrit-HasComments: No


[kudu-CR] benchmarks: update parameters for mt-bloomfile benchmark

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10242


Change subject: benchmarks: update parameters for mt-bloomfile benchmark
..

benchmarks: update parameters for mt-bloomfile benchmark

The old parameters were somewhat unrealistic:
- The default bloom block size is 4kb, but it was using 32kb.
- The cache capacity was set to 1MB, which meant that each shard on a
  multicore system was only 50KB or less. With the new optimization that
  allows 1% slack in MemTracker accounting, that slack was only
  500bytes or less. With 4kb bloom blocks, that optimization was
  effectively disabled.

The new parameters are slightly more realistic while keeping the
performance reasonable: 100MB cache capacity (>1MB per shard even on 88
core system) and 4kb blocks. Runtime is still reasonable - 4-5 seconds
for the query phase. The number of unique keys is bumped up to 150M to
ensure cache churn.

Change-Id: I0a85fe85ee20d1e21ddf260cdb50c4994e96
---
M src/kudu/scripts/benchmarks.sh
1 file changed, 5 insertions(+), 3 deletions(-)



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

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


[kudu-CR] mini-cluster: support parallel multi-master clusters

2018-04-30 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8280 )

Change subject: mini-cluster: support parallel multi-master clusters
..


Patch Set 3:

(14 comments)

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

http://gerrit.cloudera.org:8080/#/c/8280/1//COMMIT_MSG@9
PS1, Line 9: This commit refactors the mini-clusters to internally use reserved
> This change is also to the internal mini cluster though, right?
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/CMakeLists.txt@a63
PS1, Line 63:
> RUN_SERIAL was just to reduce concurrent test load, IIRC, but we probably d
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_cert_authority-itest.cc
File src/kudu/integration-tests/master_cert_authority-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_cert_authority-itest.cc@186
PS1, Line 186:   MasterServiceProxy proxy(messenger_, m->bound_rpc_addr(), 
m->bound_rpc_addr().host());
> Not used, can be removed.
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_failover-itest.cc@150
PS1, Line 150:   }
> Can be removed.
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_replication-itest.cc
File src/kudu/integration-tests/master_replication-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/master_replication-itest.cc@135
PS1, Line 135: .set_range_partition_columns({ "key" })
> Can be removed; iterate on opts_.num_masters_ instead.
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/token_signer-itest.cc
File src/kudu/integration-tests/token_signer-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/token_signer-itest.cc@126
PS1, Line 126:   InternalMiniClusterOptions opts_;
> Remove; use opts_.num_masters_ instead.
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/integration-tests/token_signer-itest.cc@127
PS1, Line 127:   unique_ptr cluster_;
> Not sure what purpose this serves either; could just as easily use opts_.nu
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.h
File src/kudu/mini-cluster/external_mini_cluster.h:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.h@251
PS1, Line 251:   int num_tablet_servers() const override {
> What purpose does this now serve?
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.h@345
PS1, Line 345:
 :  private:
 :   FRIEND_T
> Only for masters though, right? Might want to clarify that.
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.cc@295
PS1, Line 295:   return paths;
> Good cleanup here, but it might be better to pull it out into its own patch
The diff between the old StartDistributedMasters and the new StartMasters is 
pretty minimal, pretty much only the reuse port stuff.


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/external_mini_cluster.cc@318
PS1, Line 318:   vector master_rpc_addrs;
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/internal_mini_cluster.h
File src/kudu/mini-cluster/internal_mini_cluster.h:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/internal_mini_cluster.h@211
PS1, Line 211:
 :   Env* const env_;
 :
> Masters only?
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/internal_mini_cluster.cc
File src/kudu/mini-cluster/internal_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/internal_mini_cluster.cc@125
PS1, Line 125:   
RETURN_NOT_OK_PREPEND(ReserveDaemonSocket(MiniCluster::MASTER, i, 
opts_.bind_mode,
> This is repeated in ExternalMiniCluster; consolidate into a helper? Could b
Done


http://gerrit.cloudera.org:8080/#/c/8280/1/src/kudu/mini-cluster/internal_mini_cluster.cc@223
PS1, Line 223: MiniTabletServer* 
InternalMiniCluster::mini_tablet_server_by_uuid(const string& uuid) const {
> Why is this implementation any different than ExternalMiniCluster's impleme
This has changed slightly, but the two implementations are slightly different.



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

[kudu-CR] rpc: add experimental rpc reuseport flag

2018-04-30 Thread Dan Burkert (Code Review)
Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: rpc: add experimental rpc_reuseport flag
..

rpc: add experimental rpc_reuseport flag

This option sets the SO_REUSEPORT socket option on a server's bound
socket. This socket option is supported on all platforms which Kudu
supports, except RHEL 6.4 and below.

The motivation is to use the option in the minicluster so that master
ports can be reserved by the mini cluster control processes and reused
by the master processes.

Change-Id: I5d8ce9faa646fa2be554f5cfdf8b6ed0c48b496e
---
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/messenger.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/util/net/socket.cc
M src/kudu/util/net/socket.h
7 files changed, 41 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d8ce9faa646fa2be554f5cfdf8b6ed0c48b496e
Gerrit-Change-Number: 8279
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] mini-cluster: support parallel multi-master clusters

2018-04-30 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: mini-cluster: support parallel multi-master clusters
..

mini-cluster: support parallel multi-master clusters

This commit refactors the mini-clusters to internally use reserved
sockets for their child daemons. Reserved sockets are simply sockets
bound to a random port with SO_REUSEPORT. This allows for a few
simplifications:

* master addresses in multi-master clusters can be reserved up-front,
  which removes any chance of a race condition in binding. As a result,
  multi-master clusters no longer need hard-coded ports, and tests using
  multi-master clusters no longer need to run in sequence.

WIP: This commit removes the ability to set the desired master ports.
master_migration-itest relies on this functionality, so it's currently
broken.

Change-Id: I0b0ff7bfc179d8fdb1ed306d1bbd12acddeb060c
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/client-stress-test.cc
M src/kudu/integration-tests/master-stress-test.cc
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/master_migration-itest.cc
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-master-auth-itest.cc
M src/kudu/integration-tests/token_signer-itest.cc
M src/kudu/integration-tests/webserver-stress-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/CMakeLists.txt
M src/kudu/mini-cluster/external_mini_cluster-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.cc
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/tool_action_test.cc
27 files changed, 170 insertions(+), 276 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b0ff7bfc179d8fdb1ed306d1bbd12acddeb060c
Gerrit-Change-Number: 8280
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10237 )

Change subject: [tests] fixed flake in consensus_peer_health_status
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10237/1//COMMIT_MSG@11
PS1, Line 11: happened when the target tablet server was shutdown during an 
on-going
: tablet copy.  In that situation, the source tablet server had
: c
curious why you took this approach instead of just waiting for the tablet copy 
to finish? The tablet copy here is caused by the prior part of the test, right?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
Gerrit-Change-Number: 10237
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Apr 2018 16:41:02 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2422. Fix TSAN hangs in Subprocess::Start

2018-04-30 Thread Todd Lipcon (Code Review)
Hello Adar Dembo,

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

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

to review the following change.


Change subject: KUDU-2422. Fix TSAN hangs in Subprocess::Start
..

KUDU-2422. Fix TSAN hangs in Subprocess::Start

This pulls in an upstream patch from LLVM to fix an occasional hang when
starting processes in TSAN builds[1]

Tested with:

KUDU_ALLOW_SLOW_TESTS=0 ./build-support/dist_test.py loop -n 1000 \
  ./build/latest/bin/subprocess-test --gtest_filter=\*CurrentDir \
  --gtest_repeat=1000 --stress-cpu-threads=3

Without the patch, two of the tasks hung in Subprocess::Start and
failed. With the patch, all succeeded.

[1] https://github.com/google/sanitizers/issues/945

Change-Id: I9426b28c8a925b6d59ec4241aa12007a07f6ec70
---
M thirdparty/download-thirdparty.sh
A 
thirdparty/patches/llvm-tsan-disable-trace-switching-after-multithreaded-for.patch
2 files changed, 56 insertions(+), 2 deletions(-)



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

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


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 15:29:46 +
Gerrit-HasComments: No


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..

Fix table formatting in full data dirs docs

Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Reviewed-on: http://gerrit.cloudera.org:8080/10211
Tested-by: Alexey Serbin 
Reviewed-by: Alexey Serbin 
---
M docs/administration.adoc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 2: Verified+1

Unrelated flake in on of tests in TSAN build.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 15:29:43 +
Gerrit-HasComments: No


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [tests] fixed flake in consensus peer health status

2018-04-30 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10237


Change subject: [tests] fixed flake in consensus_peer_health_status
..

[tests] fixed flake in consensus_peer_health_status

Fixed flake in the TestPeerHealthStatusTransitions scenario of the
ConsensusPeerHealthStatusITest test.  Prior to the fix, the flakiness
happened when the target tablet server was shutdown during an on-going
tablet copy.  In that situation, the source tablet server had
corresponding WAL segments anchored, so they could not be GCed.
As a result, the tablet replica would not get FAILED_UNRECOVERABLE
health status assigned.

Test results using dist-test, before and after the fix (ASAN),
before:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525098763.54471

after:
  http://dist-test.cloudera.org/job?job_id=aserbin.1525099526.63998

Change-Id: I8eb640604e98361029aa3342ffa3050e922b6629
---
M src/kudu/integration-tests/consensus_peer_health_status-itest.cc
1 file changed, 1 insertion(+), 0 deletions(-)



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

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


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Will Berkeley (Code Review)
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: Fix table formatting in full data dirs docs
..

Fix table formatting in full data dirs docs

Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
---
M docs/administration.adoc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 2:

Rendered by GitHub: 
https://github.com/wdberkeley/kudu/blob/docfix/docs/administration.adoc


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 14:47:34 +
Gerrit-HasComments: No


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-30 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10211/1/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/10211/1/docs/administration.adoc@828
PS1, Line 828: options="he
> Why not to drop this at all?  At least, asciidoctor is able to auto-detect
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 30 Apr 2018 14:47:06 +
Gerrit-HasComments: Yes