[kudu-CR] python-client: fix test scan rows string predicate and projection
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]
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]
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]
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
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
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
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]
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
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
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
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
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
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
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
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]
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
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]
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
