[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6809/1/src/kudu/integration-tests/cluster_itest_util.cc
File src/kudu/integration-tests/cluster_itest_util.cc:

Line 51: using client::KuduTable;
> warning: using decl 'KuduTable' is unused [misc-unused-using-decls]
Done


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

Line 43: using consensus::OpId;
> warning: using decl 'OpId' is unused [misc-unused-using-decls]
Rebased and this became used


Line 44: using consensus::RECEIVED_OPID;
> warning: using decl 'RECEIVED_OPID' is unused [misc-unused-using-decls]
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config and the pending leader, if the pending config has a leader.
Previously, consumers of the consensus state would ask for either
the committed or active (= pending if there is a pending config,
else the committed config) config. With this change, the committed
and active configs are always included.

This change would be backwards-incompatible (the master receives
a ConsensusStatePB in a tablet report) but because the new semantics
are that the old fields on the ConsensusStatePB hold the committed
config, and the master only ever looked at the committed config,
it's ok.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
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/metadata.proto
M src/kudu/consensus/quorum_util.cc
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_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
24 files changed, 80 insertions(+), 126 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] WIP [java] re-acquire authn token if expired

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: WIP [java] re-acquire authn token if expired
..

WIP [java] re-acquire authn token if expired

This patch introduces automatic authn token re-acquisition when current
authn token expires.  The client automatically retries the RPC that
hits the token expiration error.

Added a test to exercise the new retry logic for token expiration in
case of a basic workload scenario.

In this commit I also added a few lines of code to retry RPC for the
case if server sends ERROR_UNAVAILABLE error code (which is a broader
version of its ERROR_SERVER_TOO_BUSY counterpart).

WIP: there are a couple of TODOs to address
  * synchronization of setting/resetting authn token in the client
context
  * a test scenario with more exhaustive workload

Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.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/TabletClient.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
4 files changed, 231 insertions(+), 18 deletions(-)


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

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

[kudu-CR] maintenance manager: schedule work immediately when threads are free

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: maintenance_manager: schedule work immediately when threads are 
free
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6815/1/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:

PS1, Line 221: or it is time to run another op.
The "shutting down" part is fairly intuitive, but this part isn't. It might be 
useful to expand on this and explain how it relates to the two parts of the 
condition (comparing running_ops to num_threads and force_sleep_next_iter).


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

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

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: cert: add check for pending SSL errors in cert-related code
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6814/1/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:

Line 139:   OPENSSL_RET_NOT_OK(X509_check_private_key(data_.get(), 
key.GetRawData()),
Would it make sense to add SCOPED_OPENSSL_NO_PENDING errors here as well?


Line 212:   EVP_PKEY* raw_key = X509_get_pubkey(data_.get());
Ditto?


Line 240:   EVP_PKEY* raw_key = X509_REQ_get_pubkey(data_.get());
ditto?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1970: node density integration test

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: KUDU-1970: node density integration test
..


KUDU-1970: node density integration test

This patch introduces a new itest that simulates a storage-dense Kudu
deployment. The idea is simple: rather than actually generating and storing
lots of data (which is both time intensive and developer unfriendly), let's
run a workload that produces a lot of metadata with a minimal amount of
data. This is cheaper, and the metadata can proxy for data in areas we care
about (such as start up time, thread count, memory usage, etc.). The test
itself isn't that interesting; most of the challenge was in running it
repeatedly to determine which flag values yielded the most metadata.

In a run of the test on a 48 core el6.6 machine (max_blocks_per_container=8,
num_tablets=1000, num_seconds=240), I produced ~110K blocks across ~21k LBM
containers, which yielded a subsequent ~100s LBM startup time.

I made the following modifications elsewhere to make this work:
- TestWorkload now supports arbitrary schemas.
- EMC-based tests can configure the amount of time they wait on each daemon
  process to start as the server info file isn't dumped until after FS
  startup is complete (maybe that should be changed?)
- The benchmarks.sh script runs the test with some customized parameters.

I also snuck in changes to remove an unused variable from random.h and to
switch TestWorkload from kudu::Thread to std::thread.

Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d
Reviewed-on: http://gerrit.cloudera.org:8080/6662
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.h
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/scripts/benchmarks.sh
M src/kudu/tools/data_gen_util.cc
M src/kudu/tools/data_gen_util.h
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/util/random.h
13 files changed, 355 insertions(+), 48 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d
Gerrit-PatchSet: 12
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: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] maintenance manager: schedule work immediately when threads are free

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: maintenance_manager: schedule work immediately when threads are 
free
..

maintenance_manager: schedule work immediately when threads are free

This changes the MM so that, when a worker thread becomes available, it
immediately wakes up the scheduler to schedule the next available work item.
Additionally, the scheduler will loop scheduling new items as long as there
are free worker threads, instead of only scheduling once per polling
interval.

Change-Id: I63c4b48f5f02f3a1d3a8964993e78037ce72b1da
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 12 insertions(+), 1 deletion(-)


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

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

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: cert: add check for pending SSL errors in cert-related code
..

cert: add check for pending SSL errors in cert-related code

We missed these functions which use SSL libraries before. Adar saw a
test failure with a pending error from the OBJ library in an unrelated
test, so my best guess is it came from here. The scoped checker should
help us find if this is the case.

Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
---
M src/kudu/security/cert.cc
1 file changed, 6 insertions(+), 0 deletions(-)


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

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

[kudu-CR] KUDU-1970: node density integration test

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1970: node density integration test
..


Patch Set 11: Code-Review+2

Chatting about the naming situation on another channel Adar made the point that 
this is more of a stress test than an actual benchmark. Which makes sense

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d
Gerrit-PatchSet: 11
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: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [rpc] use C++11 move semantics for selected methods

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [rpc] use C++11 move semantics for selected methods
..


Patch Set 3:

(1 comment)

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

Line 159:   for (auto it = awaiting_response_.begin(); it != end; ++it) {
> Move semantics are making the code significantly more complex here, and onl
+1, not worth optimizing this at the expense of clarity


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ee8d69a620db597ed9d27c4284656a4169bb09
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: tablet: skip over gaps in rowsets using key index instead of bloom

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: tablet: skip over gaps in rowsets using key index instead 
of bloom
..


Patch Set 2:

review?

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

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

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..


KUDU-1991. master: Retry background tasks even if TS UUID not registered

Before this patch, if a master had just started up and got a message
from a tablet server that would require some action in response (for
example, deleting an evicted replica or replacing a replica in an
under-replicated tablet) then if an affected tablet server had not
registered with the master the task would fail and would not be retried.
This patch causes the task to be retried even if the initial tablet
server UUID lookup fails.

This change is tested by
AdminCliTest.TestUnsafeChangeConfigOnSingleFollower, which prior to this
patch was flaky (7/200 tests failed on dist-test before the patch).
After the changes to that test, it requires the above master fix to pass
but is no longer flaky (200/200 passed on dist-test).

Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Reviewed-on: http://gerrit.cloudera.org:8080/6534
Reviewed-by: Adar Dembo 
Tested-by: Mike Percy 
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-admin-test.cc
4 files changed, 102 insertions(+), 61 deletions(-)

Approvals:
  Mike Percy: Verified
  Adar Dembo: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..


Patch Set 1:

(1 comment)

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

PS1, Line 14: This patch causes the task to be retried even if the initial 
tablet
: server UUID lookup fails.
> Works for me.
Great. Thanks for the review.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..


Patch Set 4: Verified+1

Overriding Jenkins failure due to flaky test caused by KUDU-1736

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config and the pending leader, if the pending config has a leader.
Previously, consumers of the consensus state would ask for either
the committed or active (= pending if there is a pending config,
else the committed config) config. With this change, the committed
and active configs are always included.

This change would be backwards-incompatible (the master receives
a ConsensusStatePB in a tablet report) but because the new semantics
are that the old fields on the ConsensusStatePB hold the committed
config, and the master only ever looked at the committed config,
it's ok.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
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/metadata.proto
M src/kudu/consensus/quorum_util.cc
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_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
24 files changed, 78 insertions(+), 122 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..


Patch Set 4: Code-Review+2

(1 comment)

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

PS1, Line 14: This patch causes the task to be retried even if the initial 
tablet
: server UUID lookup fails.
> I agree that is a problem. This works around the steady-state case that may
Works for me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] spark: add support for fault tolerant scanner

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: spark: add support for fault tolerant scanner
..


Patch Set 5:

(2 comments)

Would be nice to have a test of the new functionality (setting the fault 
tolerant option), even if it doesn't inject faults.

http://gerrit.cloudera.org:8080/#/c/6782/5/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala:

Line 50:   val FAULT_TOLERANT_SCANNER = "kudu.fault.tolerant.scan"
hmm should this be something like 'kudu.fault-tolerant-scan'?  I typically 
think of the dots as representing namespaces.


Line 147:   private val isFaultTolerant: Boolean = faultTolerantScanner;
This isn't strictly necessary, you can reference 'FaultTolerantScanner' 
directly on line 184.  The 'val' part of line 135 actually makes it a field 
automatically.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f3192025ca5d74197600480fd3d040d70b4bbc2
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Fix flaky test TestRecoverFromOpIdOverflow

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Fix flaky test TestRecoverFromOpIdOverflow
..


Patch Set 1:

(3 comments)

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

PS1, Line 9: This test is flaky because we race against the COMMIT message for 
the
   : first NO_OP in the WAL being written. It is currently hard to know 
when
   : the actual COMMIT message is written to the WAL so we use a 
workaround
   : to delete the first log segment before restarting the EMC in this 
test.
can you explain a little better how we'd get an overflow here?


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

PS1, Line 368: // Before restarting the tablet server, delete the initial log 
segment from
 : // disk (the original leader election NO_OP) if it exists 
since it will
 : // contain OpId 1.1; if it doesn't also contain the COMMIT 
message for 1.1
 : // yet then it will trigger a CHECK complaining about 
non-sequential OpIds
 : // in the WAL at tablet bootstrap time.
If I understand what you're doing it that you remove the first segment to make 
sure the old op isn't committed on restart. Is that right? Can this happen in 
the wild?


PS1, Line 383: wal_children
you mean "wal_dir" right?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib382819307da04bb76d68d2c015dc0edd9f60267
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..


Patch Set 1:

(1 comment)

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

PS1, Line 14: This patch causes the task to be retried even if the initial 
tablet
: server UUID lookup fails.
> Yeah I meant that this is edge triggered (always has been). I agree level t
I agree that is a problem. This works around the steady-state case that may 
commonly happen at cluster startup time and I've filed KUDU-1991 to track this 
issue.

I've also filed KUDU-1997 to track the edge-triggered vs. level-triggered issue 
separately so I think we should get this in now and prioritize KUDU-1997 
separately.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1991. master: Retry background tasks even if TS UUID not registered

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1991. master: Retry background tasks even if TS UUID not 
registered
..

KUDU-1991. master: Retry background tasks even if TS UUID not registered

Before this patch, if a master had just started up and got a message
from a tablet server that would require some action in response (for
example, deleting an evicted replica or replacing a replica in an
under-replicated tablet) then if an affected tablet server had not
registered with the master the task would fail and would not be retried.
This patch causes the task to be retried even if the initial tablet
server UUID lookup fails.

This change is tested by
AdminCliTest.TestUnsafeChangeConfigOnSingleFollower, which prior to this
patch was flaky (7/200 tests failed on dist-test before the patch).
After the changes to that test, it requires the above master fix to pass
but is no longer flaky (200/200 passed on dist-test).

Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-admin-test.cc
4 files changed, 102 insertions(+), 61 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a3de7fe87266f11392fd3bb0c74f19ad803de9d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Fix bug in incorrect response rebuilding on tablet bootstrap

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Fix bug in incorrect response rebuilding on tablet bootstrap
..


Fix bug in incorrect response rebuilding on tablet bootstrap

This fixes the bug described in: https://gerrit.cloudera.org/#/c/5417/
... and enables the test disabled in that patch.

Along the way this also performs some cleanup of tablet bootstrap.

I ran raft_consensus-itest's TestInsertDuplicateKeysWithCrashyNodes
on dist-test for 5000 loops with the following config:

KUDU_ALLOW_SLOW_TESTS=1 ../../build-support/dist_test.py \
--disable-sharding loop -n 5000 -- bin/raft_consensus-itest \
--gtest_filter=*DuplicateKeysWithCrashyNodes

Prior to this patch the test failed 62/5000:
http://dist-test.cloudera.org//job?job_id=david.alves.1493915326.763

After this patch the test passes 5000/5000:
http://dist-test.cloudera.org//job?job_id=david.alves.1493914745.27867

Change-Id: I1219ed5f7835e93cd7f3b128cedd650fc3384482
Reviewed-on: http://gerrit.cloudera.org:8080/5489
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/tablet/row_op.cc
M src/kudu/tablet/row_op.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tserver/tablet_server-test.cc
5 files changed, 122 insertions(+), 114 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1219ed5f7835e93cd7f3b128cedd650fc3384482
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix bug in incorrect response rebuilding on tablet bootstrap

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Fix bug in incorrect response rebuilding on tablet bootstrap
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1219ed5f7835e93cd7f3b128cedd650fc3384482
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: reduce contention on MemTracker::Release and Consume in block cache

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: WIP: reduce contention on MemTracker::Release and Consume in 
block cache
..


Patch Set 1:

think i'll end up rolling this into some other stuff

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

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

[kudu-CR] WIP: reduce contention on MemTracker::Release and Consume in block cache

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: reduce contention on MemTracker::Release and Consume in 
block cache
..


Patch Set 1:

needs review? or will be rolled in to other patches?

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

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

[kudu-CR] [java client] - Add getter/setter for the last propagated timestamp to KuduClient

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has abandoned this change.

Change subject: [java client] - Add getter/setter for the last propagated 
timestamp to KuduClient
..


Abandoned

this change was superceded by aserbin's changes

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: If62edd8e2f96221c7f6f318d79f8198e10e2dba6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [util] use emplace() instead of insert()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [util] use emplace() instead of insert()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6807/1/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

Line 364:   pair ret = 
collection->emplace(key, value);
> I think you are right, but that would be another level of optimization.  As
hm, I'm not sure I believe that it makes a difference... I wrote this test 
program:

https://gist.github.com/075a7dfe3db157893af526078968ba25

which puts shared_ptr<>s in a map, so that there is a non-trivial copy 
constructor (incrementing ref count), and found that there was no difference 
between these options. The optimizer's probably smart enough to see where it 
can do these optimizations as is


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: release WritableLogSegment buffers when log is idle

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: release WritableLogSegment buffers when log is idle
..


Patch Set 2:

(2 comments)

yeah, it'd nice to quantify how much data we're wasting on this.

http://gerrit.cloudera.org:8080/#/c/6611/2/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

PS2, Line 224: // TODO(todd): consider rolling to a non-preallocated segment if 
we are
 :   // idle for a good period of time (eg a few minutes), so 
that startup time is
 :   // improved and disk usage is minimized.
I forget, do all of you memstores get flushed in a time basis, no matter how 
small?


http://gerrit.cloudera.org:8080/#/c/6611/2/src/kudu/util/faststring-test.cc
File src/kudu/util/faststring-test.cc:

PS2, Line 18: #include 
: #include "kudu/util/faststring.h"
: #include "kudu/util/random.h"
: #include "kudu/util/random_util.h"
: #include "kudu/util/test_util.h"
: 
: namespace kudu {
: class FaststringTest : public KuduTest {};
: 
: TEST_F(FaststringTest, TestShrinkToFit_Empty) {
:   faststring s;
:   s.shrink_to_fit();
:   ASSERT_EQ(faststring::kInitialCapacity, s.capacity());
: }
: 
: TEST_F(FaststringTest, 
TestShrinkToFit_SmallerThanInitialCapacity) {
:   faststring s;
:   s.append("hello");
:   s.shrink_to_fit();
:   ASSERT_EQ(faststring::kInitialCapacity, s.capacity());
: }
: 
: TEST_F(FaststringTest, TestShrinkToFit_Random) {
:   Random r(GetRandomSeed32());
:   int kMaxSize = faststring::kInitialCapacity * 2;
:   std::unique_ptr random_bytes(new char[kMaxSize]);
:   RandomString(random_bytes.get(), kMaxSize, );
: 
:   faststring s;
:   for (int i = 0; i < 100; i++) {
: int new_size = r.Uniform(kMaxSize);
: s.resize(new_size);
: memcpy(s.data(), random_bytes.get(), new_size);
: s.shrink_to_fit();
: ASSERT_EQ(0, memcmp(s.data(), random_bytes.get(), new_size));
: ASSERT_EQ(std::max(faststring::kInitialCapacity, 
new_size), s.capacity());
:   }
: }
split the faststring changes?


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

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

[kudu-CR] cache: add a benchmark

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: cache: add a benchmark
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

Line 95: 
nit: extra line


PS1, Line 97: TEST_P(CacheBench, RunBench) {
would it be interesting to also measure cache churn?


PS1, Line 98: 16;
make this a flag?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1970: node density integration test

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1970: node density integration test
..


Patch Set 11:

here's your precedent (unmerged) :) 
https://gerrit.cloudera.org/#/c/6696/1/src/kudu/util/cache-bench.cc

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d
Gerrit-PatchSet: 11
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: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP Don't suicide on EIO

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP Don't suicide on EIO
..

WIP Don't suicide on EIO

Rather than suiciding when reaching an EIO, this patch adds a
mechanism that triggers error-handling in the form of a callback.
This handler is attached to the lowest non-env operations that may
result in EIO.
E.g. PosixRWFile::Write() is an env operation that may result in an
EIO. LogBlockContainer::WriteData()'s call to it must be wrapped in
the new error-handling macro KUDU_RETURN_OR_HANDLE_EIO. Thus, all
direct readers/writers of files must now implement EIO-handling code
and wrap disk IO in KUDU_RETURN_OR_HANDLE_EIO.

This patch is marked WIP, as the correct behavior after a disk fails
is not yet included. For now, upon failure, the block managers will
keep track of the dead disks and will mark the appropriate tablet
peers as failed. Additionally, the error handling has been moved
between layers offline, so more cleanup is being done to align with
the above description.

Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
---
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
A src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/tablet/tablet.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/fault_injection.cc
M src/kudu/util/fault_injection.h
M src/kudu/util/status.h
20 files changed, 426 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP Don't suicide on EIO

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP Don't suicide on EIO
..

WIP Don't suicide on EIO

Rather than suiciding when reaching an EIO, this patch adds a
mechanism that triggers error-handling in the form of a callback.
This handler is attached to the lowest non-env operations that may
result in EIO.
E.g. PosixRWFile::Write() is an env operation that may result in an
EIO. LogBlockContainer::WriteData()'s call to it must be wrapped in
the new error-handling macro KUDU_RETURN_OR_HANDLE_EIO. Thus, all
direct readers/writers of files must now implement EIO-handling code
and wrap disk IO in KUDU_RETURN_OR_HANDLE_EIO.

This patch is marked WIP, as the correct behavior after a disk fails
is not yet included. For now, upon failure, the block managers will
keep track of the dead disks and will mark the appropriate tablet
peers as failed. Additionally, the error handling has been moved
between layers offline, so more cleanup is being done to align with
the above description.

Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
---
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/tablet/tablet.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/fault_injection.cc
M src/kudu/util/fault_injection.h
M src/kudu/util/status.h
19 files changed, 350 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [util] use emplace() instead of insert()

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [util] use emplace() instead of insert()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6807/1/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

Line 364:   pair ret = 
collection->emplace(key, value);
> don't we need to change 'key' and 'value' above to be by-value and then use
I think you are right, but that would be another level of optimization.  As I 
understand, using emplace() instead of insert(), we are getting rid of extra 
value_type() (i.e. pair) constructor.

I would prefer to address changing const-ref semantics to move semantics 
separately, because there might be some implications at call sites which would 
require modifying more code in many different places.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cfile set: avoid Status allocation for row not found

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: cfile_set: avoid Status allocation for row not found
..


cfile_set: avoid Status allocation for row not found

We expect Bloom Filter lookups to typically return "row not found". The
code currently indicates this using a Status::NotFound(). However,
constructing such a status involves an alloc/free pair, which is
relatively expensive to do millions of times per second for this common
case.

In a profile of tcph_real_world, the tserver spends a good chunk of its
CPU in these code paths, especially now that our alloc/free contain
heavier instrumentation for memory accounting.

This changes to using a boost::optional out-parameter for the
common case instead.

Benchmarked with tpch_real_world SF300 as in change ID
I8b7e703c82ac14fbce3a699bdf6a2f0fb4ed93a1:

Before   After
Mem rejections  62,790   75,318   (1.20x more)
Wall time   2490s2185s(1.14x faster)
Server CPU  72,873s  61,879s  (1.18x faster)

The increased mem rejections makes sense, since we're now able to insert
a bit faster while maintenance operations stayed the same speed. The CPU
improvements are due to this optimization, and the wall improvement is a
result of the CPU improvement.

Change-Id: I3056aaaf0eec6b7deebcd1f206cb551b75996927
Reviewed-on: http://gerrit.cloudera.org:8080/6804
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diskrowset.cc
3 files changed, 33 insertions(+), 24 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3056aaaf0eec6b7deebcd1f206cb551b75996927
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1949. Maintenance Manager should trigger flushes earlier

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1949. Maintenance Manager should trigger flushes earlier
..


KUDU-1949. Maintenance Manager should trigger flushes earlier

This changes the maintenance manager to start triggering flushes at the
60% heap usage threshold, and adjusts the memory limiting to only start
rejecting writes at the 80% threshold.

This is based on analysis in [1] ("After tweaking memory backpressure
rejection") in which we found that earlier flushing could keep the
system from making too many rejections.

I ran tpch_real_world with SF=300 as described in the doc below before
and after the patch. Some key metrics:

Before   After
Mem rejections  1,162,52762,790(18.5x fewer)
Wall time   3156s2490s (1.26x faster)
Server CPU  37,348s  72,873s   (1.95x less efficient)

This matches the results from the original experiment. Because the
writes come in faster, we don't get enough time to run compactions, and
so each write ends up taking more CPU. Some future optimizations are
still pending which will reduce the CPU time per bloom lookup.
Regardless, this new behavior is configurable such that, if overall CPU
efficiency is more important than wall time, we could bump the "memory
pressure threshold" up higher than the "memory rejection threshold".

[1] 
https://docs.google.com/document/d/1U1IXS1XD2erZyq8_qG81A1gZaCeHcq2i0unea_eEf5c/edit#heading=h.gw87heq4fq6l

Change-Id: I8b7e703c82ac14fbce3a699bdf6a2f0fb4ed93a1
Reviewed-on: http://gerrit.cloudera.org:8080/6802
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/process_memory.cc
M src/kudu/util/process_memory.h
3 files changed, 31 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8b7e703c82ac14fbce3a699bdf6a2f0fb4ed93a1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [util] use emplace() instead of insert()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [util] use emplace() instead of insert()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6807/1/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

Line 364:   pair ret = 
collection->emplace(key, value);
don't we need to change 'key' and 'value' above to be by-value and then use 
std::move here? Otherwise it's still going to be calling the copy constructor 
instead of an available move constructor


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
..


Patch Set 17:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 428:   ASSERT_STR_CONTAINS(s.ToString(), "DataDirGroup not found for 
tablet");
Before asserting on the status' string, assert on its type (i.e. 
ASSERT_TRUE(s.IsNotFound()) or whatever).


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs-test.cc
File src/kudu/fs/data_dirs-test.cc:

PS17, Line 56:   virtual void TearDown() override {
 : KuduTest::TearDown();
 :   }
You can omit this since it doesn't do anything beyond what the base class does.


PS17, Line 79:   // Parent block manager. The selection of FBM over LBM 
shouldn't be relevant
 :   // to these tests.
Would it be possible to instantiate a DataDirManager and forgo the block 
manager altogether? That would eliminate some unnecessary test setup.


Line 96:   Status s = dd_manager_->GetNextDataDir(non_existent_opts, nullptr);
Test the Status programmatically before testing its string representation.

Elsewhere too.


PS17, Line 115:   FLAGS_fs_data_dirs_reserved_bytes = 1;
  :   FLAGS_disk_reserved_bytes_free_for_testing = 2;
Why is it necessary to set these? Won't GetNextDataDir() return Status::OK() 
even if they're unset?


Line 138:   FLAGS_fs_target_data_dirs_per_tablet = group_size_;
Maybe do this once in the test constructor, and do it again in each test that 
needs to override group_size_?


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

PS17, Line 254:   // Seed the RNG for data directory group operations.
  :   std::srand(std::time(0));
I don't think it makes sense to modify process-level state deep in an object's 
constructor like this. Why did you need this in the first place? When we need 
to seed a PRNG in unit tests, we use SeedRandom().


Line 394: InsertOrDie(_by_uuid_idx_map_, idx, 
std::set());
Don't need std:: prefixes here.


Line 479: DataDir* candidate = data_dir_by_uuid_idx_[uuid_idx];
FindOrDie() here?


PS17, Line 518: no_dirs_found
Instead of the separate bool, could you wrap new_uuid_idx in a boost::optional? 
It'll be initialized to boost::none by default I think, and then the caller can 
check if it's still boost::none after the call or whether it's been initialized 
to a real value.


Line 548: 
RETURN_NOT_OK(candidate->RefreshIsFull(DataDir::RefreshMode::ALWAYS));
Why do we need to refresh?


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

Line 51: // A DataDirGroup is a group of directories used by an entity for block
I don't understand why you had to move DataDirGroup back to the header. The 
only usage I see is as a value in TabletDataDirGroupMap; can't that be forward 
declared?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 14:

(1 comment)

Looks good, just two other things:
1. What was your conclusion re: bit flipping in the test?
2. Let's continue the comment thread about ReadBlock() when ptr.size() < 4.

http://gerrit.cloudera.org:8080/#/c/6630/14/docs/design-docs/cfile.md
File docs/design-docs/cfile.md:

Line 27: Header
Can you update this to reflect that the checksums are optional (and maybe 
describe the cflags that control them a little)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-1826 add propagated timestamp to sync client

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: WIP KUDU-1826 add propagated timestamp to sync client
..


Patch Set 1:

(1 comment)

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

PS1, Line 62:   public long getLastPropagatedTimestamp() {
> docs
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbfeb9526423f5acd0ef6482869d36cdd146e921
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-1826 add propagated timestamp to sync client

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-1826 add propagated timestamp to sync client
..

WIP KUDU-1826 add propagated timestamp to sync client

Updating and getting the propagated timestamp only exist as functions
of the AsyncKuduClient, but these functions are still useful to the
KuduClient.

This patch exposes AsyncKuduClient's updateLastPropagatedTimestamp()
and getLastPropagatedTimestamp() to the KuduClient.

Marked WIP since I haven't done any testing.

Change-Id: Ibbfeb9526423f5acd0ef6482869d36cdd146e921
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
2 files changed, 26 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibbfeb9526423f5acd0ef6482869d36cdd146e921
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) KUDU-1993: fixed validation of 'grouped' gflags

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1993: fixed validation of 'grouped' gflags
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) KUDU-1993: fixed validation of 'grouped' gflags

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1993: fixed validation of 'grouped' gflags
..


KUDU-1993: fixed validation of 'grouped' gflags

Added generic implementation for grouped gflags validators.

Use CUSTOM_FLAG_VALIDATOR() macro to register late-phase validator
function for command-line flags. The validation is performed upon call
of HandleCommonFlags() invoked by the top-level ParseCommandLineFlags()
function.

Added unit test to cover the new functionality.

Updated validators for security-related RPC and embedded webserver
flags.

As a workaround for LSAN reports on the leaks in case of ASAN build,
added two scoped leak check disablers into CreateAndStartTimeoutThread.
That does not seem harmful or hiding any potential leaks since it
affects only the timeout thread itself.  Opened separate JIRA to track
the (false positive?) leak warning: KUDU-1995.

Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Reviewed-on: http://gerrit.cloudera.org:8080/6795
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3)
Reviewed-on: http://gerrit.cloudera.org:8080/6806
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/rpc/messenger.cc
M src/kudu/server/webserver_options.cc
M src/kudu/util/CMakeLists.txt
A src/kudu/util/flag_validators-test.cc
A src/kudu/util/flag_validators.cc
A src/kudu/util/flag_validators.h
M src/kudu/util/flags.cc
M src/kudu/util/test_main.cc
8 files changed, 415 insertions(+), 36 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured. An unsafe
flag 'allow_unauthenticated_public_connections' is provided to enable
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 131 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured. An unsafe
flag 'allow_unauthenticated_public_connections' is provided to enable
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 135 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured. An unsafe
flag 'allow_unauthenticated_public_connections' is provided to enable
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 138 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [util] use emplace() instead of insert()

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [util] use emplace() instead of insert()
..


Patch Set 1: Verified+1

Unrelated flake: ClientTest.TestFailedDnsResolution

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [util] use emplace() instead of insert()

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [util] use emplace() instead of insert()
..

[util] use emplace() instead of insert()

Since we are using C++11-compiant compiler, it's possible to avoid
unnecessary copying and use emplace() instead of insert() for dictionary
and hash containers.

Change-Id: Ic40b17d9315d7c42d589fd30ce55242540e7767f
---
M src/kudu/gutil/map-util.h
1 file changed, 10 insertions(+), 18 deletions(-)


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

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