[kudu-CR] KUDU-2042 fix flakiness in raft consensus-itest

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2042 fix flakiness in raft_consensus-itest
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7227/3/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

Line 294:   RETURN_NOT_OK(ets->Pause());
> I think these should either stay as CHECK or have unique PREPENDs so it's e
Done


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

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

[kudu-CR] KUDU-2042 fix flakiness in raft consensus-itest

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2042 fix flakiness in raft_consensus-itest
..

KUDU-2042 fix flakiness in raft_consensus-itest

Fixed flakiness in the raft_consensus-itest test exposed by recent
change 86116e4c515a9c89e728dd699decaf20d097edac.

The source of the flakiness was the race between possible leader
election and calls of GetLeaderReplicaWithRetries() and
GetOnlyLiveFollowerReplicas() methods.

DEBUG build, run with --cpu-stress-threads=8 before the fix:
  http://dist-test.cloudera.org//job?job_id=aserbin.1498102286.18617
13 out of 2048 failed (~0.6% failure rate)

DEBUG build, run with --cpu-stress-threads=8, after the fix:
  http://dist-test.cloudera.org//job?job_id=aserbin.1498103715.1697
0  out of 2048 failed (~0.0% failure rate)

Change-Id: Ia5fb2b82afb15b50659d068ef83d11b7cc291ca9
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 75 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5fb2b82afb15b50659d068ef83d11b7cc291ca9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2042 fix flakiness in raft consensus-itest

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-2042 fix flakiness in raft_consensus-itest
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7227/3/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

Line 294:   RETURN_NOT_OK(ets->Pause());
I think these should either stay as CHECK or have unique PREPENDs so it's 
easier to diagnose when the test fails


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

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

[kudu-CR] KUDU-2042 fix flakiness in raft consensus-itest

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2042 fix flakiness in raft_consensus-itest
..

KUDU-2042 fix flakiness in raft_consensus-itest

Fixed flakiness in the raft_consensus-itest test exposed by recent
change 86116e4c515a9c89e728dd699decaf20d097edac.

The source of the flakiness was the race between possible leader
election and calls of GetLeaderReplicaWithRetries() and
GetOnlyLiveFollowerReplicas() methods.

DEBUG build, run with --cpu-stress-threads=8 before the fix:
  http://dist-test.cloudera.org//job?job_id=aserbin.1498102286.18617
13 out of 2048 failed (~0.6% failure rate)

DEBUG build, run with --cpu-stress-threads=8, after the fix:
  http://dist-test.cloudera.org//job?job_id=aserbin.1498103715.1697
0  out of 2048 failed (~0.0% failure rate)

Change-Id: Ia5fb2b82afb15b50659d068ef83d11b7cc291ca9
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 86 insertions(+), 39 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5fb2b82afb15b50659d068ef83d11b7cc291ca9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2042 fix flakiness in raft consensus-itest

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2042 fix flakiness in raft_consensus-itest
..

KUDU-2042 fix flakiness in raft_consensus-itest

Fixed flakiness in the raft_consensus-itest test exposed by recent
change 86116e4c515a9c89e728dd699decaf20d097edac.

The source of the flakiness was the race between possible leader
election and calls of GetLeaderReplicaWithRetries() and
GetOnlyLiveFollowerReplicas() methods.

Change-Id: Ia5fb2b82afb15b50659d068ef83d11b7cc291ca9
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
2 files changed, 86 insertions(+), 39 deletions(-)


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

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

[kudu-CR] disk failure: coordinate error handling

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: disk failure: coordinate error handling
..

disk failure: coordinate error handling

I/O errors are spawned when calls to env functions fail. The response to
these errors may vary and may span many layers of abstraction.

For instance, if an EIO were triggered while writing a block, one
response might be to shut down the tablet whose operation just failed.
In order to do this without messing up the layering, a new entity, the
FsErrorManager is added to coordinate error handling.

Callbacks are registered with the FsErrorManager as responses to errors,
and callers to at-risk functions trigger these callbacks as errors are
spawned.

This patch includes the FsErrorManager and its placement w.r.t. the
BlockManager, FsManager, TSTabletManager, etc. Actual handling and
testing will come in a later patch.

Change-Id: Ib3169deada702c527b70fabf9d2223364f9a9ea6
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.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-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/tablet/mvcc.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
14 files changed, 231 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib3169deada702c527b70fabf9d2223364f9a9ea6
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP disk failure: coordinate disk failure handling

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP disk failure: coordinate disk failure handling
..

WIP disk failure: coordinate disk failure handling

This patch adds the logic required to prevent a crash on disk failure.
Checks that previously depended on successful disk IO now yield if the
returned status' POSIX code matches one corresponding to disk failure.

For the most part, failure handling is done by the lowest abstraction to
touch disk: blocks and containers.

A set of basic tests are added in ts_disk_failure-itest.

TODO:
- crash if tablet metadata dir is bad
- don't crash if a disk can't be read at startup and we try to
  CheckIntegrity, but it's missing

Change-Id: Ia03bfb711a1b022d7516f4adb37fe9fb28ec949c
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs-test-util.h
M src/kudu/fs/fs_manager.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
A src/kudu/tserver/ts_disk_failure-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/util/status.h
35 files changed, 488 insertions(+), 80 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia03bfb711a1b022d7516f4adb37fe9fb28ec949c
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: Add gradle build

[Grant Henke (Code Review)]

Grant Henke has uploaded a new change for review.

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

Change subject: WIP: Add gradle build
..

WIP: Add gradle build

Adds an experimental Gradle build that results in similar
tasks and artifacts as the existing maven build. See the
readme for usage and common commands.

The build is broken out by module with common configurations
in the root build.gradle file. Additionally the gradle
directory contains “drop in” scripts containing functional
pieces of the build configuration to simplify sharing
configurations and the unclutter the main build files.

Includes support for:
- Shaded dependencies that can be used across modules
- Running unit tests before integration tests
- Reporting available dependency updates
- Protobuf and Avro code generation
- Publishing to remote and local maven repositories

Remaining Work:
- Partially shade kudu-client (async and slf4j)
- Validate and adjust pom contents
- Investigate test failures
- Other inline TODO comments

Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
---
M java/README.md
A java/build.gradle
A java/gradle.properties
A java/gradle/artifacts.gradle
A java/gradle/buildscript.gradle
A java/gradle/compile.gradle
A java/gradle/dependencies.gradle
A java/gradle/protobuf.gradle
A java/gradle/publishing.gradle
A java/gradle/quality.gradle
A java/gradle/scopes.gradle
A java/gradle/shadow.gradle
A java/gradle/tests.gradle
A java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.jar
A java/gradle/wrapper/gradle-wrapper.properties
A java/gradlew
A java/gradlew.bat
A java/interface-annotations/build.gradle
A java/kudu-client-tools/build.gradle
A java/kudu-client/build.gradle
A java/kudu-flume-sink/build.gradle
A java/kudu-jepsen/build.gradle
A java/kudu-mapreduce/build.gradle
A java/kudu-spark-tools/build.gradle
R 
java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ITBigLinkedList.scala
A java/kudu-spark/build.gradle
A java/settings.gradle
28 files changed, 984 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 

[kudu-CR] KUDU-1955 refuse to use world-readable keytabs

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1955 refuse to use world-readable keytabs
..


Patch Set 2:

(1 comment)

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

Line 273:   
RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
> Okay, do you think I should move the keytab file check to a validator as we
Yah, if possible we prefer to put validation like this in an explicit gflags 
validator function.  It runs earlier in server startup, which is usually good.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] threadpool: token-based task sequencing

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: threadpool: token-based task sequencing
..


Patch Set 12:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6874/12/src/kudu/util/threadpool.cc
File src/kudu/util/threadpool.cc:

PS12, Line 134: m
> Is this different than pool_->lock_ ?
It's not. I should just reference that directly to avoid confusion.


Line 196:   while (state() != State::QUIESCED) {
> Could use a comment indicating who is responsible for transitioning the tok
Done


http://gerrit.cloudera.org:8080/#/c/6874/12/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:

PS12, Line 410: m
> Would be nice to mention what 'm' is for.
Moot now that pool->lock is referenced directly.


-- 
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: 12
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 
Gerrit-HasComments: Yes

[kudu-CR] [java] Replace interface-annotations with Apache Yetus annotations

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java] Replace interface-annotations with Apache Yetus 
annotations
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7235/6//COMMIT_MSG
Commit Message:

Could you add some notes here about the backwards compatibility implications? I 
think we discussed this on Slack but it would be good to capture it in the git 
commit too.


http://gerrit.cloudera.org:8080/#/c/7235/6/java/kudu-client/pom.xml
File java/kudu-client/pom.xml:

Line 37: compile
Isn't this the default scope? Can it be omitted?


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

Line 104: compile
Likewise.


http://gerrit.cloudera.org:8080/#/c/7235/6/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:

Line 68: compile
And here too.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860e59f497d96f507d5a2b0ea517f9f8d0794dac
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 38 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1955 refuse to use world-readable keytabs

[Sam Okrent (Code Review)]

Sam Okrent has posted comments on this change.

Change subject: KUDU-1955 refuse to use world-readable keytabs
..


Patch Set 2:

(5 comments)

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

Line 273:   
RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
> did you try adding this check to the ValidateExternalPkiFlags method?  I th
Okay, do you think I should move the keytab file check to a validator as well?


Line 274:   
_readable_private_key));
> alignment
Done


Line 276: return Status::InvalidArgument(Substitute(
> Same comment about InvalidArgument as in the other check.
Done


http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/security/init.cc
File src/kudu/security/init.cc:

Line 455: return Status::InvalidArgument(Substitute(
> Status::InvalidArgument can take an optional second string parameter which 
Done


http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

PS2, Line 1564: OVERRIDE
> use lowercase 'override'
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1955 refuse to use world-readable keytabs

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1955 refuse to use world-readable keytabs
..


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:

Line 232: TEST_F(SecurityITest, TestWorldReadableKeytab) {
Can you add a test for the cert case as well?


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

Line 273:   
RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
did you try adding this check to the ValidateExternalPkiFlags method?  I think 
it would be better there if possible.


Line 274:   
_readable_private_key));
alignment


Line 276: return Status::InvalidArgument(Substitute(
Same comment about InvalidArgument as in the other check.


http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/security/init.cc
File src/kudu/security/init.cc:

Line 455: return Status::InvalidArgument(Substitute(
Status::InvalidArgument can take an optional second string parameter which is 
equivalent to joining with ': ', so you can drop the Substitute call and just 
pass the string and flag:

return Status::InvalidArgument(
"cannot use keytab file with world-readable permissions: $0",
FLAGS_keytab_file);


http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env.h
File src/kudu/util/env.h:

PS2, Line 333: O
capitalization


http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

PS2, Line 1564: OVERRIDE
use lowercase 'override'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

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

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 38 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 2: Code-Review+1

Failure doesn't look related.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: Fix gmock link errors on macOS
..


Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:

/Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib
 (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
Reason: image not found
fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or 
breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Reviewed-on: http://gerrit.cloudera.org:8080/7251
Reviewed-by: Alexey Serbin 
Reviewed-by: Dan Burkert 
Tested-by: Dan Burkert 
---
M thirdparty/build-definitions.sh
1 file changed, 10 insertions(+), 2 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, but someone else must approve; Verified
  Alexey Serbin: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 2:

right, that's a fair assesment

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 2: Code-Review+2

If f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5 caused some regression and we have 
no time to investigate, let's merge this change.  and this patch is kind of 
rolling back of that. f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5 

LGTM -- this patch is kind of rolling back of the corresponding modifications 
introduced in f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 2:

I tried applying the patch here: https://github.com/google/googletest/pull/829, 
but that had no effect.  I believe the two things you mentioned are equivalent. 
 I can't spend any more time on this issue, so I'm going to suggest we merge as 
is and not investigate more.  The rsync install method has worked reliably for 
the entire life of the project to date.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Fix gmock link errors on macOS
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7251/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS1, Line 339:   echo Installing gmock...
 :   cp -a $GMOCK_SHARED_BDIR/libgmock.$DYLIB_SUFFIX $PREFIX/lib/
 :   cp -a $GMOCK_STATIC_BDIR/libgmock.a $PREFIX/lib/
 :   rsync -av $GMOCK_SOURCE/include/ $PREFIX/include/
 :   rsync -av $GMOCK_SOURCE/gtest/include/ $PREFIX/include/
So, playing with additional

-DCMAKE_MACOSX_RPATH=xxx

for the 'cmake' command above didn't help?

And CMP0042 didn't work either?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Fix gmock link errors on macOS
..

Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:

/Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib
 (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
Reason: image not found
fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or 
breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
---
M thirdparty/build-definitions.sh
1 file changed, 10 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix gmock link errors on macOS

[Dan Burkert (Code Review)]

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: Fix gmock link errors on macOS
..

Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:

/Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib
 (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 
125.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
Reason: image not found
fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or 
breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
---
M thirdparty/build-definitions.sh
1 file changed, 11 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 

[kudu-CR] WIP KUDU-1943: Add BlockTransaction to Block Manager

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: WIP KUDU-1943: Add BlockTransaction to Block Manager
..


Patch Set 4:

(16 comments)

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

PS3, Line 10: which happen
: in a logical op
> 'which happen in a logical operation'
Done


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/cfile/bloomfile.h
File src/kudu/cfile/bloomfile.h:

Line 47:   // Close the bloom's CFile, finalizing the underlying blocks and
> Update this comment.
Done


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/cfile/cfile_writer.h
File src/kudu/cfile/cfile_writer.h:

Line 109:   // Close the CFile. Finalize the underlying blocks and release
> Update this comment.
Done


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/fs/block_manager.h
File src/kudu/fs/block_manager.h:

Line 67: // Close() is an expensive operation, as it must flush both dirty 
block data
> This comment should be updated to explain Finalize.
Done


Line 142:   virtual size_t BytesAppended() const = 0;
> Add more info about what 'finalize' means in this context, either here or i
Done


Line 143: 
> Perhaps we could simplify this interface by combining 'FlushDataAsync' and 
yes, there is some cases the block is flushed but not finalized, you can call 
FlushDataAsync() and then Close(). Or it is finalized but not flushed, in cfile 
writer, if FLAGS_cfile_do_on_finish is 'nothing', the block will not be flushed.


Line 144:   virtual State state() const = 0;
> On the naming of 'FinalizeWrite' - the name is good, but I'd be in favor of
Done


Line 283:   // nor is the order guaranteed to be deterministic. Furthermore, if
> Update comment
Done


Line 305: STLDeleteElements(_blocks_);
> Hmm I know we discussed on slack (or the design doc?) why CommitWrites need
We discussed that in flush/compaction, superblock gets flushed in between the 
new rowsets get created and the old rowsets get GCed.  We do not want to delete 
blocks that should be kept, if superblock flush fails.


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

Line 429:   string metadata_path = path + 
LogBlockManager::kContainerMetadataFileSuffix;
> Is this needed?
Done


Line 915:   unique_ptr block;
> Should this be committing the writes?  Seems strange the previous version d
Looking at the purpose of the tests, the writes seem not necessarily need to be 
committed. So I keep it the way it was.


Line 1107:   unique_ptr block;
> Same here.
Same reason above.


http://gerrit.cloudera.org:8080/#/c/7207/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 24: #include 
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 274: 
> Any reason to prefer a set here to a vector?  Vectors are typically lighter
Was trying to only have unique block here. But given the way how this method 
gets called the blocks should all be unique, so will change to vector instead.


Line 367:   // the container data file's position, as round up could already be 
done via
> Can't this method internally determine whether the round up has already bee
Good point, but I feel it is a little bit harder than that, as we also have 
state as 'FLUSHING'.  Also, LogBlock does not have the state info.


http://gerrit.cloudera.org:8080/#/c/7207/3/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

Line 90:   // Closes the CFiles, finalizing the underlying blocks and releasing
> Update comment.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a4bd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-1943: Add BlockTransaction to Block Manager

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-1943: Add BlockTransaction to Block Manager
..

WIP KUDU-1943: Add BlockTransaction to Block Manager

This adds a new layer of abstraction 'BlockTransaction' to
Block Manager, to accumulate creates/deletes which happen
in a logical operation, e.g. flush/compaction. Avoid using
fsync() per block can potentionaly reduce a lot I/O overhead.

This patch also add a new API FinalizeWrite() to Block Manager,
so that for LBM container can be resued, without flushing
in-flight writable blocks to disk.

In this patch, it also intergates 'BlockTransaction' with
flush/compaction operation.

A design doc can be found here:
https://docs.google.com/a/cloudera.com/document/d/15zZaKeLENRGC_AdjF2N3hxtM_6_OuydXA1reWvDbbK0/edit?usp=sharing

Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a4bd
---
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/bloomfile.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
18 files changed, 407 insertions(+), 150 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a4bd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

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

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

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 current
authn token expires.  The client automatically retries the RPC that hits
the token expiration error (actually, the error to re-try is seen as
FATAL_INVALID_AUTHENTICATION_TOKEN sent by the server during connection
negotiation).

Added a couple 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
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, 596 insertions(+), 43 deletions(-)


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

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

[kudu-CR] [java-client] separating Connection

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new patch set (#13).

Change subject: [java-client] separating Connection
..

[java-client] separating Connection

This patch separates lower-level, connection-related functionality
from the TabletClient class into the new Connection class.
The 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,395 insertions(+), 1,266 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/7146/13
-- 
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: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1955 refuse to use world-readable keytabs

[Sam Okrent (Code Review)]

Sam Okrent has uploaded a new change for review.

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

Change subject: KUDU-1955 refuse to use world-readable keytabs
..

KUDU-1955 refuse to use world-readable keytabs

Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.

Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 67 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent 

[kudu-CR] Simplify OpId/Timestamp assignment

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Simplify OpId/Timestamp assignment
..

Simplify OpId/Timestamp assignment

There are a couple of constraints that are tricky to enforce and were
requiring us to have convoluted interactions on OpId/Timestamp assignment
and registration between the TimeManager, MvccManager and ConsensusQueue.

These convoluted interactions are currently preventing a clean and simple
leader leases implementation, particularly the single timestamp outstanding
in TimeManager.

The "tricky" constraints are:

i) Before clean time is advanced in mvcc, all transactions with a
timestamp lower than the current time must be registered with mvcc.
This allows to wait for a certain timestamp t to be "safe" with TimeManager
and then wait for all transactions with a timestamp t1 < t to commit, thus
making sure that a scan at t is repeatable.

ii) The leader can't advance follower safe timestamp to t before
all the operations with a timestamp lower than it have also been sent.
This is hard since timestamps are assigned outside of consensus.

In general it's problematic to generate OpIds non-atomically with
timestamps, since these are both supposed to be monotonically increasing
in unison. To address this we had some complex state and interactions between
the components. For instance this was requiring us to have TimeManager
stop safe time advancement until a transaction being prepared was
submitted to the queue, after assigning a timestamp in the
TransactionDriver. This was problematic because with would only allow at most
one assigned timestamp in-flight, meaning we were not able to extract the
current safe time at will.

This patch completely addresses ii), Timestamps and OpIds are now assigned
by PeerMessageQueue, atomically. This makes it trivial to make sure that the
safe time sent to replicas is inline with the sent messages. This also allows
some simplifications and refactorings of the PeerMessageQueue and TimeManager.

As for i) the way we enforce this constraint changed. Instead of registering
a transaction before submitting it to consensus (which we now can't since
consensus submission is also where we assign the timestamp) we register
another, dummy transaction on the MvccManager before submission and abort it
afterwards. This allows to "pin" safe time advancement down to a timestamp
that is mandatorily smaller than then one that will be assigned.

One side effect of this change is that, for new config changes, we won't
know the OpId until after the queue submission, which needs to happen
after actually setting the pending config. This is largely inconsequential
but did require some refactoring.

Change-Id: I2bedcc5c3708d06f48131f34ea14c835867b4800
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
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/log_cache-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
15 files changed, 247 insertions(+), 206 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2bedcc5c3708d06f48131f34ea14c835867b4800
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] disk failure: coordinate error handling

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: disk failure: coordinate error handling
..

disk failure: coordinate error handling

I/O errors are spawned when calls to env functions fail. The response to
these errors may vary and may span many layers of abstraction.

For instance, if an EIO were triggered while writing a block, one
response might be to shut down the tablet whose operation just failed.
In order to do this without messing up the layering, a new entity, the
FsErrorManager is added to coordinate error handling.

Callbacks are registered with the FsErrorManager as responses to errors,
and callers to at-risk functions trigger these callbacks as errors are
spawned.

This patch includes the FsErrorManager and its placement w.r.t. the
BlockManager, FsManager, TSTabletManager, etc. Actual handling and
testing will come in a later patch.

Change-Id: Ib3169deada702c527b70fabf9d2223364f9a9ea6
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.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-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
14 files changed, 260 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib3169deada702c527b70fabf9d2223364f9a9ea6
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon