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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

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
Reviewed-on: http://gerrit.cloudera.org:8080/3080
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
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, 244 insertions(+), 6 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: 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 26:

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

-- 
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: 26
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 (#26).

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, 244 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3080/26
-- 
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: 26
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 24: -Verified

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

-- 
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: 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] 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 (#24).

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, 243 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3080/24
-- 
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: 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 

[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 24:

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

-- 
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: 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] 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 23:

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

-- 
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: 23
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

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

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


Patch Set 22:

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

-- 
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: 22
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 (#22).

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/22
-- 
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: 22
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)]

David Ribeiro Alves has posted comments on this change.

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


Patch Set 17:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3080/17/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 212:   // failure.
> something funny happened to a comment here
Done


http://gerrit.cloudera.org:8080/#/c/3080/17/src/kudu/rpc/rpc_controller.h
File src/kudu/rpc/rpc_controller.h:

Line 119:   // calling this method.
> I think this is more simply stated: REQUIRES: the controller has a request 
Done


-- 
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: 17
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] 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] 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] 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] 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] 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 17:

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

-- 
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: 17
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

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

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


Patch Set 16:

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

-- 
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: 16
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

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

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


Patch Set 15:

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

-- 
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: 15
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

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

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


Patch Set 14:

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

-- 
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: 14
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

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

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


Patch Set 13: -Verified

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

-- 
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: 13
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 (#13).

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.

Since this is hard to test in isolation (without the server part) and it
is already tested by the next patch in the sequence, this doesn't contain
any specific tests.

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_controller.cc
M src/kudu/rpc/rpc_controller.h
7 files changed, 65 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3080/13
-- 
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: 13
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)]

David Ribeiro Alves has posted comments on this change.

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


Patch Set 8:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/3080/8//COMMIT_MSG
Commit Message:

Line 15: any specific tests.
> I guess I can come up with a unit test for retriable rpc, if that helps, bu
as we discussed in person, the new plan is to test retriable rpc when the 
result tracker is in, on rpc-stress-test. punting on this until then


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 194:const scoped_refptr& request_tracker,
> what about storing this in the Batcher? or grabbing it via batcher->client-
turns out this can't be stored in the batcher (I tried and got sigsevs), rpcs 
may outlive the batcher


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 72: using rpc::RequestTracker;
> spurious changes in this file?
Done


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

Line 27: #include "kudu/rpc/request_tracker.h"
> can be a forward decl, no?
Done


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

Line 34: #include "kudu/rpc/request_tracker.h"
> spurious?
Done


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 125: // in flight, so we retry with a delay.
> I think this merits a warning.
this doesn't return a non-ok status currently (we removed the max in flights 
thing) so I just made this a check


Line 193: request_id->set_first_incomplete_seq_no(internal::NO_SEQ_NO);
> why not just leave it unset in the PB?
removed this branching since First incomplete now just returns a NO_SEQ_NO.


Line 210:   request_tracker_->RpcCompleted(sequence_number_);
> nit: mid-comment
Done


Line 210:   request_tracker_->RpcCompleted(sequence_number_);
> as a safety check, I think we should probably set the seq number back to NO
Done


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/rpc_controller.cc
File src/kudu/rpc/rpc_controller.cc:

Line 103:   return *request_id_.get();
> nit: I think you dont need .get()
Done


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/rpc_controller.h
File src/kudu/rpc/rpc_controller.h:

Line 114:   const RequestIdPB& request_id() const;
> I think this is somewhat error prone - IIRC the RPC layer "steals" the requ
there's a DCHECK there, is that not what you meant? added the comment you 
suggested


-- 
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: 8
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] 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 10:

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

-- 
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: 10
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)]

David Ribeiro Alves has posted comments on this change.

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


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3080/8//COMMIT_MSG
Commit Message:

Line 15: any specific tests.
> it's not possible to use RetriableRpc within rpc_stub-test to test the clie
I guess I can come up with a unit test for retriable rpc, if that helps, but it 
wouldn't test it with the meta cache (retriable rpc received a server picker 
that, on the client, is backed by the meta cache).


-- 
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: 8
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] Integrate the request tracker with the client

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

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


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 210:   request_tracker_->RpcCompleted(sequence_number_);
> nit: mid-comment
as a safety check, I think we should probably set the seq number back to 
NO_SEQ_NO, and in the dtor, DCHECK that it's NO_SEQ_NO. Otherwise we might have 
some very tricky-to-catch "leaks" of RPCs in the request tracker.


-- 
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: 8
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] Integrate the request tracker with the client

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

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


Patch Set 8:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/3080/8//COMMIT_MSG
Commit Message:

Line 15: any specific tests.
it's not possible to use RetriableRpc within rpc_stub-test to test the client 
part?


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 194:const scoped_refptr& request_tracker,
what about storing this in the Batcher? or grabbing it via 
batcher->client->data->request_tracker as necessary? seems like an unnecessary 
extra parameter (and refcount incr/decr)


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 72: using rpc::RequestTracker;
spurious changes in this file?


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

Line 27: #include "kudu/rpc/request_tracker.h"
can be a forward decl, no?


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

Line 34: #include "kudu/rpc/request_tracker.h"
spurious?


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 125: // in flight, so we retry with a delay.
I think this merits a warning.


Line 193: request_id->set_first_incomplete_seq_no(internal::NO_SEQ_NO);
why not just leave it unset in the PB?


Line 210:   request_tracker_->RpcCompleted(sequence_number_);
nit: mid-comment


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/rpc_controller.cc
File src/kudu/rpc/rpc_controller.cc:

Line 103:   return *request_id_.get();
nit: I think you dont need .get()


http://gerrit.cloudera.org:8080/#/c/3080/8/src/kudu/rpc/rpc_controller.h
File src/kudu/rpc/rpc_controller.h:

Line 114:   const RequestIdPB& request_id() const;
I think this is somewhat error prone - IIRC the RPC layer "steals" the request 
ID out of the RpcController at send time, no? Seems like it's worth a doc 
comment that these are not accessible after an RPC has been sent. And maybe a 
DCHECK like some of the other accessors have?


-- 
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: 8
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] Integrate the request tracker with the client

[David Ribeiro Alves (Code Review)]

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

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.

Since this is hard to test in isolation (without the server part) and it
is already tested by the next patch in the sequence, this doesn't contain
any specific tests.

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/client/meta_cache.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
8 files changed, 74 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3080/8
-- 
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: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon