[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11429 ) Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. [catalog_manager] optimization in AsyncAddReplicaTask Introduced a minor optimization on AsyncAddReplicaTask::SendRequest() while populating the set of blacklisted candidates for an extra replica. With this patch, the code iterates over the UUIDs of the peers in the committed config to populate the set of the blacklisted candidates, rather than filtering all tablet servers using the IsRaftConfigMember() utility function. Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Reviewed-on: http://gerrit.cloudera.org:8080/11429 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/master/catalog_manager.cc 1 file changed, 21 insertions(+), 12 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2245 Graceful leadership transfer
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11251 )
Change subject: KUDU-2245 Graceful leadership transfer
..
Patch Set 7:
(14 comments)
Few more nits.
http://gerrit.cloudera.org:8080/#/c/11251/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11251/7//COMMIT_MSG@10
PS7, Line 10: original Raft thesis
I think a reference (like https://raft.github.io/raft.pdf) would not hurt.
http://gerrit.cloudera.org:8080/#/c/11251/7//COMMIT_MSG@16
PS7, Line 16: beings
begins
Maybe replace with 'starts' ?
http://gerrit.cloudera.org:8080/#/c/11251/7//COMMIT_MSG@33
PS7, Line 33: Leadership transfer should usually be much faster and it
: allows the client to select the new leader among current voters.
Did you do any measurements in that regard? If yes, I think it would be nice
to mention the summary of your findings in this commit message.
http://gerrit.cloudera.org:8080/#/c/11251/7//COMMIT_MSG@42
PS7, Line 42: WIP
Is it still WIP patch? The WIP tag in the first line is gone. Should I assume
those 3 items below are still TODO or already done?
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h
File src/kudu/consensus/consensus-test-util.h:
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@299
PS7, Line 299: virtual
nit: drop 'virtual' since 'override' is used.
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@355
PS7, Line 355: virtual
nit: drop
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@482
PS7, Line 482: virtual
nit: drop
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h
File src/kudu/consensus/consensus_queue.h:
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@362
PS7, Line 362: boost:none
boost::none
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@365
PS7, Line 365: WatchForSuccessor
nit: maybe, rename to BeginWatchForSuccessor() to pair with the
EndWatchForSuccessor() below.
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@559
PS7, Line 559: designated_successor_
designated_successor_uuid_ ?
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@593
PS7, Line 593: virtual void NotifyPeerToStartElection(const std::string&
peer_uuid) = 0;
Add documentation.
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/integration-tests/raft_consensus-itest.cc@3001
PS7, Line 3001: TEST_F(RaftConsensusITest,
TestLeaderTransferWhenFollowerFallsBehindLeaderGC) {
Nice test. How long does it run? If more than 5 seconds, consider adding
if (!AllowSlowTests()) {
LOG(WARNING) << "test is skipped; set KUDU_ALLOW_SLOW_TESTS=1 to run";
return;
}
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/integration-tests/raft_consensus-itest.cc@3054
PS7, Line 3054:
:
nit: two extra lines
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/tools/kudu-admin-test.cc@1514
PS7, Line 1514: , { }
nit: drop
--
To view, visit http://gerrit.cloudera.org:8080/11251
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083
Gerrit-Change-Number: 11251
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Fengling Wang
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Wed, 12 Sep 2018 06:42:29 +
Gerrit-HasComments: Yes
[kudu-CR] [iwyu] add catalog manager.{cc,h} files
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11430 )
Change subject: [iwyu] add catalog_manager.{cc,h} files
..
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11430
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b65dc90a5e6378286e51fcfdcdefdf499fbd89c
Gerrit-Change-Number: 11430
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 12 Sep 2018 06:37:14 +
Gerrit-HasComments: No
[kudu-CR] [iwyu] add catalog manager.{cc,h} files
Alexey Serbin has uploaded this change for review. (
http://gerrit.cloudera.org:8080/11430
Change subject: [iwyu] add catalog_manager.{cc,h} files
..
[iwyu] add catalog_manager.{cc,h} files
Removed catalog_manager.{cc,h} files from the IWYU's black list.
It seems at this point the IWYU tool no longer outputs conflicting
suggestions.
Change-Id: I3b65dc90a5e6378286e51fcfdcdefdf499fbd89c
---
M build-support/iwyu.py
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
3 files changed, 3 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/11430/1
--
To view, visit http://gerrit.cloudera.org:8080/11430
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b65dc90a5e6378286e51fcfdcdefdf499fbd89c
Gerrit-Change-Number: 11430
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 ) Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 12 Sep 2018 06:12:49 + Gerrit-HasComments: No
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Hello Will Berkeley, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11429 to look at the new patch set (#2). Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. [catalog_manager] optimization in AsyncAddReplicaTask Introduced a minor optimization on AsyncAddReplicaTask::SendRequest() while populating the set of blacklisted candidates for an extra replica. With this patch, the code iterates over the UUIDs of the peers in the committed config to populate the set of the blacklisted candidates, rather than filtering all tablet servers using the IsRaftConfigMember() utility function. Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 --- M src/kudu/master/catalog_manager.cc 1 file changed, 21 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/11429/2 -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 ) Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3474 PS1, Line 3474: excluded.emplace(std::move(desc)); > Nit: EmplaceOrDie() Done http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3480 PS1, Line 3480: auto replacement_replica = SelectReplica(ts_descs, excluded, rng_); > Does SelectReplica() assume that every member of 'excluded' must be in 'ts_ SelectReplica() does not assume that 'excluded' to be among 'ts_descs'. The 'presumably dead' servers are not included into 'ts_descs', but they might be in the 'excluded' set. That should work as expected. Overall it's a good point; I add a comment about that to clarify. -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 12 Sep 2018 05:31:38 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 ) Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3474 PS1, Line 3474: excluded.emplace(std::move(desc)); Nit: EmplaceOrDie() http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3480 PS1, Line 3480: auto replacement_replica = SelectReplica(ts_descs, excluded, rng_); Does SelectReplica() assume that every member of 'excluded' must be in 'ts_descs'? I'm asking because the new approach to populating excluded includes a slight behaviorial change: tservers "presumed dead" are no longer filtered out because LookupTSByUUID doesn't filter out dead tservers as GetAllLiveDescriptors did. -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 12 Sep 2018 05:06:27 + Gerrit-HasComments: Yes
[kudu-CR] [location awareness] replica selection honors placement policy
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11207 )
Change subject: [location_awareness] replica selection honors placement policy
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11207/7/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/11207/7/src/kudu/master/catalog_manager.cc@3391
PS7, Line 3391: for (const auto& ts_desc : ts_descs) {
: if (IsRaftConfigMember(ts_desc->permanent_uuid(),
cstate_.committed_config())) {
: InsertOrDie(&existing, ts_desc);
: }
: }
> It isn't very important because this code path isn't hot and the number of
Done:
http://gerrit.cloudera.org:8080/11429
--
To view, visit http://gerrit.cloudera.org:8080/11207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4169098abf17d5591d4c1675561afc15b5477fcd
Gerrit-Change-Number: 11207
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Wed, 12 Sep 2018 04:45:24 +
Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11429 Change subject: [catalog_manager] optimization in AsyncAddReplicaTask .. [catalog_manager] optimization in AsyncAddReplicaTask Introduced a minor optimization on AsyncAddReplicaTask::SendRequest() while populating the set of blacklisted candidates for an extra replica. With this patch, the code iterates over the UUIDs of the peers in the committed config to populate the set of the blacklisted candidates, rather than filtering all tablet servers using the IsRaftConfigMember() utility function. Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 --- M src/kudu/master/catalog_manager.cc 1 file changed, 9 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/11429/1 -- To view, visit http://gerrit.cloudera.org:8080/11429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413 Gerrit-Change-Number: 11429 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] Add helper macro for tool invocations
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11411 )
Change subject: Add helper macro for tool invocations
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11411/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11411/3/src/kudu/tools/kudu-admin-test.cc@140
PS3, Line 140: "kudu",
I think this should be
ASSERT_TOOL_OK("cluster", "ksck", master_addrs);
right?
--
To view, visit http://gerrit.cloudera.org:8080/11411
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ffd357d79982ee5c93f8d3c7cfd7cc1f0863f07
Gerrit-Change-Number: 11411
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Wed, 12 Sep 2018 04:08:11 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2463 pt 1: adjust MVCC when replaying no-ops
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11142 ) Change subject: KUDU-2463 pt 1: adjust MVCC when replaying no-ops .. Patch Set 5: @mpercy Currently dist testing some seemingly test-only failures, but pushed the major changes in case you wanted to review the changes/tests, had comments/suggestions, etc. -- To view, visit http://gerrit.cloudera.org:8080/11142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f Gerrit-Change-Number: 11142 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 12 Sep 2018 01:54:38 + Gerrit-HasComments: No
[kudu-CR] KUDU-2463 pt 1: adjust MVCC when replaying no-ops
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11142 to look at the new patch set (#5). Change subject: KUDU-2463 pt 1: adjust MVCC when replaying no-ops .. KUDU-2463 pt 1: adjust MVCC when replaying no-ops Previously, during tablet bootstrap, a tablet would only update its MVCC safetime based on write messages, as the timstamps in the write messages are guaranteed to be serialized with respect to one another, by virtue of being assigned in a single thread (the prepare thread) on the leader replica. >From this, we conclude that timestamps for write operations are monotonically increasing in unison with opid. The same cannot necessarily be said for timestamps of no-ops and change configs. This is a conservative conclusion about assigned timestamps, and this patch hinges on the fact that our Raft implementation ensures the following sequence of events: 1. replica A becomes leader of Term N 2. leader A assigns a timestamp t1 to its no-op 3. leader A replicates the no-op to replicas B and C, asserting its leadership for Term N 4. leader A prepares a write and assigns it a timestamp t2 > t1 5. leader A replicates the write to replicas B and C, checking that it is leader for the Term N Given the above series of operations, within the same term, the no-op used to assert leadership is always assigned a timestamp that must be lower than any writes in that term. As such, the timestamps assigned to no-ops can and should be used to bump safetime. This patch does so at bootstrap time with committed no-ops found in the WAL. This alone isn't enough to prevent KUDU-2463, but it reduces the window during which incorrect results can be returned. Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f --- M src/kudu/consensus/consensus.proto M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/raft_consensus-itest.cc A src/kudu/integration-tests/timestamp_advancement-itest.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 6 files changed, 336 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/11142/5 -- To view, visit http://gerrit.cloudera.org:8080/11142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f Gerrit-Change-Number: 11142 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-2463 pt 2: adjust MVCC on Raft no-op
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11427 Change subject: KUDU-2463 pt 2: adjust MVCC on Raft no-op .. KUDU-2463 pt 2: adjust MVCC on Raft no-op Based on the same rationale as Part 1 of this patch series, this patch updates MVCC's safe and clean time using the no-op timestamp provided by the leader following a successful Raft election. There isn't an obvious reference to the tablet (to get to the MVCC module) in Raft consensus, but there is a ReplicaTransactionFactory, that the TabletReplica implements. I've extended this to be a more general ConsensusRoundHandler that can be used to create transactions or finish transactions as needed. An interesting thing to note is that in some cases (e.g. brand new tablets), the first election would replicate the no-op with a timestamp of 1 (the timestamp we're trying to avoid). I tracked the cause of this to be the fact that sometimes the hybrid clock doesn't get updated before sending out the first no-op, and this will result in the first assigned timestamp being 1. To work around this, I updated the clock to the initial clean time, which seems in line with other updates to the hybrid clock in the time manager. I tested this in the following ways: - to ensure nothing terrible happens when there is a lot of election churn (and hence, a lot of timestamp advancement), I've tweaked exactly_once_writes-itest to actually churn elections. Previously it attempted this with just a low timeout; I injected some latency to make it churn a bit harder. - I added a test that ensured that, on its own, a tablet would bump its MVCC timestamps, by virtue of its elections - I tested the above with single-replica tablets as well - a few other tests needed to be tweaked given the extra bump to the hybrid clock Change-Id: Icbf812e2cb7c322fd980245cfe40c886a15a --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/time_manager.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/timestamp_advancement-itest.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h 10 files changed, 179 insertions(+), 78 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/11427/1 -- To view, visit http://gerrit.cloudera.org:8080/11427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icbf812e2cb7c322fd980245cfe40c886a15a Gerrit-Change-Number: 11427 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-2463 pt 3: don't scan if MVCC hasn't moved
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11428 Change subject: KUDU-2463 pt 3: don't scan if MVCC hasn't moved .. KUDU-2463 pt 3: don't scan if MVCC hasn't moved In cases when a tablet bootstrap yields an MvccManager whose clean time has not been advanced (e.g. if there are no write ops in the WAL), and the tablet has otherwise not bumped its MVCC timestamps (e.g. if it has not yet elected a leader), a scan (whose correctness depends on the MvccManager to determine what transactions have been applied) will return incorrect results. In the same way that we prevent compactions from occuring if MVCC's timestamps have not been moved, this patch prevents incorrect results from being returend from a scan, forcing it to be retried elsewhere. Tests are added to attempt to scan in this state, verifying that we get an error. Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf --- M src/kudu/integration-tests/timestamp_advancement-itest.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.h M src/kudu/tablet/mvcc.cc M src/kudu/tablet/mvcc.h M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/tserver/tablet_service.cc 10 files changed, 176 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/11428/1 -- To view, visit http://gerrit.cloudera.org:8080/11428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf Gerrit-Change-Number: 11428 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [location awareness] Add location info in ksck report
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11422 )
Change subject: [location_awareness] Add location info in ksck report
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11422/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11422/1//COMMIT_MSG@29
PS1, Line 29: Tablet Server UUID| Location
: --+---
: fc18b4bdb2ae4dd0a5717e8fe1f1de69 |
: 0f25d1891fce48789e06fdec493fb90e |
: ad868d782dc743369d96a9e958811f81 |
: c541b6ebc3a14f0f9bbee26d52545683 | /foo-bar0/BAAZ.9-quux
:
:Location| Count
: ---+-
: /foo-bar0/BAAZ.9-quux | 1
: | 3
Is this a part of master summary as well?
I thought master summary is information about masters. Maybe, separate the
newly introduced information into 'Location Summary' or something like that?
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck.h@283
PS1, Line 283: std::unordered_map
Consider introducing a typedef for this and document what key and value contain.
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck.h@421
PS1, Line 421: std::string location_;
Is it possible to make this const?
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.h@145
PS1, Line 145: std::unordered_map
I thought KsckServerHealthSummary is just for a single server instance, why is
it necessary to have map of locations in there?
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.cc@367
PS1, Line 367: "\n"
here and below: prefer to use std::endl instead
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.cc@389
PS1, Line 389: std::unordered_map
There is 'using std::unordered_map' in the beginning, so there is no need to
add namespace prefix here.
http://gerrit.cloudera.org:8080/#/c/11422/1/src/kudu/tools/ksck_results.cc@391
PS1, Line 391: for (const auto& loc : s.ts_locations) {
: if (recorded_uuids.count(loc.first)) continue;
: loc_table.AddRow({ loc.first, loc.second });
: recorded_uuids.insert(loc.first);
: location_counts[loc.second]++;
: }
I don't think this is a safe way to get up-to-date information from multiple
masters. What if a non-leader master reports no location for a tablet server,
and that information from a non-leader comes first, but the up-to-date
information coming from leader master comes later?
Overall, is it possible to leverage ListTServers() or from
tool_action_tserver.cc to get that info from the leader master?
--
To view, visit http://gerrit.cloudera.org:8080/11422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c
Gerrit-Change-Number: 11422
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Fengling Wang
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Wed, 12 Sep 2018 00:56:33 +
Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 6: (14 comments) Almost all nits pretty much. Looking good! http://gerrit.cloudera.org:8080/#/c/11263/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11263/6//COMMIT_MSG@9 PS6, Line 9: Link to the version with images: Very nice :) http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md File _posts/2018-08-17-index-skip-scan-optimization-in-kudu.md: http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@39 PS6, Line 39: table nit: tablet, here and elsewhere http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@45 PS6, Line 45: (we will refer to it as : `prefix column` and it's specific value as `prefix key`) nit: since we're not using these as a variable names, but rather as definitions, we should use quotations. Also drop the apostrophe in "its". I.e: we will refer to it as the "prefix column" and its specific value as the "prefix key" http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@48 PS6, Line 48: Therefore, we can use the index to **skip** to the rows that have distinct prefix keys, : and also satisfy the predicate on the `tstamp` column. nit: maybe drop the ** around "skip" here, since you do it down below anyway. http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@58 PS6, Line 58: `host` = helium nit: would be nice if the entire thing were in backticks, since it's a condition? Seems a little awkward, this mix of backticks and no backticks. WDYT? http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@60 PS6, Line 60: such as `ubuntu`, `westeros` nit: probably not needed http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@59 PS6, Line 59: with : this prefix key nit: reword "until the predicate no longer matches. At that point we would know that no more rows with `host = helium` will satisfy the predicate, and we can skip to the next prefix key. http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@67 PS6, Line 67: (s) nit: this is a little distracting, below too. Let's just keep it singular since you call out at the end that it can be any number of prefix columns at the end anyway. http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@73 PS6, Line 73: . This and below are being rendered weirdly by github. Would like to confirm this doesn't happen with jekyll http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@74 PS6, Line 74: consistent performance with : respect to the prefix columns cardinality "consistent performance in cases of large prefix column cardinality" http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@93 PS6, Line 93: on the non-first key columns(s) nit: probably not needed, below too. http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@94 PS6, Line 94: IN list nit: In-list http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@98 PS6, Line 98: Team nit: team http://gerrit.cloudera.org:8080/#/c/11263/6/_posts/2018-08-17-index-skip-scan-optimization-in-kudu.md@101 PS6, Line 101: References : == : : [[1]](https://storage.googleapis.com/pub-tools-public-publication-data/pdf/42851.pdf): Gupta, Ashish, et al. "Mesa: : Geo-replicated, near real-time, scalable data warehousing." Proceedings of the VLDB Endowment 7.12 (2014): 1259-1270. : : [[2]](https://oracle-base.com/articles/9i/index-skip-scanning/): Index Skip Scanning - Oracle Database : : [[3]](https://www.sqlite.org/optoverview.html#skipscan): Skip Scan - SQLite It's really up to you, but WDYT about just linking these in-line? This is a webpage after all :) -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 6 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: A
[kudu-CR] KUDU-2463 pt 1: adjust MVCC when replaying no-ops
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11142 to look at the new patch set (#4). Change subject: KUDU-2463 pt 1: adjust MVCC when replaying no-ops .. KUDU-2463 pt 1: adjust MVCC when replaying no-ops Previously, during tablet bootstrap, a tablet would only update its MVCC safetime based on write messages, as the timstamps in the write messages are guaranteed to be serialized with respect to one another, by virtue of being assigned in a single thread (the prepare thread) on the leader replica. >From this, we conclude that timestamps for write operations are monotonically increasing in unison with opid. The same cannot necessarily be said for timestamps of no-ops and change configs. This is a conservative conclusion about assigned timestamps, and this patch hinges on the fact that our Raft implementation ensures the following sequence of events: 1. replica A becomes leader of Term N 2. leader A assigns a timestamp t1 to its no-op 3. leader A replicates the no-op to replicas B and C, asserting its leadership for Term N 4. leader A prepares a write and assigns it a timestamp t2 > t1 5. leader A replicates the write to replicas B and C, checking that it is leader for the Term N Given the above series of operations, within the same term, the no-op used to assert leadership is always assigned a timestamp that must be lower than any writes in that term. As such, the timestamps assigned to no-ops can and should be used to bump safetime. This patch does so at bootstrap time with committed no-ops found in the WAL. This alone isn't enough to prevent KUDU-2463, but it reduces the window during which incorrect results can be returned. Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f --- M src/kudu/consensus/consensus.proto M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/raft_consensus-itest.cc A src/kudu/integration-tests/timestamp_advancement-itest.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 6 files changed, 332 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/11142/4 -- To view, visit http://gerrit.cloudera.org:8080/11142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f Gerrit-Change-Number: 11142 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Implement BloomFilter Predicate in server side.
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11100 )
Change subject: Implement BloomFilter Predicate in server side.
..
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate-test.cc@20
PS5, Line 20: #include
cstdlib
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.h@20
PS5, Line 20: #include
prefer and , and group with the other C++ headers.
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.h@190
PS5, Line 190: *d
here and elsewhere, put the asterisk with the type (not the variable name).
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.h@262
PS5, Line 262: bool EvaluateCellForBloomFilter(DataType type, const void*
cell) const;
Can this be private?
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.h@298
PS5, Line 298: struct BloomFilterInner {
Please add docs for this class, and especially explain what 'nhash' is.
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.cc@905
PS5, Line 905: return (lower_ == other.lower_ ||
You could reduce the duplication with Range by wrapping this in a lambda.
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.cc@930
PS5, Line 930: return EvaluateCell(column_.type_info()->physical_type(),
value);
Given this definition, why is this method necessary instead of directly calling
EvaluateCell?
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/column_predicate.cc@943
PS5, Line 943: case PredicateType::InBloomFilter: rank = 6; break;
This should probably be ahead of IsNotNull.
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/common.proto
File src/kudu/common/common.proto:
http://gerrit.cloudera.org:8080/#/c/11100/5/src/kudu/common/common.proto@352
PS5, Line 352: message BloomFilter {
Could you extract out the HashAlgorithm message above, and add a field here for
it as well? I think that's the conclusion we came to in
https://gerrit.cloudera.org/c/11333/
--
To view, visit http://gerrit.cloudera.org:8080/11100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
Gerrit-Change-Number: 11100
Gerrit-PatchSet: 5
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Tue, 11 Sep 2018 23:27:39 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2463 pt 1: adjust MVCC when replaying no-ops
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11142 ) Change subject: KUDU-2463 pt 1: adjust MVCC when replaying no-ops .. Patch Set 3: For simplicity of the patch, I've stripped away the "non-serialized" flag from earlier PSs. -- To view, visit http://gerrit.cloudera.org:8080/11142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f Gerrit-Change-Number: 11142 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 11 Sep 2018 23:15:33 + Gerrit-HasComments: No
[kudu-CR] KUDU-2463 pt 1: adjust MVCC when replaying no-ops
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11142 to look at the new patch set (#3). Change subject: KUDU-2463 pt 1: adjust MVCC when replaying no-ops .. KUDU-2463 pt 1: adjust MVCC when replaying no-ops Previously, during tablet bootstrap, a tablet would only update its MVCC safetime based on write messages, as the timstamps in the write messages are guaranteed to be serialized with respect to one another, by virtue of being assigned in a single thread (the prepare thread) on the leader replica. >From this, we conclude that timestamps for write operations are monotonically increasing in unison with opid. The same cannot necessarily be said for timestamps of no-ops and change configs. This is a conservative conclusion about assigned timestamps, and this patch hinges on the fact that our Raft implementation ensures the following sequence of events: 1. replica A becomes leader of Term N 2. leader A assigns a timestamp t1 to its no-op 3. leader A replicates the no-op to replicas B and C, asserting its leadership for Term N 4. leader A prepares a write and assigns it a timestamp t2 > t1 5. leader A replicates the write to replicas B and C, checking that it is leader for the Term N Given the above series of operations, within the same term, the no-op used to assert leadership is always assigned a timestamp that must be lower than any writes in that term. As such, the timestamps assigned to no-ops can and should be used to bump safetime. This patch does so at bootstrap time with committed no-ops found in the WAL. This alone isn't enough to prevent KUDU-2463, but it reduces the window during which incorrect results can be returned. Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/timestamp_advancement-itest.cc M src/kudu/tablet/tablet_bootstrap.cc 3 files changed, 304 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/11142/3 -- To view, visit http://gerrit.cloudera.org:8080/11142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26deff32da8c990cb8a2ba220bb81858ddd6d73f Gerrit-Change-Number: 11142 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] HMS integration: provide Java API to override owner during table create
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java@27 PS3, Line 27: > I don't see an immediate use case. But after Sentry integration I guess the There are a few metadata fields in the HMS which we could expose, but aren't currently. Things like ownership, table & column comments, table properties, etc. If we want to start exposing these I think we should take a more holistic look at it (I suspect, however, that we'll never need to). -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 23:09:03 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle File java/kudu-client/build.gradle: http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle@42 PS4, Line 42: // Avro conflicts with kudu-flume's Avro dependency, and isn't necessary for the HMS client. : exclude group: "org.apache.avro", module: "avro" > I haven't had any issues running the maven build/tests locally. Do we have Until we remove the Maven build entirely, we should continue to ensure it's working (i.e. you can run 'mvn install', the modules compile, and the tests pass). Weird that you didn't need the exclusion for Maven to keep working though. On a related note, Grant has been itching to remove Maven but wants to run through a release or two using Gradle to shake out any remaining bugs. -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 23:06:24 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java File java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java@228 PS3, Line 228: setOwner > We don't have the concept of a service user in Kudu, so it's not straightfo Sounds good. http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java@27 PS3, Line 27: > I'm against providing that API since I don't know of a use-case for it. I don't see an immediate use case. But after Sentry integration I guess the users may want to know the owner of the table to see the ownership? -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:53:30 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle File java/kudu-client/build.gradle: http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle@42 PS4, Line 42: // Avro conflicts with kudu-flume's Avro dependency, and isn't necessary for the HMS client. : exclude group: "org.apache.avro", module: "avro" > Don't we need the same exclusion in pom.xml too? I haven't had any issues running the maven build/tests locally. Do we have a policy of keeping them consistent anyway? -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:49:58 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/build.gradle File java/kudu-client/build.gradle: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/build.gradle@45 PS3, Line 45: testCompile libs.junit > Nit: alphabetical sort order. Done http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: PS3: > Missing Apache header. Done -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:50:34 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle File java/kudu-client/build.gradle: http://gerrit.cloudera.org:8080/#/c/11413/4/java/kudu-client/build.gradle@42 PS4, Line 42: // Avro conflicts with kudu-flume's Avro dependency, and isn't necessary for the HMS client. : exclude group: "org.apache.avro", module: "avro" Don't we need the same exclusion in pom.xml too? -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:48:57 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: provide Java API to override owner during table create
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 4: This is still failing on my machine with a mysterious failure on kudu-backup (see below), but I don't have time to debug it at the moment: > Task :kudu-backup:test FAILED org.apache.kudu.backup.TestKuduBackup > initializationError FAILED java.lang.NoClassDefFoundError: org/apache/kudu/client/MiniKuduCluster Caused by: java.lang.ClassNotFoundException: org.apache.kudu.client.MiniKuduCluster 1 test completed, 1 failed -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:46:48 + Gerrit-HasComments: No
[kudu-CR] HMS integration: provide Java API to override owner during table create
Hello Andrew Wong, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11413 to look at the new patch set (#4). Change subject: HMS integration: provide Java API to override owner during table create .. HMS integration: provide Java API to override owner during table create This is necessary so that Impala can override the table owner, instead of using the inferred username of the connection (which would be 'impala'). This API is marked as unstable since it's not clear we will want to support it long term, and there are some open questions about the fine-grained privileges required to use it. Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 --- M java/kudu-client/build.gradle M java/kudu-client/pom.xml M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto 6 files changed, 103 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/11413/4 -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] HMS integration: set table owner field in HMS table metadata
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11398 ) Change subject: HMS integration: set table owner field in HMS table metadata .. HMS integration: set table owner field in HMS table metadata Other systems use table ownership for purposes like assigning privileges. This patch sets the owner field in the HMS for Kudu tables to the client's user name. A follow-up patch will add additional APIs to the client CreateTable builders which will allow clients to override the owner, for situations in which the client is actually proxying through the table creation on behalf of a different user. The HMS fix tool will create HMS table metadata for Kudu tables which are missing it. This replacement table metadata will omit the table owner, since it can't be reconstructed just from the Kudu table metadata. This is the conservative approach, if we want to extend the tool to allow passing in the owner as a flag we can do that in the future. Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Reviewed-on: http://gerrit.cloudera.org:8080/11398 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/hms/hms_catalog-test.cc M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 7 files changed, 145 insertions(+), 55 deletions(-) Approvals: Kudu Jenkins: Verified Dan Burkert: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Gerrit-Change-Number: 11398 Gerrit-PatchSet: 8 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] HMS integration: set table owner field in HMS table metadata
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11398 ) Change subject: HMS integration: set table owner field in HMS table metadata .. Patch Set 7: Code-Review+2 Carrying through Hao's +2 and Adar's +1. -- To view, visit http://gerrit.cloudera.org:8080/11398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Gerrit-Change-Number: 11398 Gerrit-PatchSet: 7 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 11 Sep 2018 22:45:24 + Gerrit-HasComments: No
[kudu-CR] move KUDU NO EXPORT attribute on methods
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11415 ) Change subject: move KUDU_NO_EXPORT attribute on methods .. move KUDU_NO_EXPORT attribute on methods The original location was ineffective due to a bug in older versions of GCC, [1] appears to be the best tracking issue. Adding the annotation at the end of the method signature silences the warnings on GCC 4.8, and maintains symbol hiding as checked with nm. [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30361 Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 Reviewed-on: http://gerrit.cloudera.org:8080/11415 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/client/client.h 1 file changed, 8 insertions(+), 8 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 Gerrit-Change-Number: 11415 Gerrit-PatchSet: 5 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [location awareness] replica selection honors placement policy
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11207 ) Change subject: [location_awareness] replica selection honors placement policy .. Patch Set 10: Code-Review+1 Will should +2 since there have been some changes since his last review. -- To view, visit http://gerrit.cloudera.org:8080/11207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4169098abf17d5591d4c1675561afc15b5477fcd Gerrit-Change-Number: 11207 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 22:28:59 + Gerrit-HasComments: No
[kudu-CR] [location awareness] replica selection honors placement policy
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11207 ) Change subject: [location_awareness] replica selection honors placement policy .. Patch Set 8: (7 comments) Thank you for the review! http://gerrit.cloudera.org:8080/#/c/11207/9/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11207/9/src/kudu/master/catalog_manager.cc@3394 PS9, Line 3394: } > Nit: emplace_back in new code. I'm going to address this in a separate small patch: Will pointed at the fact that iterating over all tablet servers is not that effective vs iterating just over the members of the tablet Raft group. http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.h File src/kudu/master/placement_policy.h: http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.h@65 PS8, Line 65: typedef std::map LocationToDescriptorsMap; > Makes sense, but do we currently take advantage of key sorting? Do we itera Nope, right now there is no code that would take advantage of the fact that the key are sorted. I assume making this unordered_map for a while would be OK? http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.cc File src/kudu/master/placement_policy.cc: http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.cc@131 PS8, Line 131: location_per_load_overflow.emplace( : GetLocationLoad(location, ltd, locations_info), location); > Could you add a test case that would fail if these weren't multi-maps? Added PlacementPolicyTest.SelectLocationTest -- it also verifies that the placement policy selects locations randomly (uniform distribution). http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.cc@379 PS8, Line 379: auto ins_info = ltd->emplace(std::move(location), TSDescriptorVector()); > The problem with raw emplace() is that it doesn't explain what your intent Yes, that makes sense, but that time LookupOrEmplace() didn't exist yet, and there was code at line 380 that would help understand the intention. However, now we have LookupOrEmplace() and I updated the code to use it. http://gerrit.cloudera.org:8080/#/c/11207/8/src/kudu/master/placement_policy.cc@380 PS8, Line 380: ins_info.first->second.emplace_back(std::move(desc)); > I'm not sure I understand: descs is a const ref, and move is for a copy of But it's possible to use move semantics for 'descs', sure. http://gerrit.cloudera.org:8080/#/c/11207/9/src/kudu/master/placement_policy.cc File src/kudu/master/placement_policy.cc: http://gerrit.cloudera.org:8080/#/c/11207/9/src/kudu/master/placement_policy.cc@71 PS9, Line 71: CHECK(it != ltd.end()); : // Get information on the > Remove? Done http://gerrit.cloudera.org:8080/#/c/11207/9/src/kudu/master/placement_policy.cc@191 PS9, Line 191: > Nit: extra space here. Done -- To view, visit http://gerrit.cloudera.org:8080/11207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4169098abf17d5591d4c1675561afc15b5477fcd Gerrit-Change-Number: 11207 Gerrit-PatchSet: 8 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 22:19:11 + Gerrit-HasComments: Yes
[kudu-CR] [location awareness] replica selection honors placement policy
Hello Will Berkeley, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11207 to look at the new patch set (#10). Change subject: [location_awareness] replica selection honors placement policy .. [location_awareness] replica selection honors placement policy This patch introduces placement policy into the catalog manager's replica selection process. The replica selection logic is factored out into the new PlacementPolicy class. In essence (for details see [1]), the placement policy is about: * in case of N locations, N > 2, not placing the majority of replicas in one location * spreading replicas evenly among available locations * within a location, spreading replicas evenly among tablet servers This patch also adds a few test scenarios for the new functionality. [1] https://s.apache.org/location-awareness-design Change-Id: I4169098abf17d5591d4c1675561afc15b5477fcd --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/placement_policy-test.cc A src/kudu/master/placement_policy.cc A src/kudu/master/placement_policy.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h 10 files changed, 1,217 insertions(+), 211 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/11207/10 -- To view, visit http://gerrit.cloudera.org:8080/11207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4169098abf17d5591d4c1675561afc15b5477fcd Gerrit-Change-Number: 11207 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Will Berkeley
[kudu-CR] move KUDU NO EXPORT attribute on methods
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11415 ) Change subject: move KUDU_NO_EXPORT attribute on methods .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 Gerrit-Change-Number: 11415 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 22:10:07 + Gerrit-HasComments: No
[kudu-CR] [KUDU-2521] Java Implementation for BloomFilter
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11333 )
Change subject: [KUDU-2521] Java Implementation for BloomFilter
..
Patch Set 5:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java
File java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java:
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@30
PS5, Line 30: public class BloomFilter {
Could you add Javadocs to the class as well as to all public
non-visiblefortesting methods, explaining how they work and how to use them?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@33
PS5, Line 33: private byte[] bitmap;
Curious why you chose to implement your own bitmap instead of reusing
java.util.BitSet?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@36
PS5, Line 36: private HashFunction hashFunction;
Some of these fields can be marked final because they're not modified after the
constructor call, right?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@39
PS5, Line 39: this.nBits = nBits;
Should we precondition that this is at least 8?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@40
PS5, Line 40: this.bitmap = bitmap;
Likewise, should we precondition that this has space for at least 8 bits?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@65
PS5, Line 65: nBytes
Shouldn't this be nBits? I'm surprised this worked as-is; could you add a unit
test that would fail or malfunction when this is nBytes?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@74
PS5, Line 74: if (data) {
: byteBuffer[0] = 1;
: } else {
: byteBuffer[0] = 0;
: }
Nit: byteBuffer[0] = data ? 1 : 0;
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@143
PS5, Line 143: private void updateBitmap(byte[] byteBuffer, int length) {
Should we precondition that bitmap.size >= length?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@151
PS5, Line 151: tmp = tmp + h2;
Nit: tmp += h2;
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@155
PS5, Line 155: // This is for test.
Methods whose visibility has been increased for testing should be annotated
with:
@InterfaceAudience.LimitedPrivate("Test”)
Then you don't need the "This is for test" comments either.
See
https://github.com/apache/kudu/commit/abbde75e12f2275e1a286df60d788e9c5b411bcb
for more details.
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@225
PS5, Line 225: private boolean checkIfContains(byte[] bytes) {
Likewise, precondition on bytes.length vs. bitmap size here?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@233
PS5, Line 233: bitpos
Nit: bitPos (camel-case)
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@237
PS5, Line 237: tmp = tmp + h2;
Nit: tmp += h2;
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@243
PS5, Line 243: private static double kNaturalLog2= 0.69314;
Nit: reformat as:
private static double kNaturalLog2 = 0.69314;
(separate the assignment operator '=' from the variable name 'kNaturalLog2'
with a space)
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@257
PS5, Line 257: n_bits,
Nit: camel-case in Java, so 'nBits'
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@271
PS5, Line 271: private static long pickBit(long hash, int nBits) {
Nit: maybe rename to GetBitIndex or some such, to emphasize that what's
returned is an index into a bitmap?
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@276
PS5, Line 276: public String toString() {
Nit: mind implementing this via MoreObjects.toStringHelper(), or even just a
StringBuilder? Easier to follow, I think.
http://gerrit.cloudera.org:8080/#/c/11333/5/java/kudu-client/src/test/java/org/apache/kudu/client/TestBloomFilter.java
File java/kudu-client/src/test/java/org/apache
[kudu-CR] move KUDU NO EXPORT attribute on methods
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11415 to look at the new patch set (#4). Change subject: move KUDU_NO_EXPORT attribute on methods .. move KUDU_NO_EXPORT attribute on methods The original location was ineffective due to a bug in older versions of GCC, [1] appears to be the best tracking issue. Adding the annotation at the end of the method signature silences the warnings on GCC 4.8, and maintains symbol hiding as checked with nm. [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30361 Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 --- M src/kudu/client/client.h 1 file changed, 8 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/11415/4 -- To view, visit http://gerrit.cloudera.org:8080/11415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 Gerrit-Change-Number: 11415 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] HMS integration: provide Java API to override owner during table create
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11413/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11413/3//COMMIT_MSG@7 PS3, Line 7: Java > A C++ client API as well? This commit only includes an unstable Java API. http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java File java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java@228 PS3, Line 228: setOwner > Do we want to validate if this is only be called by a service users? e.g Im We don't have the concept of a service user in Kudu, so it's not straightforward to do that. Without fine grained authorizations I think it's fine to have this be usable by any user who can create a table, however we'll need to revisit this once Kudu supports finer grained access control. http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java@27 PS3, Line 27: provide an API to look up the table owner > +1 it's seams unusual to provide a way to set but not check. It would also I'm against providing that API since I don't know of a use-case for it. -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 21:24:12 + Gerrit-HasComments: Yes
[kudu-CR] build: add option to ignore test failures
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11399 ) Change subject: build: add option to ignore test failures .. build: add option to ignore test failures Sometimes when running build-and-test.sh, we may not care about the failure of any given test, so long as the tests ran, e.g. if running only for the sake of reporting the results of the tests. As such, this patch adds an option to ignore test failures during runs of build-and-test.sh. I tested this by injecting errors into a test and doing a build-and-test.sh run with the new ERROR_ON_TEST_FAILURE=1, verifying that it still returns an error; and with ERROR_ON_TEST_FAILURE=0, verifying that it returns no error. I also broke the build and verified that, despite ERROR_ON_TEST_FAILURE=0, it returns an error. Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7 Reviewed-on: http://gerrit.cloudera.org:8080/11399 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M build-support/jenkins/build-and-test.sh 1 file changed, 21 insertions(+), 12 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7 Gerrit-Change-Number: 11399 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] HMS integration: provide Java API to override owner during table create
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java@27 PS3, Line 27: provide an API to look up the table owner > Do you think it is worth providing such API since table has ownership now? +1 it's seams unusual to provide a way to set but not check. It would also simplify the testing. -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 20:37:29 + Gerrit-HasComments: Yes
[kudu-CR] [master] fix compilation warning of DCHECK NOTNULL
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11421 ) Change subject: [master] fix compilation warning of DCHECK_NOTNULL .. [master] fix compilation warning of DCHECK_NOTNULL Fixed compilation warning in case of RELEASE build: kudu/master/ts_descriptor.cc:103:18: warning: expression result unused [-Wunused-value] DCHECK_NOTNULL(location); ^~~~ This patch does not contain any functional changes. This is a follow-up to dcc39d53d8c36a3f4896ce4c208b855abee8da83. Change-Id: I768afd0ec34f8127f66ea8ca29ec337488c5 Reviewed-on: http://gerrit.cloudera.org:8080/11421 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/master/ts_descriptor.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I768afd0ec34f8127f66ea8ca29ec337488c5 Gerrit-Change-Number: 11421 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] [location awareness] Add location info in ksck report
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 ) Change subject: [location_awareness] Add location info in ksck report .. Patch Set 1: The expected output is https://gist.github.com/wdberkeley/e49b582f98f5d4d35b35bb3919c62c70 . But I found it easier to append the location info in the master summary since the location info is retrieved from the master. Waiting for opinions. -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 1 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 20:28:41 + Gerrit-HasComments: No
[kudu-CR] [location awareness] Add location info in ksck report
Fengling Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11422 Change subject: [location_awareness] Add location info in ksck report .. [location_awareness] Add location info in ksck report This patch adds two additional tablet server location tables into the ksck report. Before: Master Summary UUID | Address | Status --+-+- 44c3844d3a564b789743398c1f2d91c4 | 127.0.0.1:52748 | HEALTHY 6789594661bd4b88bd1aceaf346db370 | 127.0.0.1:52747 | HEALTHY df7badcf5c38444c8cf724e24be218d2 | 127.0.0.1:52746 | HEALTHY ... After: Master Summary UUID | Address | Status --+-+- 44c3844d3a564b789743398c1f2d91c4 | 127.0.0.1:52748 | HEALTHY 6789594661bd4b88bd1aceaf346db370 | 127.0.0.1:52747 | HEALTHY df7badcf5c38444c8cf724e24be218d2 | 127.0.0.1:52746 | HEALTHY Tablet Server UUID| Location --+--- fc18b4bdb2ae4dd0a5717e8fe1f1de69 | 0f25d1891fce48789e06fdec493fb90e | ad868d782dc743369d96a9e958811f81 | c541b6ebc3a14f0f9bbee26d52545683 | /foo-bar0/BAAZ.9-quux Location| Count ---+- /foo-bar0/BAAZ.9-quux | 1 | 3 ... Change-Id: Ideff2dd4975c99a1135002624de2620fb95c --- M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h 7 files changed, 109 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/11422/1 -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 1 Gerrit-Owner: Fengling Wang
[kudu-CR] [master] fix compilation warning of DCHECK NOTNULL
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11421 ) Change subject: [master] fix compilation warning of DCHECK_NOTNULL .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I768afd0ec34f8127f66ea8ca29ec337488c5 Gerrit-Change-Number: 11421 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 20:20:44 + Gerrit-HasComments: No
[kudu-CR] [master] fix compilation warning of DCHECK NOTNULL
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11421 Change subject: [master] fix compilation warning of DCHECK_NOTNULL .. [master] fix compilation warning of DCHECK_NOTNULL Fixed compilation warning in case of RELEASE build: kudu/master/ts_descriptor.cc:103:18: warning: expression result unused [-Wunused-value] DCHECK_NOTNULL(location); ^~~~ This patch does not contain any functional changes. This is a follow-up to dcc39d53d8c36a3f4896ce4c208b855abee8da83. Change-Id: I768afd0ec34f8127f66ea8ca29ec337488c5 --- M src/kudu/master/ts_descriptor.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/11421/1 -- To view, visit http://gerrit.cloudera.org:8080/11421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I768afd0ec34f8127f66ea8ca29ec337488c5 Gerrit-Change-Number: 11421 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] HMS integration: provide Java API to override owner during table create
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11413/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11413/3//COMMIT_MSG@7 PS3, Line 7: Java A C++ client API as well? http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java File java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java@228 PS3, Line 228: setOwner Do we want to validate if this is only be called by a service users? e.g Impala? http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java@27 PS3, Line 27: provide an API to look up the table owner Do you think it is worth providing such API since table has ownership now? -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 19:04:48 + Gerrit-HasComments: Yes
[kudu-CR] Remove unnecessary friendship
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11416 ) Change subject: Remove unnecessary friendship .. Remove unnecessary friendship This commit removes unnecessary friendship and declaration in client.h. Change-Id: Ia7c5f35a8b6a6ec2de1d57bbb4f68b53aae4e700 Reviewed-on: http://gerrit.cloudera.org:8080/11416 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-by: Alexey Serbin --- M src/kudu/client/client.h 1 file changed, 0 insertions(+), 5 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11416 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia7c5f35a8b6a6ec2de1d57bbb4f68b53aae4e700 Gerrit-Change-Number: 11416 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Remove unnecessary friendship
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11416 ) Change subject: Remove unnecessary friendship .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11416 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c5f35a8b6a6ec2de1d57bbb4f68b53aae4e700 Gerrit-Change-Number: 11416 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 18:15:37 + Gerrit-HasComments: No
[kudu-CR] HMS integration: set table owner field in HMS table metadata
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11398 ) Change subject: HMS integration: set table owner field in HMS table metadata .. Patch Set 6: The LINT failure is related to the other patch. https://gerrit.cloudera.org/#/c/11415/ -- To view, visit http://gerrit.cloudera.org:8080/11398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Gerrit-Change-Number: 11398 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 11 Sep 2018 18:10:57 + Gerrit-HasComments: No
[kudu-CR] HMS integration: set table owner field in HMS table metadata
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11398 ) Change subject: HMS integration: set table owner field in HMS table metadata .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Gerrit-Change-Number: 11398 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 11 Sep 2018 18:07:43 + Gerrit-HasComments: No
[kudu-CR] Remove unnecessary friendship
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11416 ) Change subject: Remove unnecessary friendship .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11416 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c5f35a8b6a6ec2de1d57bbb4f68b53aae4e700 Gerrit-Change-Number: 11416 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 18:00:58 + Gerrit-HasComments: No
[kudu-CR] HMS integration: provide Java API to override owner during table create
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11413 ) Change subject: HMS integration: provide Java API to override owner during table create .. Patch Set 3: (2 comments) Not sure what happened to the kudu-spark build. Seems unrelated, yet all instances failed the same way. http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/build.gradle File java/kudu-client/build.gradle: http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/build.gradle@45 PS3, Line 45: testCompile libs.hiveMetastore Nit: alphabetical sort order. http://gerrit.cloudera.org:8080/#/c/11413/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java: PS3: Missing Apache header. -- To view, visit http://gerrit.cloudera.org:8080/11413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0 Gerrit-Change-Number: 11413 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 17:52:05 + Gerrit-HasComments: Yes
[kudu-CR] HMS integration: set table owner field in HMS table metadata
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11398 ) Change subject: HMS integration: set table owner field in HMS table metadata .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a25aa0bb52bdd28df28a078fe91f55db9e29482 Gerrit-Change-Number: 11398 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 11 Sep 2018 17:46:19 + Gerrit-HasComments: No
[kudu-CR] move KUDU NO EXPORT attribute on methods
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11415 ) Change subject: move KUDU_NO_EXPORT attribute on methods .. Patch Set 3: Code-Review+2 LINT failure is legit. -- To view, visit http://gerrit.cloudera.org:8080/11415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0ebdf97263065e14e7a43cf9fcfa7cb26953f57 Gerrit-Change-Number: 11415 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 11 Sep 2018 17:45:45 + Gerrit-HasComments: No
[kudu-CR] [docs] Add basic advice on setting block cache size
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11420 Change subject: [docs] Add basic advice on setting block cache size .. [docs] Add basic advice on setting block cache size This adds a short section to the troubleshooting guide about improving the block cache size. It's fuzzy since the effectiveness of the cache and the efficacy of enlarging it are so workload dependent (e.g. consider a workload doing full table scans vs. one mostly re-scanning a small range checking for updates), but I tried to provide a starting point for users to evaluate their cache size since we've totally lacked any advice on that up to this point. I also added information about the change due to release in 1.8 that servers won't start when the block cache capacity is set too large relative to the memory limit. Change-Id: Idc7411c38b6fcc8694509ec89c32e2fe74e6c0db --- M docs/troubleshooting.adoc 1 file changed, 72 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/11420/1 -- To view, visit http://gerrit.cloudera.org:8080/11420 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idc7411c38b6fcc8694509ec89c32e2fe74e6c0db Gerrit-Change-Number: 11420 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] WIP: Breakout the Java MiniKuduCluster
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11418 Change subject: WIP: Breakout the Java MiniKuduCluster .. WIP: Breakout the Java MiniKuduCluster This change will simplify using the Java MiniKuduCluster outside of the Kudu project itself. This makes integration testing with Kudu more straightforward by providing a common base test class for testing. Additionally some of the test utilities are broken out to simplify inter-module dependencies. Follow on work could include more client setup and testing utilities. The patch: - Breaks out the compile protobuf protocol classes into its own module/jar because they are used directly in the MiniKuduCluster - Breaks out the MiniKuduCluster and a few supporting classes into its own module/jar. - Adjusts the binary locating logic to be less fragile and externally useful. - Adjust the Maven and Gradle build to define the `kuduBinDir` system property. This also fixes the issue where Maven tests would fail to locate the binary because they were using jars installed into the local repository. - Removed usage of Guava HostAndPort class. It is marked as Beta and because we shade and relocate Guava it should be privately consumed only. - Marks the InterfaceAudience and InterfaceStability of testing utilities. - Removed direct usage of the MiniKuduCluster in our tests in favor of the KuduClusterTest base testing methods. TODO: - Documentation (readme and dev docs) Change-Id: I4f39bc012a19fea5a57e18bc12192e1ea859feba --- M java/README.adoc M java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java M java/gradle/tests.gradle M java/kudu-backup/build.gradle M java/kudu-backup/pom.xml M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client-tools/build.gradle M java/kudu-client-tools/pom.xml M java/kudu-client/build.gradle M java/kudu-client/pom.xml M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquireOpen.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.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/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestHandleTooBusy.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.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/TestMasterFailover.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java D java/kudu-client/src/test/java/org/apache/kudu/client/TestTestUtils.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java M java/kudu-client/src/test/java/org/apache/kudu/util/CapturingLogAppender.java M java/kudu-client/src/test/java/org/apache/kudu/util/ClientTestUtil.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestAsyncUtil.java M java/kudu-flume-sink/build.gradle M java/kudu-flume-sink/pom.xml M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java M java/kudu-hive/build.gradle M java/kudu-hive/pom.xml M java/kudu-mapreduce/build.gradle M java/kudu-mapreduce/pom.xml A java/kudu-protocol/build.gradle A java/kudu-protocol/pom.xml M java/kudu-spark-tools/build.gradle M java/kudu-spark-tools/pom.xml M java/kudu-spark/build.gradle M java/kudu-spark/pom.xml M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala A java/kudu-test-utils/build.gradle A java/kudu-test-utils/pom.xml R java/kudu-test-utils/src/main/java/org/apache/kudu/junit/AssertHelpers.java R java/kudu-test-utils/src/main/java/org/apache/kudu/junit/RetryRule.java A java/kudu-test-utils/src/main/java/org/apache/kudu/test/RandomUtils.java A java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryLocator.java A java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduClusterTest.java R java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java R java/kudu-test-utils/src/main/java/org/apache/kudu/test/network/FakeDNS.java M java/pom.xml M java/settings
[kudu-CR] KUDU-2566: Enhance rowset tree pruning and stop copying strings
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11381 ) Change subject: KUDU-2566: Enhance rowset tree pruning and stop copying strings .. KUDU-2566: Enhance rowset tree pruning and stop copying strings Three improvements: 1) Support open-ended intervals: Previously, the rowset tree could only compute overlap with a closed and bounded interval. This changeset adds the ability to find overlap for unbounded intervals as well. As a result, tablet iterators with a single primary key bound are more efficient because they do not iterate over rowsets that don't overlap with the key bound. 2) End up fetching one more rowset for the upper bound which is exclusive: This changeset also adds the ability to query the rowset tree using an exclusive upper bound, whereas previously only inclusive intervals were supported. This makes scans more efficient since primary key upper bounds are passed as exclusive bounds, so now rowsets that overlap only in the upper bound are excluded. 3) Perf improvement: Using raw slices instead of copying to strings while querying. The copying from slices to string is discarded, which could help to increase the query performance. Change-Id: I0e34b4169f93f3519f694c9bf86ca5295c318e79 Reviewed-on: http://gerrit.cloudera.org:8080/11381 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley --- M src/kudu/tablet/rowset_tree-test.cc M src/kudu/tablet/rowset_tree.cc M src/kudu/tablet/rowset_tree.h M src/kudu/tablet/tablet.cc M src/kudu/util/interval_tree-inl.h M src/kudu/util/interval_tree-test.cc M src/kudu/util/interval_tree.h 7 files changed, 352 insertions(+), 85 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11381 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0e34b4169f93f3519f694c9bf86ca5295c318e79 Gerrit-Change-Number: 11381 Gerrit-PatchSet: 9 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2566: Enhance rowset tree pruning and stop copying strings
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11381 ) Change subject: KUDU-2566: Enhance rowset tree pruning and stop copying strings .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11381 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e34b4169f93f3519f694c9bf86ca5295c318e79 Gerrit-Change-Number: 11381 Gerrit-PatchSet: 8 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 16:24:08 + Gerrit-HasComments: No
[kudu-CR] Add helper macro for tool invocations
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11411 to look at the new patch set (#3). Change subject: Add helper macro for tool invocations .. Add helper macro for tool invocations Using `ASSERT_OK` to test the results of the `kudu` tool is normal, but it results in lousy test failure output: ../../src/kudu/tools/kudu-admin-test.cc:235: Failure Failed Bad status: Runtime error: /Users/wdberkeley/src/kudu/build/debug/bin/kudu: process exited with non-zero status 1 This adds a new macro, `ASSERT_TOOL_OK`, that also logs the stdout and stderr of a `kudu` tool invocation: ../../src/kudu/tools/kudu-admin-test.cc:235: Failure Failed Runtime error: /Users/wdberkeley/src/kudu/build/debug/bin/kudu: process exited with non-zero status 1 stdout: stderr: W0910 12:39:07.483736 2830984064 flags.cc:406] Enabled unsafe flag: --never_fsync=true Invalid argument: Unrecognized peer type: FOOVOTER Change-Id: I7ffd357d79982ee5c93f8d3c7cfd7cc1f0863f07 --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 47 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/11411/3 -- To view, visit http://gerrit.cloudera.org:8080/11411 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ffd357d79982ee5c93f8d3c7cfd7cc1f0863f07 Gerrit-Change-Number: 11411 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR](gh-pages) [blog] Data Pipelines Simplified with Kudu
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11417 ) Change subject: [blog] Data Pipelines Simplified with Kudu .. Patch Set 2: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Gerrit-Change-Number: 11417 Gerrit-PatchSet: 2 Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 15:44:30 + Gerrit-HasComments: No
[kudu-CR](gh-pages) [blog] Data Pipelines Simplified with Kudu
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11417 ) Change subject: [blog] Data Pipelines Simplified with Kudu .. [blog] Data Pipelines Simplified with Kudu Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Reviewed-on: http://gerrit.cloudera.org:8080/11417 Reviewed-by: Attila Bukor Tested-by: Attila Bukor --- A _posts/2018-09-11-simplified-pipelines-with-kudu.md 1 file changed, 44 insertions(+), 0 deletions(-) Approvals: Attila Bukor: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: merged Gerrit-Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Gerrit-Change-Number: 11417 Gerrit-PatchSet: 3 Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] build: add option to ignore test failures
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11399 ) Change subject: build: add option to ignore test failures .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7 Gerrit-Change-Number: 11399 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 11 Sep 2018 14:06:53 + Gerrit-HasComments: No
[kudu-CR] KUDU-2012 Kudu Flume sink auth support
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11334 ) Change subject: KUDU-2012 Kudu Flume sink auth support .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917 Gerrit-Change-Number: 11334 Gerrit-PatchSet: 3 Gerrit-Owner: Ferenc Szabo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Ferenc Szabo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 11 Sep 2018 13:55:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-1882 Configuration improvements for Flume Kudu Sink regexp operations producer.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11391 ) Change subject: KUDU-1882 Configuration improvements for Flume Kudu Sink regexp operations producer. .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b352b27583ae9eee6cdbe28a768362bea36e00f Gerrit-Change-Number: 11391 Gerrit-PatchSet: 4 Gerrit-Owner: Ferenc Szabo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 11 Sep 2018 13:48:41 + Gerrit-HasComments: No
[kudu-CR](gh-pages) [blog] Data Pipelines Simplified with Kudu
Jordan Birdsell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11417 ) Change subject: [blog] Data Pipelines Simplified with Kudu .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md File _posts/2018-09-11-simplified-pipelines-with-kudu.md: http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md@38 PS1, Line 38: ficant > I think this word is hyphenated, or maybe it should be replaced by some oth Done http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md@42 PS1, Line 42: e of > caches ? Done http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md@42 PS1, Line 42: changes, and re > updates/deletes ? Maybe, rephrase it to '... of merging in changes, and .. Done -- To view, visit http://gerrit.cloudera.org:8080/11417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Gerrit-Change-Number: 11417 Gerrit-PatchSet: 2 Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 13:25:13 + Gerrit-HasComments: Yes
[kudu-CR](gh-pages) [blog] Data Pipelines Simplified with Kudu
Hello Will Berkeley, Alexey Serbin, Attila Bukor, Todd Lipcon, Brock Noland, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11417 to look at the new patch set (#2). Change subject: [blog] Data Pipelines Simplified with Kudu .. [blog] Data Pipelines Simplified with Kudu Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 --- A _posts/2018-09-11-simplified-pipelines-with-kudu.md 1 file changed, 44 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/11417/2 -- To view, visit http://gerrit.cloudera.org:8080/11417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newpatchset Gerrit-Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Gerrit-Change-Number: 11417 Gerrit-PatchSet: 2 Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1882 Configuration improvements for Flume Kudu Sink regexp operations producer.
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11391 to look at the new patch set (#4). Change subject: KUDU-1882 Configuration improvements for Flume Kudu Sink regexp operations producer. .. KUDU-1882 Configuration improvements for Flume Kudu Sink regexp operations producer. Adding new properties for the different parsing error policies. Deprecating the old ones. Default value constants are not removed as they are public variables on a public class and it would be an API change. Adding new test class to test the configuration and behaviour. Change-Id: I0b352b27583ae9eee6cdbe28a768362bea36e00f --- M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java 2 files changed, 389 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/11391/4 -- To view, visit http://gerrit.cloudera.org:8080/11391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0b352b27583ae9eee6cdbe28a768362bea36e00f Gerrit-Change-Number: 11391 Gerrit-PatchSet: 4 Gerrit-Owner: Ferenc Szabo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2012 Kudu Flume sink auth support
Hello Attila Bukor, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11334 to look at the new patch set (#3). Change subject: KUDU-2012 Kudu Flume sink auth support .. KUDU-2012 Kudu Flume sink auth support Adding FlumeAuthenticator to KuduSink and creating KuduClient inside a priviligedExecuter action. Added an extra step to the mini cluster to create a keyTab for the client used for testing. Added automated test with short KDC ticket lifetime to test reacquiring. Manual testing was done on a secure cluster as well. Change-Id: I11b5f08802883afa178d346af48d3bcd15281917 --- M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.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 A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/security/test/mini_kdc.cc M src/kudu/security/test/mini_kdc.h 12 files changed, 364 insertions(+), 253 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/11334/3 -- To view, visit http://gerrit.cloudera.org:8080/11334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917 Gerrit-Change-Number: 11334 Gerrit-PatchSet: 3 Gerrit-Owner: Ferenc Szabo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Ferenc Szabo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](gh-pages) [blog] Data Pipelines Simplified with Kudu
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11417 ) Change subject: [blog] Data Pipelines Simplified with Kudu .. Patch Set 1: Verified+1 (1 comment) verified that it's rendering correctly http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md File _posts/2018-09-11-simplified-pipelines-with-kudu.md: http://gerrit.cloudera.org:8080/#/c/11417/1/_posts/2018-09-11-simplified-pipelines-with-kudu.md@38 PS1, Line 38: value add I think this word is hyphenated, or maybe it should be replaced by some other word like 'enhanced' to sound less a sales pitch-y. -- To view, visit http://gerrit.cloudera.org:8080/11417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I222d2462da86c3aad3fa9afd71f686faaa9aa025 Gerrit-Change-Number: 11417 Gerrit-PatchSet: 1 Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Brock Noland Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 11 Sep 2018 08:26:43 + Gerrit-HasComments: Yes
