[kudu-CR] Create ConsensusMetadataManager
Mike Percy has posted comments on this change. Change subject: Create ConsensusMetadataManager .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc File src/kudu/consensus/consensus_meta_manager.cc: Line 44: // Ensure we don't interleave with other ops on this tablet. > Isn't interleaving on Create() already a disallowed operation? > Isn't interleaving on Create() already a disallowed operation? Well... ConsensusMetadata::Create() will fail for the 2nd caller, if that's what you mean. > Even with this locking, if you had interleaved with another Create or Load, > you'd just crash on the InsertOrDie, right? No, I don't think so. The 2nd thread would have to wait until the ScopedCleanup clears op_in_progress_[tablet_id] and then it can enter the main block. If, at that time, the cmeta exists, it will get an error back from ConsensusMetadata::Create(). > And if you are concurrent with a Delete(), then you wouldn't crash but still > seems like invalid usage of the API. I think this does the right thing with multiple concurrent ops, such as 2 concurrent calls Create() and one to Delete(). However, all of that stuff should probably not be going on at once. > What are the legal interleavings that actually could/should happen in > practice? Good question. If we assume Create() and Delete() calls are externally mutexed for a given tablet then we can make it simpler. In the current Kudu code base, that would be a safe assumption due to the TSTabletManager "tablet state transition" reservation mechanism. > Maybe something simpler is possible here, eg allowing concurrent Load() to > potentially do extra work and "first loader wins" in the case of a race? I think concurrent Load() would be straightforward to simplify as long as only one ConsensusMetadata instance survives at the end. I guess the question is whether adding additional constraints on the API s.t. concurrent Create() and Delete() are illegal will come back to bite us later since it may be hard to detect when it occurs. But maybe it's worth getting rid of this additional complexity. -- To view, visit http://gerrit.cloudera.org:8080/7191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Make ConsensusMetadata thread-safe
Hello David Ribeiro Alves, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6958 to look at the new patch set (#8). Change subject: Make ConsensusMetadata thread-safe .. Make ConsensusMetadata thread-safe This patch simply adds locks around all accessors and makes ConsensusMetadata refcounted. This enables ConsensusMetadata to be safely used outside of the RaftConsensus class (with certain caveats). This is needed to implement tombstoned voting in a follow-up patch. Also, add some helper methods to ConsensusMetadata to avoid making excessive copies of the RaftConfigPB in various cases: bool IsVoterInConfig(const std::string& uuid, RaftConfigState type); bool IsMemberInConfig(const std::string& uuid, RaftConfigState type); int CountVotersInConfig(RaftConfigState type); int64_t GetConfigOpIdIndex(RaftConfigState type); Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 --- M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/consensus_meta.cc M src/kudu/consensus/consensus_meta.h M src/kudu/consensus/quorum_util.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc 18 files changed, 335 insertions(+), 147 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6958/8 -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Make ConsensusMetadata thread-safe
Hello David Ribeiro Alves, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6958 to look at the new patch set (#7). Change subject: Make ConsensusMetadata thread-safe .. Make ConsensusMetadata thread-safe This patch simply adds locks around all accessors and makes ConsensusMetadata refcounted. This enables ConsensusMetadata to be safely used outside of the RaftConsensus class (with certain caveats). This is needed to implement tombstoned voting in a follow-up patch. Also, add some helper methods to ConsensusMetadata to avoid making excessive copies of the RaftConfigPB in various cases: bool IsVoterInConfig(const std::string& uuid, RaftConfigState type); bool IsMemberInConfig(const std::string& uuid, RaftConfigState type); int CountVotersInConfig(RaftConfigState type); int64_t GetConfigOpIdIndex(RaftConfigState type); Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 --- M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/consensus_meta.cc M src/kudu/consensus/consensus_meta.h M src/kudu/consensus/quorum_util.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc 18 files changed, 338 insertions(+), 147 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/6958/7 -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Make ConsensusMetadata thread-safe
Mike Percy has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/6958/6/src/kudu/consensus/consensus_meta.h File src/kudu/consensus/consensus_meta.h: Line 99: RaftConfigPB committed_config() const; > If this returns a copy of a heavy-weight PB it should be renamed to Committ OK, I went through and removed all of the hot-path copies that I found. Also renamed the copying methods to InitialCaps convention to make them more obviously copying. Line 106: RaftConfigPB pending_config() const; > same Done Line 114: RaftConfigPB active_config() const; > same Done -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired
Alexey Serbin has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 668: TO_BE_RENEGOTIATED, // The connection negotiation has failed and has to be be re-negotiated. > Yep, ideally it would be nice to have just DISCONNECTED state and recycle d An update: after looking at how to avoid re-connecting a connection, I came into conclusion this might be too cumbersome. If not allowing the TO_BE_RENEGOTIATED state and further re-connecting, it would require calling status errbacks for all the enqueued RPC messages after a new authentication token is acquired (we cannot silently drop the connection which is in TO_BE_RENEGOTIATED phase because there are enqueued messages). So, this would lead the upper-level code to retry corresponding RPCs, one-at-a-time (basically, that's about finding or establishing a new connection via the connection cache and then creating new copies of the messages in the wire format to be put into the queuedMessages member of other Connection object). Do we want to go that way? -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7250 to look at the new patch set (#3). Change subject: [java] KUDU-2013 re-acquire authn token if expired .. [java] KUDU-2013 re-acquire authn token if expired This patch introduces automatic authn token re-acquisition when the current authn token expires. The client automatically retries the RPC that hits the token expiration error (the error to re-try is seen as FATAL_INVALID_AUTHENTICATION_TOKEN sent by the server during connection negotiation). Added a few of tests to exercise the new retry logic for automatic token re-acquisition in case of master-only operations, a bare minimum workload scenario, and one special case of a connection to the master opened with secondary credentials. Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquireOpen.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java 9 files changed, 656 insertions(+), 66 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/7250/3 -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Use dynamic bind address for internal + external mini clusters
Hello Adar Dembo, Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7274 to look at the new patch set (#6). Change subject: Use dynamic bind address for internal + external mini clusters .. Use dynamic bind address for internal + external mini clusters This technique helps avoid port conflicts. It assigns a unique loopback IP, based on pid and per-cluster numbering, to every InternalMiniCluster or ExternalMiniCluster component (including masters and tablet servers). Changes: 1. Apply the "dynamic bind address" technique to internal mini clusters (previously it was only used for external mini clusters). 2. Apply it to masters (previously it was only used for tablet servers). I will post a follow-up patch to remove some of the CMakeLists.txt limitations related to port conflicts, although we may need to put some back due to lack of macOS support for unique loopback addresses. I had to refactor a few host-related APIs to do this in a relatively non-hacky manner. All tests still pass (and should more often). Additional changes: * Move BindMode to MiniCluster base class. * Fix a few brittle tests that started failing because of this change. Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0 --- M src/kudu/client/client-test.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/internal_mini_cluster.cc M src/kudu/integration-tests/internal_mini_cluster.h M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/master_replication-itest.cc A src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/master/master-test.cc M src/kudu/master/mini_master.cc M src/kudu/master/mini_master.h M src/kudu/master/sys_catalog-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/mini_tablet_server.h M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/ts_tablet_manager-test.cc 20 files changed, 316 insertions(+), 192 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/7274/6 -- To view, visit http://gerrit.cloudera.org:8080/7274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] log block manager: do not fail block creation if truncate fails
Adar Dembo has posted comments on this change. Change subject: log block manager: do not fail block creation if truncate fails .. Patch Set 1: > seems reasonable. did you hit this in real life somehow? Nah, noticed it while reviewing Hao's patch. -- To view, visit http://gerrit.cloudera.org:8080/7312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rename MiniClusterBase to MiniCluster
Mike Percy has submitted this change and it was merged. Change subject: Rename MiniClusterBase to MiniCluster .. Rename MiniClusterBase to MiniCluster Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36 Reviewed-on: http://gerrit.cloudera.org:8080/7273 Reviewed-by: Todd LipconTested-by: Kudu Jenkins --- M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/internal_mini_cluster.h R src/kudu/integration-tests/mini_cluster.h M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h 5 files changed, 12 insertions(+), 12 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired
Alexey Serbin has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 272: RpcHeader.ErrorStatusPB.RpcErrorCodePB.FATAL_INVALID_AUTHENTICATION_TOKEN)) { > Could we get in a weird state here where this connection is actually a mast An update -- the new can be created, but it's not outside of the ConnectionCache. A new connection will be scheduled by tokenReacquirer.reacquire(), but it will be created by using the cache when executing ConnectToCluster.run(). -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] log block manager: do not fail block creation if truncate fails
Todd Lipcon has posted comments on this change. Change subject: log block manager: do not fail block creation if truncate fails .. Patch Set 1: Code-Review+2 seems reasonable. did you hit this in real life somehow? -- To view, visit http://gerrit.cloudera.org:8080/7312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Create ConsensusMetadataManager
Todd Lipcon has posted comments on this change. Change subject: Create ConsensusMetadataManager .. Patch Set 7: (1 comment) just looked at the manager impl for starters http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc File src/kudu/consensus/consensus_meta_manager.cc: Line 44: // Ensure we don't interleave with other ops on this tablet. Isn't interleaving on Create() already a disallowed operation? Even with this locking, if you had interleaved with another Create or Load, you'd just crash on the InsertOrDie, right? And if you are concurrent with a Delete(), then you wouldn't crash but still seems like invalid usage of the API. What are the legal interleavings that actually could/should happen in practice? Maybe something simpler is possible here, eg allowing concurrent Load() to potentially do extra work and "first loader wins" in the case of a race? -- To view, visit http://gerrit.cloudera.org:8080/7191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] log block manager: do not fail block creation if truncate fails
Hello Anonymous Coward #314, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7312 to review the following change. Change subject: log block manager: do not fail block creation if truncate fails .. log block manager: do not fail block creation if truncate fails RETURN_NOT_OK here means WritableBlock::Close will fail unnecessarily. I don't _think_ that could lead to on-disk corruption (the container has to be full, so no blocks will be added to it after this one), but it'll cause some inconsistency in metrics. Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd --- M src/kudu/fs/log_block_manager.cc 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/7312/1 -- To view, visit http://gerrit.cloudera.org:8080/7312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I96ce55eb1cd3a13f7cbfaf1bde7755c4d91a7dbd Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Rename MiniClusterBase to MiniCluster
Todd Lipcon has posted comments on this change. Change subject: Rename MiniClusterBase to MiniCluster .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP KUDU-1943: Add BlockTransaction to Block Manager
Adar Dembo has posted comments on this change. Change subject: WIP KUDU-1943: Add BlockTransaction to Block Manager .. Patch Set 8: (33 comments) I reviewed everything outside the LBM, and then began reviewing the LBM changes. I stopped because I saw two pretty serious issues that we need to think about: Container thread safety. WritableBlocks aren't supposed to be shared by multiple threads, which meant that containers were also single threaded as blocks were added to them. Now that's no longer the case, and this introduces some synchronization issues when accessing container state. I think they can be addressed with the introduction of a per-container lock (and careful thinking about the right order in which to acquire the container lock and the LBM lock). Consistent container in-memory accounting when operations fail. KUDU-1793 was a serious issue, and to fix it, I tried to guarantee that container in-memory state is only updated after the "point of no return" (that is, when a block was guaranteed to be successfully written). This change explicitly does away with that due to how Finalize() works. There's no way around this, but it means we need to think very carefully about what happens to container in-memory state in the event of a failure after a block is finalized. http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/cfile/bloomfile.h File src/kudu/cfile/bloomfile.h: PS8, Line 47: blocks It's just one block, right? Why 'blocks'? http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: Line 283: block_->Finalize(); RETURN_NOT_OK. Line 289: block_->Finalize(); RETURN_NOT_OK. http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/cfile/cfile_writer.h File src/kudu/cfile/cfile_writer.h: PS8, Line 109: blocks Just one block. http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: Line 396: Nit: why add an empty line here and not in the WriterThread/ReaderThread functions, which do the same thing? Or was this a mistake? http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: Line 995: ASSERT_OK(transaction.CommitWrites()); Before committing, how about trying to open all the blocks in block_ids and verifying that they can't be found? http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: Line 143: > yes, there is some cases the block is flushed but not finalized, you can ca I think FLAGS_cfile_do_on_finish has mostly outlived its usefulness (it was used for performance experiments early on). So let's merge the "block is finalized" and "block is flushing" states. This means you should make two gflags changes: - Change cfile_do_on_finish to something like cfile_close_block_on_finish. It'd be a boolean with a default value of false. - Add something like block_manager_flush_on_finalize. It'd be a boolean with a default value of true. http://gerrit.cloudera.org:8080/#/c/7207/8/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: PS8, Line 73: For LogWritableBlock, container : //is released for further usage to avoid unnecessarily creating new containers. Generalize this example so that it doesn't refer to implementation details. PS8, Line 76: For LogWritableBlock, : //it also finishes the blocks belong to the same container together to : //reduce fsync() usage. Generalize this so that it doesn't refer to a particular implementation. Line 91: // The block is finalized as it has been fully written. How about: "There is some dirty data in the block and no more may be written." Line 146: // Finalize a fully written block. This doesn't explain what restrictions the FINALIZED state places on the WritableBlock. Can I keep writing to it? Can I call FlushDataAsync() on it? Is the block data now durably on disk? Use the comments in FlushDataAsync()/Append()/Close() for inspiration. Line 147: virtual Status Finalize() = 0; Nit: since this has a real effect on the state of the block, please move it to be just above BytesAppended(). That way the "simple accessor" functions are grouped together, and the "complicated" functions likewise. Line 149: virtual int64_t offset() const { return 0; } I don't think this belongs in WritableBlock. It doesn't make any sense to anyone outside a block manager implementation, because it's not even clear what it's an offset into (a file? a container? something else?). Why do you need it? Line 285: // Group a set of blocks writes together in a transaction. The names and comments here conflate "write" and "create". Please choose one and use it consistently. Line 320: std::vectorcreated_blocks_; Since we're now writing C++11
[kudu-CR] [java] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java] separating Connection .. Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG Commit Message: Line 11: The updated TabletClient has been renamed into RpcHelper. > Wasn't it renamed to RpcProxy? Woops, right. Done. http://gerrit.cloudera.org:8080/#/c/7146/15/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java: Line 464: try { > Shouldn't this still be a while loop? If it throws interrupted I think it We discussed this over the kudu-general Slack channel and decided go keep the internal while(true) loop and add also update how the interrupt flag is set on the thread. -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#16). Change subject: [java] separating Connection .. [java] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The updated TabletClient has been renamed into RpcProxy. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.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/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,401 insertions(+), 1,264 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/16 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Make ConsensusMetadata thread-safe
Todd Lipcon has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/6958/6/src/kudu/consensus/consensus_meta.h File src/kudu/consensus/consensus_meta.h: Line 99: RaftConfigPB committed_config() const; If this returns a copy of a heavy-weight PB it should be renamed to CommittedConfig. More broadly, I'm concerned that this may actually be a perf issue. For example see e14b82496da992c40af3fbf2c24a97f38b1d96cc in which I changed to using 'cmeta_->active_config()' specifically to avoid reallocating/copying the RaftConfigPB. I think this will regress that change and potentially introduce other problematic cases. Not sure if that's the only case of a hot-path call into this but the PBs are pretty heavy-weight to copy on every UpdateConsensus call. (I think it's fine if they get copied during voting/election/etc since those are rare) Line 106: RaftConfigPB pending_config() const; same Line 114: RaftConfigPB active_config() const; same -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Make ConsensusMetadata thread-safe
Todd Lipcon has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe .. Patch Set 6: Code-Review-1 -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rename MiniClusterOptions to InternalMiniClusterOptions
Alexey Serbin has posted comments on this change. Change subject: Rename MiniClusterOptions to InternalMiniClusterOptions .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7287 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bb1a87d916fdc0a248b5b6174306cc544333685 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Rename MiniCluster to InternalMiniCluster
Mike Percy has submitted this change and it was merged. Change subject: Rename MiniCluster to InternalMiniCluster .. Rename MiniCluster to InternalMiniCluster Since we have ExternalMiniCluster, using InternalMiniCluster as a class name for the threaded version is consistent. This also opens up the possibility of using MiniCluster as the base class name in a follow-up commit. Change-Id: I7bd326a7a46f039e18c47b8e23ee1427ccf281be Reviewed-on: http://gerrit.cloudera.org:8080/7272 Reviewed-by: Adar DemboTested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/client/client-test.cc M src/kudu/client/predicate-test.cc M src/kudu/client/scan_token-test.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/consistency-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/delete_tablet-itest.cc M src/kudu/integration-tests/external_mini_cluster.h M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc R src/kudu/integration-tests/internal_mini_cluster-itest-base.h R src/kudu/integration-tests/internal_mini_cluster.cc R src/kudu/integration-tests/internal_mini_cluster.h M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/mini_cluster_base.h M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/security-unknown-tsk-itest.cc M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/kudu-tool-test.cc 29 files changed, 114 insertions(+), 121 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7bd326a7a46f039e18c47b8e23ee1427ccf281be Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] server: consolidate tablet prepare pools
Todd Lipcon has submitted this change and it was merged. Change subject: server: consolidate tablet prepare pools .. server: consolidate tablet prepare pools Using the new serial token functionality available in ThreadPool, we can now safely consolidate all per-tablet prepare pools into a single server-wide pool. Each replica allocates a token with which to submit to the prepare pool, attaching the existing tablet-specific metrics to it. The prepare pool is configured with no upper bound on the number of running threads because prepare tasks can block on row/schema locks. Change-Id: Ic393f739cc9c1b267142b20e04a1aaa5aed97cb0 Reviewed-on: http://gerrit.cloudera.org:8080/7150 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/integration-tests/delete_tablet-itest.cc M src/kudu/kserver/kserver.cc M src/kudu/kserver/kserver.h M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tablet/transactions/transaction_driver.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc 11 files changed, 59 insertions(+), 40 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic393f739cc9c1b267142b20e04a1aaa5aed97cb0 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] server: consolidate tablet prepare pools
Todd Lipcon has posted comments on this change. Change subject: server: consolidate tablet prepare pools .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic393f739cc9c1b267142b20e04a1aaa5aed97cb0 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] threadpool: per-token metrics
Todd Lipcon has submitted this change and it was merged. Change subject: threadpool: per-token metrics .. threadpool: per-token metrics This patch makes thread pool metrics configurable on a token by token basis. The idea is simple: if a pool is shared by multiple entities in a server (i.e. one token per tablet), pool operation metrics should reflect those entities rather than be server-wide. Change-Id: I29e3f78d0c033ff24f675cd84e7f7d65bf19954b Reviewed-on: http://gerrit.cloudera.org:8080/7149 Tested-by: Adar DemboReviewed-by: Todd Lipcon --- M src/kudu/kserver/kserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/util/threadpool-test.cc M src/kudu/util/threadpool.cc M src/kudu/util/threadpool.h 5 files changed, 119 insertions(+), 75 deletions(-) Approvals: Adar Dembo: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I29e3f78d0c033ff24f675cd84e7f7d65bf19954b Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] threadpool: per-token metrics
Todd Lipcon has posted comments on this change. Change subject: threadpool: per-token metrics .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I29e3f78d0c033ff24f675cd84e7f7d65bf19954b Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] consensus: consolidate Raft thread pools
Todd Lipcon has submitted this change and it was merged. Change subject: consensus: consolidate Raft thread pools .. consensus: consolidate Raft thread pools This patch consolidates the two thread pools used in Raft consensus: the observer and "Raft" (for lack of a better name) pools. The former runs tasks for infrequent events such as term and config changes while the latter is used for periodic events such as processing heartbeat responses. Both per-replica pools are consolidated into a single per-server pool. Tokens are used to ensure that tasks belonging to a single replica are logically grouped together. One token is shared by RaftConsensus (primary) and PeerManager (secondary); this mirrors the existing sharing behavior and is safe because token operations are thread-safe. A second serial token is dedicated for the observer submissions, so that its (potentially) long-running tasks don't starve any periodic Raft events. The non-default max_threads may come as a surprise, but David showed me how tasks submitted to either pool may sometimes lead to an fsync (mostly via cmeta flushes). As such, it isn't safe to use the default num_cpus upper bound, as that may cause such IO-based tasks to block other CPU-only tasks (or worse, periodic tasks like heartbeat response processing). What per-replica threads are not addressed by this patch? - Failure detection thread: a threadpool isn't the right model for these. Something like a hash wheel timer would be ideal. - Prepare thread pool (max_threads=1): these could be consolidated too, but their metrics are per-replica, and tokens don't (yet) support that. Meaning, if they were consolidated now, the metrics would also consolidate and would be less useful as a result. Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54 Reviewed-on: http://gerrit.cloudera.org:8080/6946 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_peers.h M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/peer_manager.cc M src/kudu/consensus/peer_manager.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/kserver/kserver.cc M src/kudu/kserver/kserver.h M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc 19 files changed, 193 insertions(+), 123 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] consensus: consolidate Raft thread pools
Todd Lipcon has posted comments on this change. Change subject: consensus: consolidate Raft thread pools .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java] separating Connection
Dan Burkert has posted comments on this change. Change subject: [java] separating Connection .. Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG Commit Message: Line 11: The updated TabletClient has been renamed into RpcHelper. > Done Wasn't it renamed to RpcProxy? http://gerrit.cloudera.org:8080/#/c/7146/15/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java: Line 464: try { Shouldn't this still be a while loop? If it throws interrupted I think it means it didn't fully wait for the process to finish. -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] threadpool: new test for pools with no max threads
Todd Lipcon has posted comments on this change. Change subject: threadpool: new test for pools with no max_threads .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37d0bd0a05098bcc1a81ec9f1ac33e74e98781e4 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] threadpool: token-based task sequencing
Todd Lipcon has submitted this change and it was merged. Change subject: threadpool: token-based task sequencing .. threadpool: token-based task sequencing This patch adds task sequencing to util/threadpool. Task sequencing allows M contexts to share a single pool with N threads while also ensuring that the pool executes tasks belonging to each context in order. Previously this was only achievable via "one singleton pool per context", which grossly inflated the total number of threads and overall state. The new logic is implemented in ThreadPoolToken. Tasks submitted via tokens are logically grouped together. They may also be sequenced, depending on the token's execution mode. Tokens expose Wait() variants which will block on the task group. Tokens may also be shut down before being destroyed, which has semantics equivalent to shutting down the pool itself (i.e. tasks may no longer be submitted with these tokens). Some notes: - I evaluated two other implementations. In one, tokens had an implicit lifecycle that was automatically managed by the threadpool. While simpler for clients, the threadpool was more inefficient with more allocations and deallocations in each submission. In the other, token submission was built using regular task submission. This afforded a certain separation between the "core" of the threadpool and the token logic, but it complicated locking and tracking of queued tasks. - I tried to keep submission (whether token-based or tokenless) fast. Just the change from std::list to std::deque for the main queue ought to improve performance of tokenless submissions. The next bottleneck is likely to be lock contention on the global threadpool lock. Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70 Reviewed-on: http://gerrit.cloudera.org:8080/6874 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/util/debug-util.h M src/kudu/util/threadpool-test.cc M src/kudu/util/threadpool.cc M src/kudu/util/threadpool.h 4 files changed, 991 insertions(+), 91 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] threadpool: new test for pools with no max threads
Todd Lipcon has submitted this change and it was merged. Change subject: threadpool: new test for pools with no max_threads .. threadpool: new test for pools with no max_threads This test ensures that a pool created with effectively no max_threads works as expected. That is: 1. Tokenless tasks should trigger the creation of a new thread. 2. Serial token-based tasks can create new threads, but only up to the number of tokens submitted against. I intend to use this "feature" to consolidate some Raft thread pools where a num_cpus upper bound may be undesirable (i.e. where tasks submitted to the pools may result in blocking IO). Change-Id: I37d0bd0a05098bcc1a81ec9f1ac33e74e98781e4 Reviewed-on: http://gerrit.cloudera.org:8080/6945 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/util/threadpool-test.cc M src/kudu/util/threadpool.h 2 files changed, 66 insertions(+), 17 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I37d0bd0a05098bcc1a81ec9f1ac33e74e98781e4 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] threadpool: token-based task sequencing
Todd Lipcon has posted comments on this change. Change subject: threadpool: token-based task sequencing .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] server: move apply pool into KuduServer
Todd Lipcon has submitted this change and it was merged. Change subject: server: move apply_pool into KuduServer .. server: move apply_pool into KuduServer The server-wide apply_pool was instantiated in two places: SysCatalogTable (for the master) and TsTabletManager (for the tserver). This commit moves it into KuduServer and unifies the instantiation. Note: the apply pool semantics have not changed. Some interesting side effects: 1. The master will now generate apply pool metrics. 2. The apply pool is shut down a little bit later in server shutdown than it was before, though I don't see any issues with this. Change-Id: Ie7ffc886269aa6531517a52fef29c4408a925aed Reviewed-on: http://gerrit.cloudera.org:8080/6984 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/kserver/kserver.cc M src/kudu/kserver/kserver.h M src/kudu/master/catalog_manager.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 8 files changed, 65 insertions(+), 59 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie7ffc886269aa6531517a52fef29c4408a925aed Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] server: move apply pool into KuduServer
Todd Lipcon has posted comments on this change. Change subject: server: move apply_pool into KuduServer .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7ffc886269aa6531517a52fef29c4408a925aed Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1863: improve overall safety of graceful server shutdown
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-1863: improve overall safety of graceful server shutdown .. KUDU-1863: improve overall safety of graceful server shutdown What exactly is a "safe" graceful shutdown? I define it like this: 1. Stop handling network traffic (i.e. RPCs and HTTP requests): a. Future incoming RPCs are responded to with an error. b. Future outgoing RPCs are immediately rejected (i.e. their callbacks run on the issuing threads with an error status). c. Queued but not yet handled incoming RPCs are responded to with an error. d. A "black hole" is set up for anyone who tries to respond to a deferred incoming RPC (i.e. an RPC that was thunked from an RPC thread and may be waiting for external stimuli). e. Outgoing RPCs awaiting a response are marked as failed and their callbacks are run on RPC threads with an error status. 2. Wait for any RPC/HTTP-related threads to finish. They may be processing an incoming RPC, processing the response from an outgoing RPC, or dealing with a failed RPC from step 2e. 3. Shut down RPC-using subsystems. Each one is responsible for cleaning up its deferred incoming RPCs. If any responses are sent, they will be black holed as per step 1d. 4. Shut down remaining server state. By ordering step 3 after steps 1 and 2, we guarantee that there are no reactor threads modifying state belonging to the subsystems that we are trying to shut down. Unfortunately, adhering to this order is very difficult today: we have to choose between steps 1a and 1b or destroying the RPC subsystem altogether, which causes problems in step 3. Moreover, it's not possible to stop handling HTTP requests without tearing down state, which leads to failures in some subsystems. So for now, we'll settle for a modified form of the above: 1. Stop accepting new RPCs. 2. Shut down RPC-using subsystems. 3. Destroy all RPC RPC state, waiting for outstanding RPC threads to finish. 4. Shut down the rest of the server. This patch implements this modified form of a safe shutdown: - During shutdown, servers first disable the acceptance of new RPCs and stop the webserver. - Servers now shut down master and tserver-specific subsystems before generic subsystems. - Shutting down generic subsystems first explicitly waits for reactor threads to finish via a new synchronous Messenger::Shutdown. The bulk of the patch is to Messenger::Shutdown and friends. While there, I also reduced lock acquisition to just critical sections (fixing KUDU-1863 in the process), extended the life of tls_context_ to Messenger destruction, and changed Shutdown to be callable with registered services. Change-Id: Ibbda29dd471e4dc1a8cb4f472cc456ccdb1e48cc Reviewed-on: http://gerrit.cloudera.org:8080/7183 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/master/master.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/server/rpc_server.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/tablet_server.cc 9 files changed, 176 insertions(+), 65 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibbda29dd471e4dc1a8cb4f472cc456ccdb1e48cc Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] kserver: introduce KuduServer class
Todd Lipcon has submitted this change and it was merged. Change subject: kserver: introduce KuduServer class .. kserver: introduce KuduServer class The kserver module (with the KuduServer class) is an abstraction that sits between ServerBase and Master/TabletServer. The idea is for it to encapsulate all Kudu-specific knowledge that should be shared by Masters and Tablet Servers without polluting ServerBase, which has no Kudu-specific knowledge and could be reused by any C++ application. This patch introduces the new class (just lifecycle methods for now) and wires it into Master/TabletServer. There's no new functionality. Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Reviewed-on: http://gerrit.cloudera.org:8080/7239 Reviewed-by: Todd LipconTested-by: Kudu Jenkins --- M CMakeLists.txt A src/kudu/kserver/CMakeLists.txt A src/kudu/kserver/kserver.cc A src/kudu/kserver/kserver.h M src/kudu/master/CMakeLists.txt M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/server_base.h M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 11 files changed, 192 insertions(+), 43 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I41ffd09ce46d9d744aa25343cec77b7a33b88563 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#15). Change subject: [java] separating Connection .. [java] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The updated TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.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/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,396 insertions(+), 1,252 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/15 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1929: [rpc] Allow using encrypted private keys for TLS
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-1929: [rpc] Allow using encrypted private keys for TLS .. KUDU-1929: [rpc] Allow using encrypted private keys for TLS * This patch adds a new flag for a "password command" for the RPC private key. * This also makes the webserver move to using the new function GetPasswordFromShellCommand(). * This also consolidates certificates from security-test-util into security/tests/test_certs Testing: Adds 2 tests. One to verify that RPCs work when providing the right password for password protected private keys, and one to verify that the Messenger does not startup if the wrong password is provided when using a password protected private key. Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 Reviewed-on: http://gerrit.cloudera.org:8080/6635 Tested-by: Kudu Jenkins Reviewed-by: Sailesh Mukil--- M src/kudu/integration-tests/registration-test.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/rpc-test.cc M src/kudu/security/crypto.cc M src/kudu/security/crypto.h M src/kudu/security/openssl_util.cc M src/kudu/security/openssl_util.h M src/kudu/security/openssl_util_bio.h M src/kudu/security/security-test-util.cc M src/kudu/security/security-test-util.h M src/kudu/security/test/test_certs.cc M src/kudu/security/test/test_certs.h M src/kudu/security/tls_context.cc M src/kudu/security/tls_context.h M src/kudu/server/webserver-test.cc M src/kudu/server/webserver.cc M src/kudu/server/webserver_options.cc 17 files changed, 340 insertions(+), 165 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/6635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1929: [rpc] Allow using encrypted private keys for TLS
Sailesh Mukil has posted comments on this change. Change subject: KUDU-1929: [rpc] Allow using encrypted private keys for TLS .. Patch Set 9: Code-Review+2 > Build Successful > > http://jenkins.kudu.apache.org/job/kudu-gerrit/8758/ : SUCCESS The issue was that ASSERT_DEATH() emits a warning if the enclosed statement creates multiple threads as seen here in the docs: https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md "A warning is emitted if multiple threads are running when a death test is encountered." This fails the ASSERT_DEATH() string comparison. To mitigate this, they suggest using a "threadsafe" option which somehow internally mitigates this (they didn't explain how): :testing::FLAGS_gtest_death_test_style = "threadsafe"; I've added that to the Bad password test. Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/6635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh MukilGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Use dynamic bind address for internal + external mini clusters
Alexey Serbin has posted comments on this change. Change subject: Use dynamic bind address for internal + external mini clusters .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35eff9fbf5ccf8822cfe061673bc36598dff56f0 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1929: [rpc] Allow using encrypted private keys for TLS
Hello Dan Burkert, Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6635 to look at the new patch set (#9). Change subject: KUDU-1929: [rpc] Allow using encrypted private keys for TLS .. KUDU-1929: [rpc] Allow using encrypted private keys for TLS * This patch adds a new flag for a "password command" for the RPC private key. * This also makes the webserver move to using the new function GetPasswordFromShellCommand(). * This also consolidates certificates from security-test-util into security/tests/test_certs Testing: Adds 2 tests. One to verify that RPCs work when providing the right password for password protected private keys, and one to verify that the Messenger does not startup if the wrong password is provided when using a password protected private key. Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 --- M src/kudu/integration-tests/registration-test.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/rpc-test.cc M src/kudu/security/crypto.cc M src/kudu/security/crypto.h M src/kudu/security/openssl_util.cc M src/kudu/security/openssl_util.h M src/kudu/security/openssl_util_bio.h M src/kudu/security/security-test-util.cc M src/kudu/security/security-test-util.h M src/kudu/security/test/test_certs.cc M src/kudu/security/test/test_certs.h M src/kudu/security/tls_context.cc M src/kudu/security/tls_context.h M src/kudu/server/webserver-test.cc M src/kudu/server/webserver.cc M src/kudu/server/webserver_options.cc 17 files changed, 340 insertions(+), 165 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/6635/9 -- To view, visit http://gerrit.cloudera.org:8080/6635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh MukilGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] update dependencies and clean up some deprecate method usage
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7276 to look at the new patch set (#7). Change subject: [java] update dependencies and clean up some deprecate method usage .. [java] update dependencies and clean up some deprecate method usage Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.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/TestFlexiblePartitioning.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/TestNegotiator.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/ITInputFormatJob.java M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 18 files changed, 70 insertions(+), 75 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/7276/7 -- To view, visit http://gerrit.cloudera.org:8080/7276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] mini cluster: Test infrastructure improvements
Alexey Serbin has posted comments on this change. Change subject: mini cluster: Test infrastructure improvements .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/6959/4/src/kudu/integration-tests/delete_tablet-itest.cc File src/kudu/integration-tests/delete_tablet-itest.cc: PS4, Line 49: ts_map_[mts->uuid()] A small nit which I didn't notice on the first pass. Feel free to ignore: Would it make sense make ts_map_ private and to add a method to get a pointer to the tablet server given its UUID (something like we have in x-mini-clusters?) Or for some reason we want ts_map_ to be exposed for access and modification by tests scenarios? -- To view, visit http://gerrit.cloudera.org:8080/6959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6bc6a04b113e59243fb788fec15b9935c3dcf069 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Test infrastructure improvements
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6959 to look at the new patch set (#5). Change subject: mini cluster: Test infrastructure improvements .. mini cluster: Test infrastructure improvements This patch adds a couple small improvements to the MiniCluster infra. * Add base class helpers to MiniClusterITestBase to avoid code duplication, including StopCluster() and ts_map_ for convenience and consistency with ExternalMiniClusterITestBase. * Add ListTablets() helper function to MiniTabletServer. Change-Id: I6bc6a04b113e59243fb788fec15b9935c3dcf069 --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/delete_tablet-itest.cc A src/kudu/integration-tests/internal_mini_cluster-itest-base.cc M src/kudu/integration-tests/internal_mini_cluster-itest-base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/mini_tablet_server.h 6 files changed, 82 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/6959/5 -- To view, visit http://gerrit.cloudera.org:8080/6959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6bc6a04b113e59243fb788fec15b9935c3dcf069 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Create ConsensusMetadataManager
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7191 to look at the new patch set (#7). Change subject: Create ConsensusMetadataManager .. Create ConsensusMetadataManager This patch creates an API for a class that manages consensus metadata. This abstracts out the management of ConsensusMetadata files, providing the following benefits: 1) An instance of this class can be plumbed throughout the various classes that deal with reading, creating, and modifying consensus metadata so that we don't have to pass the individual cmeta instances around. 2) It provides additional abstraction and flexibility that will make it easier to change the underlying implementation of ConsensusMetadata in the future as needed, perhaps if we wanted to make them log structured and group committed across all replicas on a tablet server. Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 --- M src/kudu/consensus/CMakeLists.txt M src/kudu/consensus/consensus_meta.cc M src/kudu/consensus/consensus_meta.h A src/kudu/consensus/consensus_meta_manager-test.cc A src/kudu/consensus/consensus_meta_manager.cc A src/kudu/consensus/consensus_meta_manager.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 24 files changed, 574 insertions(+), 139 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/7191/7 -- To view, visit http://gerrit.cloudera.org:8080/7191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] Rename MiniCluster to InternalMiniCluster
Mike Percy has posted comments on this change. Change subject: Rename MiniCluster to InternalMiniCluster .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7272/2/src/kudu/integration-tests/internal_mini_cluster.h File src/kudu/integration-tests/internal_mini_cluster.h: PS2, Line 110: MiniMaster > honestly the true ROI of this whole rename is somewhat questionable though I agree with you and the rename was taking up too much time. I don't want to prioritize doing more right now but I'll try to go back and do the rest of it soonish. Somewhat related, I think it might make sense to also add an inheritance hierarchy for the MiniTabletServer / MiniMaster too and get rid of the TServerDetails struct so we can operate on generic MiniCluster / Mini* interfaces with our cluster test utility code. But that is a bit further down the road. -- To view, visit http://gerrit.cloudera.org:8080/7272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bd326a7a46f039e18c47b8e23ee1427ccf281be Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java] update dependencies and clean up some deprecate method usage
Dan Burkert has posted comments on this change. Change subject: [java] update dependencies and clean up some deprecate method usage .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java] Ensure the shaded packages are always in the same location
Dan Burkert has submitted this change and it was merged. Change subject: [java] Ensure the shaded packages are always in the same location .. [java] Ensure the shaded packages are always in the same location Currently when a module depends on kudu-clients and is shaded it is possible to get bloat in the jar due to dependencies like guava being included in the shaded jar twice. This moves the shade plugin and relocation configuration to the parent pom's pluginManagement section to ensure the same packages are always placed in the same location. For example this reduces the kudu-client-tools jar from 13.4 MB to 8.5 MB. Note: The relocation target is changed from org.apache.kudu.clients.shaded to org.apache.kudu.shaded since the shading is not specific to the clients anymore. Change-Id: I3b519ae75a62ed9bdbf37b7132a43392dd9082db Reviewed-on: http://gerrit.cloudera.org:8080/7305 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert--- M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 6 files changed, 38 insertions(+), 64 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3b519ae75a62ed9bdbf37b7132a43392dd9082db Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Create ConsensusMetadataManager
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7191 to look at the new patch set (#6). Change subject: Create ConsensusMetadataManager .. Create ConsensusMetadataManager This patch creates an API for a class that manages consensus metadata. This abstracts out the management of ConsensusMetadata files, providing the following benefits: 1) An instance of this class can be plumbed throughout the various classes that deal with reading, creating, and modifying consensus metadata so that we don't have to pass the individual cmeta instances around. 2) It provides additional abstraction and flexibility that will make it easier to change the underlying implementation of ConsensusMetadata in the future as needed, perhaps if we wanted to make them log structured and group committed across all replicas on a tablet server. Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 --- M src/kudu/consensus/CMakeLists.txt M src/kudu/consensus/consensus_meta.cc M src/kudu/consensus/consensus_meta.h A src/kudu/consensus/consensus_meta_manager-test.cc A src/kudu/consensus/consensus_meta_manager.cc A src/kudu/consensus/consensus_meta_manager.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 24 files changed, 574 insertions(+), 135 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/7191/6 -- To view, visit http://gerrit.cloudera.org:8080/7191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] Make ConsensusMetadata thread-safe
Mike Percy has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe .. Patch Set 6: Verified+1 overriding flaky test -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java] Ensure the shaded packages are always in the same location
Dan Burkert has posted comments on this change. Change subject: [java] Ensure the shaded packages are always in the same location .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b519ae75a62ed9bdbf37b7132a43392dd9082db Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java] remove unused scalamock dependency
Dan Burkert has posted comments on this change. Change subject: [java] remove unused scalamock dependency .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97005bd5211abd0b119373ec265b8e9d01b385cf Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java] remove unused scalamock dependency
Dan Burkert has submitted this change and it was merged. Change subject: [java] remove unused scalamock dependency .. [java] remove unused scalamock dependency Change-Id: I97005bd5211abd0b119373ec265b8e9d01b385cf Reviewed-on: http://gerrit.cloudera.org:8080/7292 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert--- M java/kudu-spark-tools/pom.xml 1 file changed, 0 insertions(+), 6 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I97005bd5211abd0b119373ec265b8e9d01b385cf Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Ensure the shaded packages are always in the same location
Grant Henke has uploaded a new change for review. http://gerrit.cloudera.org:8080/7305 Change subject: [java] Ensure the shaded packages are always in the same location .. [java] Ensure the shaded packages are always in the same location Currently when a module depends on kudu-clients and is shaded it is possible to get bloat in the jar due to dependencies like guava being included in the shaded jar twice. This moves the shade plugin and relocation configuration to the parent pom's pluginManagement section to ensure the same packages are always placed in the same location. For example this reduces the kudu-client-tools jar from 13.4 MB to 8.5 MB. Note: The relocation target is changed from org.apache.kudu.clients.shaded to org.apache.kudu.shaded since the shading is not specific to the clients anymore. Change-Id: I3b519ae75a62ed9bdbf37b7132a43392dd9082db --- M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 6 files changed, 38 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/7305/1 -- To view, visit http://gerrit.cloudera.org:8080/7305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3b519ae75a62ed9bdbf37b7132a43392dd9082db Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke
[kudu-CR] [java] Ensure the shaded packages are always in the same location
Grant Henke has abandoned this change. Change subject: [java] Ensure the shaded packages are always in the same location .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/7304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I6423a22e3cc586c76e16105582d910dc37a79b49 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Ensure the shaded packages are alway in the same location
Grant Henke has abandoned this change. Change subject: [java] Ensure the shaded packages are alway in the same location .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/7302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ib830ba386313763ad96511dc19a65beb3bb24543 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Ensure the shaded packages are always in the same location
Grant Henke has uploaded a new change for review. http://gerrit.cloudera.org:8080/7304 Change subject: [java] Ensure the shaded packages are always in the same location .. [java] Ensure the shaded packages are always in the same location Currently when a module depends on kudu-clients and isshaded it is possible to get bloat in the jar due todependencies like guava being included in the shadedjar twice.This moves the shade plugin and relocation configurationto the parent pom's pluginManagement section to ensure thesame packages are always placed in the same location.For example this reduces the kudu-client-tools jarfrom 13.4 MB to 8.5 MB.Note: The relocation target is changed fromorg.apache.kudu.clients.shaded toorg.apache.kudu.shaded since the shading is not specific tothe clients anymore.Change-Id: Ib830ba386313763ad96511dc19a65beb3bb24543 Change-Id: I6423a22e3cc586c76e16105582d910dc37a79b49 --- M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 6 files changed, 38 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/7304/1 -- To view, visit http://gerrit.cloudera.org:8080/7304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6423a22e3cc586c76e16105582d910dc37a79b49 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke
[kudu-CR] [java] Ensure the shaded packages are alway in the same location
Grant Henke has uploaded a new change for review. http://gerrit.cloudera.org:8080/7302 Change subject: [java] Ensure the shaded packages are alway in the same location .. [java] Ensure the shaded packages are alway in the same location Currently when a module depends on kudu-clients and is shaded it is possible to get bloat in the jar due to dependencies like guava being included in the shaded jar twice. This moves the shade plugin and reolocation configuration to the parent poms pluginManagement section to ensure the same packages are always placed in the same location. For example this reduces the kudu-client-tools jar from 13.4 MB to 8.5 MB. Note: The relocation target is changed from org.apache.kudu.clients.shaded to org.apache.kudu.shaded since the shading is not specific to the clients anymore. Change-Id: Ib830ba386313763ad96511dc19a65beb3bb24543 --- M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 6 files changed, 38 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/7302/1 -- To view, visit http://gerrit.cloudera.org:8080/7302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib830ba386313763ad96511dc19a65beb3bb24543 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke
[kudu-CR] [java] update dependencies and clean up some deprecate method usage
Grant Henke has posted comments on this change. Change subject: [java] update dependencies and clean up some deprecate method usage .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/7276/5/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java: Line 463: throw new IOException("no peer cert found"); > Sorry I was a bit unclear - this should remain an SSLPeerUnverifiedExceptio Done Line 582: throw new IOException("no channel bindings provided by remote peer"); > Actually these could be 'SSLPeerUnverifiedException' as well, that probably Done http://gerrit.cloudera.org:8080/#/c/7276/5/java/kudu-client/src/test/java/org/apache/kudu/client/TestFlexiblePartitioning.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestFlexiblePartitioning.java: Line 454:.add("a", a) > align periods vertically ('chop-down' style) Done -- To view, visit http://gerrit.cloudera.org:8080/7276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [java] update dependencies and clean up some deprecate method usage
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7276 to look at the new patch set (#6). Change subject: [java] update dependencies and clean up some deprecate method usage .. [java] update dependencies and clean up some deprecate method usage Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.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/TestFlexiblePartitioning.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/TestNegotiator.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/ITInputFormatJob.java M java/kudu-spark-tools/pom.xml M java/kudu-spark/pom.xml M java/pom.xml 18 files changed, 72 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/7276/6 -- To view, visit http://gerrit.cloudera.org:8080/7276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4b9746f70187c5a7ca0dd016db92ae4e6de7ecc3 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Make ConsensusMetadata thread-safe
David Ribeiro Alves has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] ConsensusMetadata::Create() should not overwrite an existing file
David Ribeiro Alves has posted comments on this change. Change subject: ConsensusMetadata::Create() should not overwrite an existing file .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0bfef1ae86a7d2c162095a76bb184a9c18dc69a7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rename MiniCluster to InternalMiniCluster
David Ribeiro Alves has posted comments on this change. Change subject: Rename MiniCluster to InternalMiniCluster .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7272/2/src/kudu/integration-tests/internal_mini_cluster.h File src/kudu/integration-tests/internal_mini_cluster.h: PS2, Line 110: MiniMaster > Adar brought this up earlier; see my earlier response. It's just low ROI gr honestly the true ROI of this whole rename is somewhat questionable though I do see the consistency point. Seems to me that if we are to be consistent and went the extra mile to do so we should be so all over, it doesn't make a lot of sense have an InternalMiniCluster with a MiniMaster/MiniTabletServer that is not explicitly internal. I don't understand why we'd draw the line there as it seems like the job is unfinished (which it didn't seem before this rename). I don't mind doing it in another patch, but I do think we should do it. -- To view, visit http://gerrit.cloudera.org:8080/7272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bd326a7a46f039e18c47b8e23ee1427ccf281be Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] TabletReplica: Move Init() logic to Start()
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7194 to look at the new patch set (#4). Change subject: TabletReplica: Move Init() logic to Start() .. TabletReplica: Move Init() logic to Start() This is a cleanup / refactor that will make it easier to implement tombstoned voting. In this patch we merge Init() and Start() since they aren't really logically different right now. An unrelated change in this patch is a minor API cleanup in TSTabletManager, where we now pass TabletReplica directly to TSTabletManager::OpenTablet() instead of registering it in a map first and then looking it up again later. Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a --- M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 8 files changed, 151 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/7194/4 -- To view, visit http://gerrit.cloudera.org:8080/7194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] Rename MiniClusterBase to MiniCluster
Alexey Serbin has posted comments on this change. Change subject: Rename MiniClusterBase to MiniCluster .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rename MiniCluster to InternalMiniCluster
Alexey Serbin has posted comments on this change. Change subject: Rename MiniCluster to InternalMiniCluster .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bd326a7a46f039e18c47b8e23ee1427ccf281be Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired
Alexey Serbin has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired .. Patch Set 1: (21 comments) Thank you for the review. I'll address the major points in the next patch revision. Right now I wanted to quickly respond and bring the code up-to-date with the updated revision of the parent patch. http://gerrit.cloudera.org:8080/#/c/7250/1//COMMIT_MSG Commit Message: PS1, Line 9: current > 'the current' Done http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 272: private RpcProxy newRpcProxy(final ServerInfo serverInfo, boolean usePrimaryCredentials) { > Put this directly under the 1-arg version, and copy over the relevant javad Done http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java File java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java: Line 38: public class AuthnTokenReacquirer { > Did you consider making this functionality part of SecurityContext? Is the Nope, I didn't think about making this is a part of SecurityContext. I'll give it a try, thanks. Line 48: private final ReentrantLock lock = new ReentrantLock(); > It looks like this could be simplified to just be an Object with synchroniz This sounds good, I will give it a try. Line 54: // TODO(aserbin): remove the client parameter > Looks like this TODO may be stale Done Line 65: // The token re-acquirer should be able to mark all the queued messages as failed if after some > Should this be a TODO? Right -- that was supposed to be a TODO. Done. Line 72:* @param connection connection to notify on the > sentence fragment Done Line 142: if (e instanceof RecoverableException && retries++ < 5) { > Could you move the increment of retries out of the if condition? It's diff Done Line 174: ConnectToCluster.run(masterTable, masterAddresses, null, > This won't reuse cached Master connections, right? Don't we do that on the This will reuse cached Master connection if the connection to the requested destination is present in the cache and established with primary credentials. http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 102: private final ClientSocketChannelFactory channelFactory; > Doesn't look like this gets used outside the ctor? Right -- done. Line 272: if (m instanceof Negotiator.Failure) { > Could we get in a weird state here where this connection is actually a mast Yep, that's possible -- I replied on your comment on ConnectionCache.getConnection() method. Line 282: Preconditions.checkState(state == State.NEGOTIATING); > Might be worth adding a check that this connection is not using primary cre That's a good idea. Yes, if that happens, we should report an exception so it would be handled appropriately. Line 668: TO_BE_RENEGOTIATED, // The connection negotiation has failed and has to be be re-negotiated. > If I understand correctly, this patch is changing Connection so that it act Yep, ideally it would be nice to have just DISCONNECTED state and recycle disconnected connections. But then it's necessary to move the queued RPCs into a new connection. Probably, that can be done at the ConnectionCache level. I'll take a closer look at that. http://gerrit.cloudera.org:8080/#/c/7250/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java: Line 100: // 3. A connection with primary credentials is requested but currently registered one > Hmm, this comment would seem to contradict my earlier understanding about a Probably, your confusion comes from the naming for the 'DISCONNECTED' state. One small detail: when connection negotiation fails, the state changes to TO_BE_RENEGOTIATED, not DISCONNECTED. The DISCONNECTED state is a terminal state in the state diagram. And yes, a Connection object starts re-negotiation, after getting into TO_BE_RENEGOTIATED state. Such a connection will stay in the cache unless that's a connection to the master where we need to establish a new connection using primary credentials. In the latter case, the re-negotiation happens anyway, but the re-negotiated connection is replaced in the cache with the primary-credentials-connection-to-master which is used to re-acquire authn token. The queued RPCs from the re-negotiated old connection-to-the-same-master will be sent when the connection is re-negotiated using the new authn token. In that case there will be two connections from the client to the same
[kudu-CR] [java] separating Connection
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7146 to look at the new patch set (#14). Change subject: [java] separating Connection .. [java] separating Connection This patch separates lower-level, connection-related functionality from the TabletClient class into the new Connection class. The updated TabletClient has been renamed into RpcHelper. Also, this patch contains other micro-updates on the related code. In addition, this patch addresses KUDU-1878. This work is done in the context of KUDU-2013. Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java A java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java A java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java D java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.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/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java 21 files changed, 1,389 insertions(+), 1,256 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/14 -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] KUDU-2013 re-acquire authn token if expired
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7250 to look at the new patch set (#2). Change subject: [java] KUDU-2013 re-acquire authn token if expired .. [java] KUDU-2013 re-acquire authn token if expired This patch introduces automatic authn token re-acquisition when the current authn token expires. The client automatically retries the RPC that hits the token expiration error (the error to re-try is seen as FATAL_INVALID_AUTHENTICATION_TOKEN sent by the server during connection negotiation). Added a few of tests to exercise the new retry logic for automatic token re-acquisition in case of master-only operations, a bare minimum workload scenario, and one special case of a connection to the master opened with secondary credentials. Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquireOpen.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/TestNegotiator.java 13 files changed, 676 insertions(+), 78 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/7250/2 -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] separating Connection
Alexey Serbin has posted comments on this change. Change subject: [java] separating Connection .. Patch Set 10: (7 comments) http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG Commit Message: Line 11: The TabletClient has been renamed into RpcHelper. Also, > update 'RpcHelper' Done http://gerrit.cloudera.org:8080/#/c/7146/9/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 151: /** A trivial cache to keep track of already opened connections to Kudu servers. */ > Hmm so I don't think the new comment is adding much clarity. I think i'd p Done Line 170:* @see ../src/kudu/common/common.proto > This is relative to where you have set up the root IntelliJ project. I'd p Done http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 479: } > omit the trailing period Done http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java: Line 51 > I think the @ needs to go inside the opening brace Done Line 61 > This comment can be omitted, it's obvious from context Done Line 73 > omit trailing periods on param docs Done -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes