[kudu-CR] Add a way to dump ResultTracker state

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a way to dump ResultTracker state
..

Add a way to dump ResultTracker state

This adds a way to dump result tracker state and makes sure we use it when
FATAling out on CHECK failures.

Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
---
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
2 files changed, 70 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a way to dump ResultTracker state

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a way to dump ResultTracker state
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 9: Verified+1

unrelated flake

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a way to dump ResultTracker state

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a way to dump ResultTracker state
..

Add a way to dump ResultTracker state

This adds a way to dump result tracker state and makes sure we use it when
FATAling out on CHECK failures.

Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
---
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
2 files changed, 70 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a way to dump ResultTracker state

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a way to dump ResultTracker state
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a way to dump ResultTracker state

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a way to dump ResultTracker state
..


Patch Set 4:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 19:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] Add information about Exactly Once RPC semantics to rpc.md

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Patch Set 5: Verified+1

unrelated java failure

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 22: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 22:

unrelated failure of LinkedListTest.TestLoadAndVerify

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

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

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 9:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a fault injection point after the leader sends a request

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a fault injection point _after_ the leader sends a request
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I504d74ac3ab3a8cb120d4e5ecee308d9846f3829
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 24:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a way to dump ResultTracker state

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#3).

Change subject: Add a way to dump ResultTracker state
..

Add a way to dump ResultTracker state

This adds a way to dump result tracker state and makes sure we use it when
FATAling out on CHECK failures.

Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
---
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
2 files changed, 70 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 597 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ToString() method to Proxy

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 8:

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

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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the ResultTracker into the rpc subsystem
..


Patch Set 22:

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

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

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

[kudu-CR] Integrate the request tracker with the client

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the request tracker with the client
..


Patch Set 21:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] c++ client: various fixes to DDL operations

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: c++ client: various fixes to DDL operations
..

c++ client: various fixes to DDL operations

1. Remove the num_attempts>1 workarounds found in some operations. The
   CreateTable one was actually broken (the PartitionSchema::FromPB() call
   triggered a CHECK) and this would be more robustly handled via the new
   "exactly once" semantics.
2. In SyncLeaderMasterRpc(), take an extra ref on the master proxy.
   Otherwise a concurrent master leader election could crash the client.
3. Add exponential backoff to SyncLeaderMasterRpc(). It's nothing fancy, but
   I was tired of the tight loops and log spew.

Change-Id: I09768240bd04cca95d95aefe17c34d276075125b
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
3 files changed, 29 insertions(+), 64 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I09768240bd04cca95d95aefe17c34d276075125b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 1:

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

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

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

[kudu-CR] master: fix corruption when AlterTable() races with CreateTable()

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: master: fix corruption when AlterTable() races with 
CreateTable()
..

master: fix corruption when AlterTable() races with CreateTable()

Admittedly, this is a contrived scenario:
1. T1 tries to create table with name 'foo'
2. T2 tries to rename table with name 'bar' to 'foo'

With just the right timing, both operations succeed and the metadata now has
two tables named 'foo', each with a different table ID. The fix is simple:
generalize the "tables being created" logic already used by CreateTable().

Without the fix, the new test failed every 50th run or so. With it, it
doesn't fail in 1000 runs.

Change-Id: I6c9e4214c09bc47a5a10b12d6ffe8b35906708c9
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
3 files changed, 231 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c9e4214c09bc47a5a10b12d6ffe8b35906708c9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] c++ client: various fixes to DDL operations

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: c++ client: various fixes to DDL operations
..


Patch Set 1:

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

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

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

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 1:

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

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

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

[kudu-CR] rw mutex: add configurable priority

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: rw_mutex: add configurable priority
..


Patch Set 1:

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

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

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

[kudu-CR] master-test: rewrite to use std::thread instead of kudu::Thread

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: master-test: rewrite to use std::thread instead of kudu::Thread
..


Patch Set 1:

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

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

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

[kudu-CR] master: fix initialization race with consensus RPCs

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: master: fix initialization race with consensus RPCs
..

master: fix initialization race with consensus RPCs

The master initialization order is such that the various RPC services are
brought up before the catalog manager. With multiple masters, it's possible
for a master to receive a consensus-related RPC at this delicate time,
causing a crash.

I spent some time trying to unravel this mess but it proved too thorny, so I
relaxed the CHECK instead. The other masters appear to cope with this error.

There's no explicit test here, but this path is exercised by a stress test
in a follow-on patch.

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


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d1276dd4d3c2f555d63d97d7a16d54181a352b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: add read-write lock to serialize operations around elections

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: master: add read-write lock to serialize operations around 
elections
..


Patch Set 4:

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

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

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

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..

KUDU-1358 (part 1): master should accept heartbeat even if follower

This patch changes the master's heartbeat acceptance code so that heartbeats
are not rejected outright if the master is a follower. To be specific,
tablet reports are ignored, but heartbeats are processed just enough to warm
the TSDescriptor cache. That way, if this master is elected leader, it can
respond to a CreateTable() even before the first round of heartbeats.

An unfortunate side effect is that the "should this tserver register or send
full tablet report?" dance is now more complicated; I tried to cover all
cases in the modified unit test.

I also snuck in a fix to TSManager::RegisterTS: it wasn't actually returning
a TSDescriptor in its out parameter.

Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
---
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_service.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_descriptor.h
M src/kudu/master/ts_manager.cc
6 files changed, 172 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: add read-write lock to serialize operations around elections

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: master: add read-write lock to serialize operations around 
elections
..

master: add read-write lock to serialize operations around elections

This rigmarole began with an investigation into a test failure [1], which
led to a new integration test that hammers VisitTablesAndTablets() while
creating tables. That test revealed other locking issues, which brings us
to where we are now.

This patch introduces a read-write lock to serialize all master operations
so that they fall on one side or the other of a leader election. The idea
is to avoid performing operations concurrently with a reload of the master
metadata; doing so can lead to problems in Shutdown() and (very rarely,
perhaps only conceptually) to inconsistent on-disk state.

I was hoping this lock could replace the fencing done by leader_ready_term_,
but eventually reasoned that we need both; without leader_ready_term_
fencing, the master's consensus state machine could fool an operation into
thinking the master became the leader before the metadata was reloaded.

Three other things of note here:
- The new lock is acquired via TryLock() so that, if the lock could not be
  acquired, the RPC will fail rather than block. A future patch modifies
  TSHeartbeat() to partially accept heartbeats even if the master is a
  follower; TryLock() means that a transitioning leader that is pelted with
  RPCs won't fill up its service queue and can still process heartbeats.
- TableInfo's AddTask() and RemoveTask() methods now don't hold the table's
  lock when adding and removing refs from the task respectively. This is
  the fix for the original test failure.
- When reloading metadata, we now abort all outstanding table tasks to
  avoid orphaning them.

1. 
http://dist-test.cloudera.org:8080/diagnose?key=224b3aa2-3c87-11e6-9a09-0242ac110001

Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
---
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master_service.cc
6 files changed, 331 insertions(+), 144 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..

KUDU-1358 (part 3): new multi-master stress test

This commit adds a stress test for multiple masters. The idea is simple:
issue DDL operations at a high rate while periodically restarting a master.

There's a balance to be struck both in the throughput of the operations and
in the periodicity of the restarts; we need to ensure that the masters can
make enough forward progress (in spite of the failures) to process all of
the requests without timing out. To assist, the client uses abnormally long
timeouts on all operations.

Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/master-stress-test.cc
2 files changed, 435 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: reduce timeout for master to tserver rpcs

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: master: reduce timeout for master to tserver rpcs
..


Patch Set 1:

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

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

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

[kudu-CR] master: fix corruption when AlterTable() races with CreateTable()

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: master: fix corruption when AlterTable() races with 
CreateTable()
..


Patch Set 1:

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

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

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

[kudu-CR] rw mutex: add try lock methods

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: rw_mutex: add try lock methods
..


Patch Set 1:

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

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

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

[kudu-CR] rw mutex: add configurable priority

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: rw_mutex: add configurable priority
..

rw_mutex: add configurable priority

The glibc implementation of pthread rwlocks exposes priorities that can help
if avoiding reader or writer starvation is desirable. I have a use case for
the latter, so let's expose the priorities in our wrapper.

Note: pthread rwlock priorities don't exist on macOS, which is why they're a
"best effort".

Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7
---
M src/kudu/util/rw_mutex.cc
M src/kudu/util/rw_mutex.h
2 files changed, 47 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: reduce timeout for master to tserver rpcs

[Adar Dembo (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: master: reduce timeout for master to tserver rpcs
..

master: reduce timeout for master to tserver rpcs

Thirty seconds seems awfully high, especially since all RPCs retry on
failure anyway. Furthermore, some RPCs are sent to leader replicas, so if an
RPC coincides with a leader election, it may have to wait up to thirty
seconds to retry on the new leader.

Change-Id: I06b5f32c0295f0a9d24ff42695905858ae6101f6
---
M src/kudu/master/catalog_manager.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I06b5f32c0295f0a9d24ff42695905858ae6101f6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: add read-write lock to serialize operations around elections

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: master: add read-write lock to serialize operations around 
elections
..


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/3550/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

PS3, Line 345:  l.c
> could be more descriptive here?
Done


PS3, Line 367: aborting the current task...
> hmm, this (old) log message is nonsense, right?
Yeah. I'll just remove the whole thing. Not useful and adds noise to the 
control flow.


Line 657: e.second->WaitTasksCompletion();
> does this put a restriction on the tasks themselves that they may not block
Correct. As it turns out, there was a separate deadlock issue which I've fixed: 
the task callbacks ran on the same singleton thread pool as this function. Now 
none of the tasks touch the leader lock.


http://gerrit.cloudera.org:8080/#/c/3550/3/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

PS3, Line 304: General status 
> a specific example of what types of "bad status" this might represent would
Done


PS3, Line 305: leadership_status
> you mean leader_status()?
Yes to both.


PS3, Line 308: Leadership status
> would a simple bool suffice?
CheckIsInitializedAndIsLeaderOrRespond() needs access to the status itself 
because it writes the message into the response, but that doesn't preclude the 
leader_status() accessor from being a bool.

But, this way it's more consistent with catalog_status(), which I like. Do you 
feel strongly?


PS3, Line 316: '.
> and returns false
Done


PS3, Line 691: etc
> what about read operations? should you specifically say write operations? a
As you observed, reads should also be guarded by this lock. It may be possible 
for reads to get by with the existing locks, but that means blocking for a long 
period of time on a spinlock, which isn't ideal. I went back and forth on this, 
but in the end reasoned that it's far simpler for _every_ operation to acquire 
it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3501/5/src/kudu/consensus/consensus_queue-test.cc
File src/kudu/consensus/consensus_queue-test.cc:

PS5, Line 822: as a followe
> this seems to conflict with the 'SetLeaderMode' call below?
See next comment.


Line 827:   queue_->SetNonLeaderMode();
> oh, I see, you change it to non-leader. perhaps just update the comment abo
Done. Is this a reasonable way to do this? I just frankenstein'd the test from 
other tests.


Line 851:   // Committed index should be the same.
> I'm not sure why this patch is changing the committed index metric to not a
This patch doesn't change how followers handle the committed index. It changes 
how followers update metrics. The situation before this patch (and after) is 
that followers don't update the committed index-- I tested this a little bit to 
try to confirm it was what was leading to the bad metrics. I'm checking it's 
not updated in the test because if the committed index ever starts to be 
updated I'd like this test to fail, so somebody will take a look at how the 
change to the committed index code affects how metrics should work (e.g. maybe 
the follower could have the same metrics as leader at that point).


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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 6:

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

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

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

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..

KUDU-763 consensus queue metrics on followers are messed up

On non-leader tablet replicas, the majority_done_ops and
in_progress_ops metrics are wrong. The reason is that non-leaders do
not update their consensus queue's committed index
(queue_state_.committed_index). This patch hides these incorrect
numbers in the web ui, where they were previously exposed (I think
unintentionally, the code having printed the queue when it meant to
print the state), documents that the numbers are not meaningful
except in the leader case, and forces them to always be zero
whenever the tablet replica is not a leader.

Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
---
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/raft_consensus.cc
4 files changed, 68 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] WIP [java client] Redo how we manage exceptions

[Jean-Daniel Cryans (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP [java client] Redo how we manage exceptions
..

WIP [java client] Redo how we manage exceptions

Right now the exceptions are hard to handle in the Java client. They're all
generic and you need to do a lot of introspection. For example, if you try
to create a table that already exists, you need to start searching the
exception's message to know if it's that or some other problem that gave
you the error.

With this patch we now only have two kinds of concrete exceptions. Here's the 
tree:

KuduException
 |
 \-> RecoverableException
 |
 \-> NonRecoverableException

The parent class has a new field, `status`, which is your regular Kudu Status
object. Wherever we can we try to recreate the Status objects that are sent
to us from the servers, else we create our own. Now for the example above we
can just query the exception's status with `isNotFound()`.

The sync APIs is also modified to only throw KuduExceptions instead of plain
Exceptions.

Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
D java/kudu-client/src/main/java/org/kududb/client/ConnectionResetException.java
M 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
D java/kudu-client/src/main/java/org/kududb/client/InvalidResponseException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/KuduException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java
M java/kudu-client/src/main/java/org/kududb/client/KuduScanner.java
D java/kudu-client/src/main/java/org/kududb/client/KuduServerException.java
M java/kudu-client/src/main/java/org/kududb/client/KuduSession.java
D java/kudu-client/src/main/java/org/kududb/client/MasterErrorException.java
M 
java/kudu-client/src/main/java/org/kududb/client/NoLeaderMasterFoundException.java
M java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeException.java
M java/kudu-client/src/main/java/org/kududb/client/NonRecoverableException.java
M java/kudu-client/src/main/java/org/kududb/client/PleaseThrottleException.java
M java/kudu-client/src/main/java/org/kududb/client/RecoverableException.java
M java/kudu-client/src/main/java/org/kududb/client/RowResultIterator.java
M java/kudu-client/src/main/java/org/kududb/client/Status.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
D 
java/kudu-client/src/main/java/org/kududb/client/TabletServerErrorException.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java
M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java
26 files changed, 273 insertions(+), 397 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a way to dump ResultTracker state

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a way to dump ResultTracker state
..


Patch Set 2:

(5 comments)

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

Line 329: string ResultTracker::DumpStateToString() {
> rename to 'ToStringUnlocked'?
Done


Line 333: result.append(Substitute("\n\tClient: $0, $1", cs.first, 
cs.second->ToString()));
> can use SubstituteAndAppend
Done


Line 344: result.append(Substitute("\n\tCompletion Record: $0, $1", 
completion_record.first,
> same
Done


Line 356: result.append(Substitute("\n\t$0", orpc.ToString()));
> same
Done


http://gerrit.cloudera.org:8080/#/c/3569/2/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

Line 217: std::string ToString();
> should this (and probably others) be const?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I418da53b52aba5f8358b08709ffe65ece132aeb1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 591 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 19:

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

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

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

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Patch Set 4:

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

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

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

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..

Add information about Exactly Once RPC semantics to rpc.md

This patch adds some info about exactly one rpc semantics to
rpc.md, mostly from a usage perspective.

Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
---
M docs/design-docs/rpc.md
1 file changed, 28 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ToString() method to Proxy

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 7:

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

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

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

[kudu-CR] Integrate the request tracker with the client

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the request tracker with the client
..


Patch Set 20:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 21:

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

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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the ResultTracker into the rpc subsystem
..


Patch Set 21:

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

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

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

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the request tracker with the client

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the request tracker with the client
..

Integrate the request tracker with the client

This integrates the request tracker with the client, making sure
that write requests sent by the client are tracked in the RequestTracker
and contain the information necessary for exactly once semantics.

This adds to rpc-stress-test a test that uses RetriableRpc, instead
of using the proxy directly.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
M src/kudu/rpc/rtest.proto
10 files changed, 245 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the request tracker with the client

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the request tracker with the client
..


Patch Set 19:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 7: Code-Review+1

(1 comment)

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

Line 65: #include 
Is it really needed here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a test for the integration of RequestTracker with the 
client and ResultTracker with the server
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3505/9/src/kudu/rpc/rpc-stress-test.cc
File src/kudu/rpc/rpc-stress-test.cc:

PS9, Line 143: very short timeout
> I dont see the very short timeout
old stuff, removed


Line 172:   rpc_->SendRpc();
> where's the sleep? need to update some comments?
copy past artifact, removed


PS9, Line 207: random amount
> not random?
Done


http://gerrit.cloudera.org:8080/#/c/3505/9/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

Line 273: context->RespondSuccess();
> would be good for this to randomly fail sometimes to test that code path
did this in when I added the client test to the client patch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has abandoned this change.

Change subject: Add a test for the integration of RequestTracker with the 
client and ResultTracker with the server
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the request tracker with the client

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the request tracker with the client
..

Integrate the request tracker with the client

This integrates the request tracker with the client, making sure
that write requests sent by the client are tracked in the RequestTracker
and contain the information necessary for exactly once semantics.

This adds to rpc-stress-test a test that uses RetriableRpc, instead
of using the proxy directly.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
M src/kudu/rpc/rtest.proto
10 files changed, 249 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the request tracker with the client

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the request tracker with the client
..


Patch Set 18:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 3:

(2 comments)

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

Line 711: return typename Collection::value_type::second_type();
It seems I missed it first time, but anyways: consider replacing 
Collection::value_type::second_type with Collection::mapped_type here.


Line 713:   typename Collection::value_type::second_type value = 
std::move(it->second);
Ditto: consider changing Collection::value_type::second_type --> 
Collection::mapped_type for better readability.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 3: Code-Review+1

just a rebase, keeping alexey's +1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add a ToString() method to Proxy

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3502/1/src/kudu/rpc/proxy.cc
File src/kudu/rpc/proxy.cc:

Line 110:   return StringPrintf("%s@%s", service_name_.c_str(), 
conn_id_.ToString().c_str());
> Why do you need the c_str() calls? If it's a StringPrintf thing, maybe you 
Done


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

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

[kudu-CR] Add a ToString() method to Proxy

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 6:

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

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

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

[kudu-CR] Add a FindPointeeOrNull method to map-util

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a FindPointeeOrNull method to map-util
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 20:

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

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

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

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add a RpcContext::RespondFailure() method

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 18:

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

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

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

[kudu-CR] Add a FindPointeeOrNull method to map-util

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a FindPointeeOrNull method to map-util
..

Add a FindPointeeOrNull method to map-util

This adds a new FindPointeeOrNull to map-util to use with maps having smart
pointers as values (like shared_ptr or unique_ptr). The new method returns
a raw pointer to the value contained in in the smart pointer, if the key exits
or nullptr if it doesn't.

Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
---
M src/kudu/gutil/map-util.h
M src/kudu/util/map-util-test.cc
2 files changed, 27 insertions(+), 0 deletions(-)


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

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

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 591 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..

Make RequestTracker not return Status on FirstIncomplete()

This addresses Todd's post-commit comment on the fact that 
RequestTracker::FirstIncomplete()
shouldn't return a Status and should return NO_SEQ_NO instead.

Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
---
M src/kudu/rpc/request_tracker-test.cc
M src/kudu/rpc/request_tracker.cc
M src/kudu/rpc/request_tracker.h
3 files changed, 16 insertions(+), 20 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add ComputeIfAbsent methods to map-util

[David Ribeiro Alves (Code Review)]

Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: Add ComputeIfAbsent methods to map-util
..

Add ComputeIfAbsent methods to map-util

This adds two new ComputeIfAbsent methods to map-util, inspired by java's 
implementation,
which can be found here:

https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#computeIfAbsent-K-java.util.function.Function-

The first one, ComputeIfAbsent, is a simple c++ implementation of the exact same
concept: Retreive a value from a map; if it doesn't exist compute it using a 
lambda
function and insert it first.

The second one is a slight variation that returns a pair instead of a pointer 
to the
value. The pair includes the pointer to the value, but also a bool indicating 
whether
the value was initially absent.

Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
---
M src/kudu/gutil/map-util.h
M src/kudu/util/map-util-test.cc
2 files changed, 71 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a FindPointeeOrNull method to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add a FindPointeeOrNull method to map-util
..


Patch Set 2:

(2 comments)

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

Line 242: // Returns the raw pointer contained in the smart pointer, if the key 
exists or null if it doesn't.
Does it make sense to add something like 'contained in the smart pointer for 
the first found key'?


Line 244: typename Collection::value_type::second_type::element_type*
Collection::value_type::second_type::element_type --> 
Collection::mapped_type::element_type?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 5:

(7 comments)

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

Line 792: pair
Why not just MapContainer::mapped_type, but 
MapContainer::value_type::first_type?


Line 793: ComputeIfAbsentReturnAbsense(MapContainer* container,
> it's due to code style. we pass by pointer when we'll be mutating the conta
OK, code style is code style, whether it's ugly or not.

But if using pointers, why there is no check for a nullptr then?  Is this ok to 
get SIGSEGV when calling this function with a null pointer as a parameter or 
there should be some code which checks for null before de-referencing it?


Line 794:  const typename 
MapContainer::value_type::first_type& key,
MapContainer::value_type::first_type --> MapContainer::key_type?


Line 814: typename MapContainer::value_type::second_type* const
MapContainer::mapped_type?


Line 815: ComputeIfAbsent(MapContainer* container,
Does in make sense to reflect the fact the value is being inserted?  I.e., name 
this InsertComputedIfAbsent()


Line 816: const typename MapContainer::value_type::first_type& 
key,
MapContainer::value_type::first_type --> MapContainer::key_type?


http://gerrit.cloudera.org:8080/#/c/3593/5/src/kudu/util/map-util-test.cc
File src/kudu/util/map-util-test.cc:

Line 58: TEST(ComputeIfAbsentTest, TestComputeIfAbsentAntReturnAbsense) {
typo? TestComputeIfAbsentAntReturnAbsense --> 
TestComputeIfAbsentAndReturnAbsense

Or just rename it into TestComputeIfAbsentReturnAbsense


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ToString() method to Proxy

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 5:

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

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

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

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..

Add a EraseKeyReturnSmartPtrValue to map-util

This adds a new method to map-util to help in erasing key/value pairs for
containers whose value is of smart pointer type.

Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
---
M src/kudu/gutil/map-util.h
M src/kudu/util/map-util-test.cc
2 files changed, 43 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Patch Set 3:

(3 comments)

I'm rejiggering the patches but I don't think we need to merge them all, so at 
least for now, keeping this one separate.

http://gerrit.cloudera.org:8080/#/c/3503/1/docs/design-docs/rpc.md
File docs/design-docs/rpc.md:

PS1, Line 171: RPCs that require exactly once semantics can benefit from them 
by enabling
 : the 'track_rpc_result' option when declaring a service interface 
method.
> Nit: how about: "RPCs that require exactly once semantics can enable them b
Done


PS1, Line 191: no automatic
 : care
> Nit: "automatic care" is a strange phrase. Can you reword?
Done


http://gerrit.cloudera.org:8080/#/c/3503/3/docs/design-docs/rpc.md
File docs/design-docs/rpc.md:

Line 192: care is taken to persist results (make responses live through 
restarts),
> see comments on earlier rev about some of the wording
Done


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

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

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 1:

(5 comments)

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

Line 700: // EraseKeyReturnSmartPtrValue(_map, "my_key, _value);
A tiny typo: "my_key -- > "my_key"


Line 701: template 
What if the collection is a multi-key container, like multimap?  Is it OK just 
to remove the first element having that key and leave the rest?  If so, please 
add explanation about that in the comment for the function.


Line 702: void EraseKeyReturnSmartPtrValue(Collection* const collection,
Consider returning the result as a smart pointer instead of placing it into the 
parameter placeholder.  From the caller's perspective that would be more terse 
then:

unique_ptr p(EraseKeyReturnSmartPtrValue(my_map, "key");

Also, why not to pass input collection by const reference?  Passing parameter 
by pointer means it might be null, so it's necessary to take care of that.


Line 703:  const typename 
Collection::value_type::first_type& key,
Why not just Collection::key_type, but Collection::value_type::first_type?


http://gerrit.cloudera.org:8080/#/c/3595/1/src/kudu/util/map-util-test.cc
File src/kudu/util/map-util-test.cc:

Line 80: TEST(EraseKeyReturnSmartPtrValueTest, TestEraseKeyReturnSmartPtrValue) 
{
Does it make sense to add a testcase for multi-key dictionary like multimap?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a RpcContext::RespondFailure() method
..

Add a RpcContext::RespondFailure() method

In the case of certain errors we use RpcContext::RespondSuccess() because
setting the error is call dependent and it was done on the call's request.
Since we'll be relying on the RpcContext to call the appropriate method
on the ResultTracker, on the common case, we'd still be marking these error
cases as successful RPCs whose results we keep. This is problematic for
various reasons, the main one being that if a client gets a transient error
back (e.g. the call was throttled) all subsequent attemtpts at the same
call will get the same result.

To avoid this we need to be able to flag that a call failed even if we don't
need anything else done to the response. To this goal this patch adds
RpcContext::RespondFailure(), which currently does nothing else besides
call RespondSuccess(), but will do so when the result tracker is integrated
in.

Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
---
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
2 files changed, 19 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 16:

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

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

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

[kudu-CR] Add a ResultTracker class that will track server side results

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 18:

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

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

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

[kudu-CR] Add a ToString() method to Proxy

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ToString() method to Proxy
..

Add a ToString() method to Proxy

ReplicatedRpc takes the server proxy type as a template argument and
uses its ToString() method to print out details in case of error. Usually
this is RemoteTablet, which has a ToString() method, but that might
not always be the case.

In fact, in a test in a follow up patch ReplicatedRpc takes Proxy as the
server proxy type and compilation would fail due to a missing ToString().
We could make ReplicatedRpc not use the ToString() method, but it seems
very helpful to have it so this patch adds it to Proxy instead.

Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
---
M src/kudu/rpc/proxy.cc
M src/kudu/rpc/proxy.h
M src/kudu/rpc/rpc-test.cc
3 files changed, 12 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 5:

(3 comments)

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

Line 780: // it uses the lambda function to create a value, inserts it into the 
map, and returns a a pair with
typo: an extra 'a': '...and returns a a pair...'


Line 791: template 
How costly is compute_func compared with locating the key in the map?  If in 
most use cases compute_func is cheap, then it makes sense just to use stock 
insert() method right away, not trying to locate the element first and not 
calling this method.  But I suppose this is not the case, right?


Line 793: ComputeIfAbsentReturnAbsense(MapContainer* container,
Why not to pass the container by reference here?  Is that due to code style?  
If passing by pointer, what if parameter is null?  Is any check needed here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes