[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

(3 comments)

Thanks for the review David. I updated the rendered HTML.

http://gerrit.cloudera.org:8080/#/c/3398/1/_posts/2016-06-17-raft-consensus-single-node.md
File _posts/2016-06-17-raft-consensus-single-node.md:

Line 17: cluster's existing master server replication factor from 1 to 3 (or 5).
> this might confuse people. its perfectly legal to increase to 2 right (just
Done


Line 71: in the future. Kudu fits this use case. When deploying Kudu, someone 
may wish
> what use case? "Kudu fits..." sounds wrong to me here. I'd remove it
Changed the wording. I don't want people to think they have a choice.


PS1, Line 78: two
> you use words for the numbers here but use the actual numbers in the 2nd pa
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Remove the LocalConsensus implementation

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..


Patch Set 5: Verified+1

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

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

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3398/1/_posts/2016-06-17-raft-consensus-single-node.md
File _posts/2016-06-17-raft-consensus-single-node.md:

Line 17: cluster's existing master server replication factor from 1 to 3 (or 5).
this might confuse people. its perfectly legal to increase to 2 right (just no 
fault tolerance) just mention from 1 to multiple nodes or something.


Line 71: in the future. Kudu fits this use case. When deploying Kudu, someone 
may wish
what use case? "Kudu fits..." sounds wrong to me here. I'd remove it


PS1, Line 78: two
you use words for the numbers here but use the actual numbers in the 2nd 
paragraph


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

(3 comments)

Thanks for the review JD. I made a couple more tweaks and updated the rendered 
HTML as well.

http://gerrit.cloudera.org:8080/#/c/3398/1/_posts/2016-06-17-raft-consensus-single-node.md
File _posts/2016-06-17-raft-consensus-single-node.md:

PS1, Line 15: even
> nit: I'd remove that word.
Done


PS1, Line 59: strong
> Is that normal parlance in Raft?
When talking about consensus algorithms, I've heard people say "Raft has a 
strong leader" but I'm explaining it in the next paragraph so no need to bring 
it up


PS1, Line 75: a Raft that
> Feels like you're missing a word, maybe a "Raft implementation"? Or "Withou
Yes, thanks for the catch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Mike Percy (Code Review)]

Mike Percy has uploaded a new patch set (#2).

Change subject: Add blog post about removing LocalConsensus
..

Add blog post about removing LocalConsensus

Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
---
A _posts/2016-06-17-raft-consensus-single-node.md
1 file changed, 92 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Reformat raft-config-change.md to clean it up

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Reformat raft-config-change.md to clean it up
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/1851/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Reformat raft-config-change.md to clean it up

[Mike Percy (Code Review)]

Mike Percy has uploaded a new patch set (#2).

Change subject: Reformat raft-config-change.md to clean it up
..

Reformat raft-config-change.md to clean it up

It just needed a little bit of markdown syntax love.

Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
---
M docs/design-docs/raft-config-change.md
1 file changed, 61 insertions(+), 88 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Reformat raft-config-change.md to clean it up

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Reformat raft-config-change.md to clean it up
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1850/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Reformat raft-config-change.md to clean it up

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Reformat raft-config-change.md to clean it up
..


Patch Set 1:

Rendered HTML: 
https://github.com/mpercy/kudu/blob/consensus-docs-4/docs/design-docs/raft-config-change.md

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
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-HasComments: No

[kudu-CR] Reformat raft-config-change.md to clean it up

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Reformat raft-config-change.md to clean it up
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1849/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacbd37b8b22ae02dba6d3fd2c389dd97eafe61ed
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

(3 comments)

I really enjoyed reading this, just minor nits.

http://gerrit.cloudera.org:8080/#/c/3398/1/_posts/2016-06-17-raft-consensus-single-node.md
File _posts/2016-06-17-raft-consensus-single-node.md:

PS1, Line 15: even
nit: I'd remove that word.


PS1, Line 59: strong
Is that normal parlance in Raft?


PS1, Line 75: a Raft that
Feels like you're missing a word, maybe a "Raft implementation"? Or "Without a 
consensus implementation that"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

Rendered: 
http://mpercy.github.io/kudu/2016/06/17/raft-consensus-single-node.html

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[Mike Percy (Code Review)]

Hello Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: Add blog post about removing LocalConsensus
..

Add blog post about removing LocalConsensus

Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
---
A _posts/2016-06-17-raft-consensus-single-node.md
1 file changed, 92 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 2:

Updated HTML: 
https://github.com/mpercy/kudu/blob/consensus-docs-3a/docs/design-docs/raft-remote-bootstrap.md

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add Raft remote bootstrap design doc

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/1848/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3395/2/docs/design-docs/raft-remote-bootstrap.md
File docs/design-docs/raft-remote-bootstrap.md:

PS2, Line 108:  `DOES_NOT_EXIST` or is
 : `DELETED`
> This highlighting looks good for states in the state machine. Could you add
Done


Line 187: 2. mkdir quarantine directory (QDIR).
> Nit: none of the other UNIX-y operations in this list are referenced by the
Done


Line 257: 1. Master sends an AddServer() RPC to the leader of the tablet to add 
a new
> Nit: a singleton list is just noise, could you remove the "1." prefix?
Done


PS2, Line 260:a. This AddServer() RPC will specify the prior committed raft 
config for the
 :   tablet to ensure that the request is idempotent.
> Unfortunately, GH markdown requires that sublists be numeric too. Otherwise
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add Raft remote bootstrap design doc
..

Add Raft remote bootstrap design doc

This was ported over from a Google doc

Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
---
M docs/design-docs/README.md
A docs/design-docs/raft-remote-bootstrap.md
2 files changed, 344 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/1846/

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

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

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/consensus/consensus.h
File src/kudu/consensus/consensus.h:

Line 35: 
> remove extra line
Done


PS4, Line 136: WaitUntilLeader
> rename this? append ForTests
Done


http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/tserver/ts_tablet_manager-test.cc
File src/kudu/tserver/ts_tablet_manager-test.cc:

Line 89: 
RETURN_NOT_OK(tablet_peer->consensus()->WaitUntilLeader(MonoDelta::FromSeconds(10)));
> can just return here
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72e94d7e78e3428a8d9696737e07b8e8f7489d49
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Remove the LocalConsensus implementation

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/1847/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..

Use RaftConsensus instead of LocalConsensus in tests

This paves the way to remove the LocalConsensus implementation.

Also add WaitUntilLeader() to Consensus interface for use by tests.

Changes the following tests to use RaftConsensus:

* tablet_peer-test
* remote_bootstrap_client-test
* remote_bootstrap_session-test
* tablet_server-test
* ts_tablet_manager-test

Change-Id: I72e94d7e78e3428a8d9696737e07b8e8f7489d49
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/local_consensus.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/remote_bootstrap-test-base.h
M src/kudu/tserver/remote_bootstrap_client-test.cc
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
13 files changed, 68 insertions(+), 39 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72e94d7e78e3428a8d9696737e07b8e8f7489d49
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Remove the LocalConsensus implementation

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

Line 89:   // Obsolete. This parameter has been retired.
> why are we keeping this around?
We need to provide some way to specify that field #2 should not be reused. 
Since our version of protobuf apparently doesn't support the "reserved" 
keyword, and comments aren't enforced by the compiler, this seems to be the 
best option.


http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

Line 255: CHECK_EQ(server_->instance_pb().permanent_uuid(), 
config.peers(0).permanent_uuid());
> can we change this to make it generic, i.e. make sure that our uuid is amon
Done


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

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

[kudu-CR] Link to raft config change design from design-docs index page

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: Link to raft config change design from design-docs index page
..


Link to raft config change design from design-docs index page

Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
Reviewed-on: http://gerrit.cloudera.org:8080/3394
Reviewed-by: Adar Dembo 
Tested-by: Mike Percy 
---
M docs/design-docs/README.md
1 file changed, 1 insertion(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Link to raft config change design from design-docs index page

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Link to raft config change design from design-docs index page
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
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-HasComments: No

[kudu-CR] Add Raft remote bootstrap design doc

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 2:

(4 comments)

Looks good, just a few formatting nits.

http://gerrit.cloudera.org:8080/#/c/3395/2/docs/design-docs/raft-remote-bootstrap.md
File docs/design-docs/raft-remote-bootstrap.md:

PS2, Line 108:  `DOES_NOT_EXIST` or is
 : `DELETED`
This highlighting looks good for states in the state machine. Could you add it 
to other references to DOES_NOT_EXIST or DELETED elsewhere in the doc, provided 
they're not in a verbatim section?

Would be good for other state names too, like COPYING.


Line 187: 2. mkdir quarantine directory (QDIR).
Nit: none of the other UNIX-y operations in this list are referenced by their 
UNIX-y name, so can you change this to "Create the quarantine directory (QDIR)"?


Line 257: 1. Master sends an AddServer() RPC to the leader of the tablet to add 
a new
Nit: a singleton list is just noise, could you remove the "1." prefix?


PS2, Line 260:a. This AddServer() RPC will specify the prior committed raft 
config for the
 :   tablet to ensure that the request is idempotent.
Unfortunately, GH markdown requires that sublists be numeric too. Otherwise it 
doesn't render them as a list.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Link to raft config change design from design-docs index page

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Link to raft config change design from design-docs index page
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 4: Code-Review+2

(1 comment)

Feel free to punt on the nit if you want to merge right now.

Also, run the deprecation by JD.

http://gerrit.cloudera.org:8080/#/c/3386/4/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 83: import java.util.concurrent.*;
Nit: personally this feels a little strange outside of tests. If there aren't a 
lot of imports, would you mind expanding this?


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

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

[kudu-CR] Add more helpful CHECK message at master startup

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: Add more helpful CHECK message at master startup
..


Add more helpful CHECK message at master startup

Maybe this could help debug KUDU-1488 a little bit

Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Reviewed-on: http://gerrit.cloudera.org:8080/3396
Reviewed-by: Jean-Daniel Cryans
Tested-by: Mike Percy 
---
M src/kudu/master/sys_catalog.cc
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Mike Percy: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Add more helpful CHECK message at master startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add more helpful CHECK message at master startup
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add more helpful CHECK message at master startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add more helpful CHECK message at master startup
..


Patch Set 1:

Jenkins in borken so +1ing myself

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: RaftConsensus: Trigger election at startup if single node
..


RaftConsensus: Trigger election at startup if single node

If a tablet's replication factor is 1 then don't wait for the failure
detector to kick in. Simply kick off an election immediately.

Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Reviewed-on: http://gerrit.cloudera.org:8080/3344
Reviewed-by: David Ribeiro Alves 
Tested-by: Mike Percy 
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/integration-tests/raft_consensus-itest.cc
3 files changed, 28 insertions(+), 3 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Mike Percy: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Add more helpful CHECK message at master startup

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Add more helpful CHECK message at master startup
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add more helpful CHECK message at master startup

[Mike Percy (Code Review)]

Hello Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: Add more helpful CHECK message at master startup
..

Add more helpful CHECK message at master startup

Maybe this could help debug KUDU-1488 a little bit

Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
---
M src/kudu/master/sys_catalog.cc
1 file changed, 3 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I86b5c9d9553b251c289b51bddfe3f84beaa489e9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 2:

See the rendered page at 
https://github.com/mpercy/kudu/blob/consensus-docs-3/docs/design-docs/raft-remote-bootstrap.md

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Mike Percy has uploaded a new patch set (#2).

Change subject: Add Raft remote bootstrap design doc
..

Add Raft remote bootstrap design doc

This was ported over from a Google doc

Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
---
M docs/design-docs/README.md
A docs/design-docs/raft-remote-bootstrap.md
2 files changed, 343 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Add Raft remote bootstrap design doc

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1844/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add Raft remote bootstrap design doc

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add Raft remote bootstrap design doc
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1843/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Link to raft config change design from design-docs index page

[Mike Percy (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.

Change subject: Link to raft config change design from design-docs index page
..

Link to raft config change design from design-docs index page

Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
---
M docs/design-docs/README.md
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0f8bc5874cdf8814053ac9b80116325ac1d3ebf3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] Add Raft remote bootstrap design doc

[Mike Percy (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.

Change subject: Add Raft remote bootstrap design doc
..

Add Raft remote bootstrap design doc

This was ported over from a Google doc

Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
---
M docs/design-docs/README.md
A docs/design-docs/raft-remote-bootstrap.md
2 files changed, 346 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3551e8fcd19628cfbeb25f822a403155f8ba2c28
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

Line 89:   // Obsolete. This parameter has been retired.
why are we keeping this around?


http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

Line 255: CHECK_EQ(server_->instance_pb().permanent_uuid(), 
config.peers(0).permanent_uuid());
can we change this to make it generic, i.e. make sure that our uuid is among 
the uuids of all peers?


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

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

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/consensus/consensus.h
File src/kudu/consensus/consensus.h:

Line 35: 
remove extra line


PS4, Line 136: WaitUntilLeader
rename this? append ForTests


http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/tserver/ts_tablet_manager-test.cc
File src/kudu/tserver/ts_tablet_manager-test.cc:

Line 89: 
RETURN_NOT_OK(tablet_peer->consensus()->WaitUntilLeader(MonoDelta::FromSeconds(10)));
can just return here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72e94d7e78e3428a8d9696737e07b8e8f7489d49
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] docs: informal design for handling permanent master failures

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: docs: informal design for handling permanent master failures
..


Patch Set 1:

(15 comments)

This is sounding more formal, let's just call it a real design doc :)

I added some suggestions to make it easier for others to review and read, IMHO.

I also added some comments about the design.

http://gerrit.cloudera.org:8080/#/c/3393/1/docs/design-docs/master-perm-failure-1.0.md
File docs/design-docs/master-perm-failure-1.0.md:

Line 19: As part of Kudu's upcoming 1.0 release, we've been working towards 
improving
Too wordy. How about replacing this paragraph with:

This document discusses one of the remaining gaps in Kudu's support for 
multi-master operation: adding and removing masters.

Regarding the transient vs permanent thing, adding / removing masters has 
nothing to do with transient failures. Maybe I'm assuming too much but when I 
read this doc I would assume that is obvious. If it's not, then maybe it's 
worth a single sentence: "Transient failures are handled by the normal Raft 
leader election mechanisms."


Line 26: Let's assume we have a healthy Raft configuration consisting of three
I think we should remove this paragraph entirely. It has nothing to do with the 
purpose of this design doc.

If you want to keep it, how about add a section header along the lines of 
"Handling transient failures" or something, so people can just skip it.


Line 38: The most important question is: should we build permanent failure 
handling into
Maybe preface this with a section header "Manual repair when data from a downed 
node is still available". We still haven't gotten to the actual design part yet.


Line 64: ## Proposal
How about "Design proposal for dynamically adding / removing masters"


Line 65: 
Before we get to the algorithm, this needs an intro to summarize the proposal 
in English. along the lines of:

We will reuse most of the configuration change and remote bootstrap design and 
implementation from the tablet servers. The differences are:

1) Adding and removing masters is not performed automatically. An administrator 
must trigger those actions with some tool.
2) Administrators must also modify command line flags on tablet servers when a 
master is replaced, so that the tablet servers can find the new master.
3) Masters will decide whether to self-initialize a new configuration or join 
an existing one based on their command-line flags and their consensus metadata 
files (discussed below in Master directory management, phase 2)


Line 66: Here is the sequence of events:
I think "algorithm" would be a better name for this


Line 95: While we believe this approach is correct, we would like more eyes on 
it to
Since this is now basically a formal design doc, we can assume more eyes on it 
is a given. Let's replace this whole paragraph with the following:

In order to implement the above design, we'll need to make the following 
changes:


PS1, Line 119: mitigate the above confusion
how about "To avoid the above inconsistency in the semantics of the 
--master_addresses gflag,"


Line 121: disk. Then, we can remove **--master_addresses** and use a new 
kudu-admin
"remove --master_addresses gflag from the kudu-master processes" ... maybe some 
wording like this will clarify that we're not talking about tservers here, for 
anybody who's lost track of that


Line 126: started in normal mode, it creates a Raft config of one (just itself) 
and
This mode stuff is quite confusing. I'm leaning toward implementing a totally 
manual process, including a "format master" command to "spawn" our first master 
server(s).

So basically if a master starts up, and it has an existing master tablet (with 
assumedly a consensus metadata file) then it starts up in normal mode. If it 
doesn't, it starts up in listening mode.


Line 133: 1. There’s a certain desire to completely remove **last_known_addr** 
from
s/There's a certain desire/It's useful on tablet servers/


Line 134:**RaftPeerPB** (as only UUIDs should be necessary for identifying 
members of
only UUIDs should be necessary, and this makes it easier for tablet servers to 
change IPs and ports transparently


Line 153: the semantics of **--tserver_master_addrs** become confusing just like
s/become/becomes/


Line 156: removed. To alleviate the confusion we could do the same thing we did 
for
I think this process is overly onerous. It also makes it possible to "orphan" a 
tserver with no way back that is covered by this spec.

How about we just say that --tserver_master_addrs is a pool of potential 
masters, tservers don't store anything about the masters on disk, and it can be 
modified in memory by the above mechanisms (i.e. ListMasters()) but if you want 
additional potential masters to be known at boot time then they have to be in 
the gflags.

Well, maybe the tserver should store one thing on disk: the sequence or OpId of 

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 3: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/1841/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I78b5d400778547a9ee090111663677901dbadd98
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/1840/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: RaftConsensus: Trigger election at startup if single node
..

RaftConsensus: Trigger election at startup if single node

If a tablet's replication factor is 1 then don't wait for the failure
detector to kick in. Simply kick off an election immediately.

Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/integration-tests/raft_consensus-itest.cc
3 files changed, 28 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3344/4/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

PS4, Line 293: tablet
> no need to mention "tablet", i.e. "...Raft configuration."
Done


Line 294:   Status IsSingleVoterConfig(bool* only_voter) const;
> you use "single" here and "only" in the param, can you refactor the param t
Done


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

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

[kudu-CR] docs: informal design for handling permanent master failures

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: docs: informal design for handling permanent master failures
..


Patch Set 1:

Rendered content available here: 
https://github.com/adembo/kudu/blob/config_change/docs/design-docs/master-perm-failure-1.0.md.

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

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

[kudu-CR] docs: informal design for handling permanent master failures

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Mike Percy, Todd Lipcon,

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

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

to review the following change.

Change subject: docs: informal design for handling permanent master failures
..

docs: informal design for handling permanent master failures

Here's an informal design doc that describes how we might address permanent
master failures. It presents a hacky way to handle some permanent failures
without implementing full master config change, then describes how config
change might be implemented.

The most important question I'd like to discuss first is: should master
config change support be implemented? As the doc describes (though not in
great detail), config change would be expensive to implement, and we're
running out of time for 1.0. We might decide that permanent failure is out
of scope for 1.0 (or at least permanent failures that also take out the
disk), and push config change out to a different release. If we do that, we
could handle migration of one node to three nodes with a script, or not at
all (i.e. this is beta software).

Change-Id: I2f05c319c89cf37e2d71fdc4b7ec951b2932a2b2
---
M docs/design-docs/README.md
A docs/design-docs/master-perm-failure-1.0.md
2 files changed, 170 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f05c319c89cf37e2d71fdc4b7ec951b2932a2b2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3386/2//COMMIT_MSG
Commit Message:

PS2, Line 9: This simplifies and improves the performance of 
AsyncKuduClient::locateTable by
   : using the client's existing tablet cache.
> Nit: there's not enough context in this patch to know why you're making thi
Done


http://gerrit.cloudera.org:8080/#/c/3386/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1230:   private final class MasterLookupCB implements Callback {
> Nit: too long now? Unclear.
Done


Line 2047: List getReplicas() {
> Quickly looking through the code, looks like we don't do it, and I'm not su
Immutability should be the default IMO.


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

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

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/1837/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I78b5d400778547a9ee090111663677901dbadd98
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Dan Burkert (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: [java-client] use tablet cache for locateTablet calls
..

[java-client] use tablet cache for locateTablet calls

This simplifies and improves the performance of AsyncKuduClient::locateTable by
using the client's existing tablet cache. The semantics of locateTable are
changed slightly- the end key is now treated as an exclusive key instead of
inclusive. This change is motivated by an upcoming fix to the locate table
logic, which will be easier without the code duplication.

The public KuduTable::locateTable methods have been deprecated since they rely
on callers having knowledge of the private internals of Kudu's partitioning
implementation. Additionally, the methods have been changed to take an exclusive
end partition key instead of an inclusive end partition key. Whether the key was
inclusive or exclusive was previously undocumented, and the only known caller
(the internal ScanToken implementation) was using it as exclusive. Users of
these methods are encouraged to use the ScanToken API instead, since it's easier
to use and does not leak internal Kudu implementation details.

Change-Id: I78b5d400778547a9ee090111663677901dbadd98
---
M docs/release_notes.adoc
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/KuduTable.java
M java/kudu-client/src/main/java/org/kududb/client/LocatedTablet.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java
5 files changed, 131 insertions(+), 92 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78b5d400778547a9ee090111663677901dbadd98
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3344/4/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

PS4, Line 293: tablet
no need to mention "tablet", i.e. "...Raft configuration."


Line 294:   Status IsSingleVoterConfig(bool* only_voter) const;
you use "single" here and "only" in the param, can you refactor the param to be 
"single" too?


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

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

[kudu-CR] Add required Debian version to installation page

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Add required Debian version to installation page
..


Patch Set 1:

Ah, so we should probably hold of on this till we can verify that it works.  Or 
we should remove debian references from the page.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65cef323648d7934cb140046cf391d56867eeb56
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3386/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1876: private final List replicas = new 
ArrayList<>();
> I don't think the tradeoff is worth it. Maybe we should document this, but 
Makes sense to a degree, though bear in mind we're already doing DNS resolution 
with the lock held, and the overhead of a COW list copy is peanuts by 
comparison.

If we began doing the resolution outside the lock, then yeah, the hit would be 
more sizable.


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

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

[kudu-CR] KUDU-1309: [java client] support tables with non-covering partition-key ranges

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1309: [java client] support tables with non-covering 
partition-key ranges
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3388/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 576:   Deferred openScanner(final 
AsyncKuduScanner scanner) {
> Do we need to preserve this indirection due to class/method visibility? If 
True, this can be removed.


http://gerrit.cloudera.org:8080/#/c/3388/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

Line 279:   if (!operationsInLookup.isEmpty()) {
> Deferreds are weird, man. I've read the code in the two branches here over 
Yeah it's a tricky one. A lot of what's happening is implied.


http://gerrit.cloudera.org:8080/#/c/3388/2/java/kudu-client/src/main/java/org/kududb/client/RowResultIterator.java
File java/kudu-client/src/main/java/org/kududb/client/RowResultIterator.java:

Line 35:   private static final RowResultIterator EMPTY = new 
RowResultIterator(0, null, null, null, null);
> If this is for tests only, perhaps make it more visible, annotate it with @
It's not, we return it if your end key on the scan is in non-covered range. We 
could document this.


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

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

[kudu-CR] [java-client] use tablet cache for locateTablet calls

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3386/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1876: private final List replicas = new 
ArrayList<>();
> Would it be safe to use a CopyOnWriteArrayList here, and in doing so avoid 
I don't think the tradeoff is worth it. Maybe we should document this, but 
replicas here would only be used when getting a list of locations. It's not a 
common thing to do, and it's not typically done in conjunction with normal 
inserts/scans.

Using CopyOnWriteArrayList means taking a hit under the lock while refreshing 
the tablet clients, which happens a lot more often than getting the replicas.


Line 2047: List getReplicas() {
> Nit: may want to name it "getImmutableReplicas" so that callers know they c
Quickly looking through the code, looks like we don't do it, and I'm not sure 
we even have occasions to do it. I like it too.


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

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