[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

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

Change subject: Integrate the result tracker with writes
..


Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Reviewed-on: http://gerrit.cloudera.org:8080/3449
Reviewed-by: David Ribeiro Alves 
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Kudu Jenkins
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)

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



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

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

[kudu-CR] Integrate the result tracker with writes

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 28: Code-Review+2

-- 
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: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 28: Code-Review+2

rebased this due to a problem with the mm patch. keeping the +2

-- 
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: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] 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 28:

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

-- 
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: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

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

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

-- 
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: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 25:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS25, Line 176: request were
> nit: either request should be plural or were should be was.
Done


-- 
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: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 25:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

PS25, Line 210: it's
> nit: its
Done


PS25, Line 268: er.");
> Any more info that we can add here?
Added the request id. Note that the user won't see this as the request is 
transparently retried.


http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS25, Line 124: client originated
> nit: client-originated
Done


PS25, Line 124: a
> nit: remove
Done


PS25, Line 149: to to 
> nit
Done


PS25, Line 155: it's
> nit: its
Done


PS25, Line 156: requests
> nit: request? same for the other "requests" on that line.
Done


PS25, Line 157:  
> nit
Done


PS25, Line 168: and
> nit: remove that 'and'
Done


PS25, Line 169: received
> nit: receive
Done


PS25, Line 302: drivers
> nit: driver
Done


http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/write_transaction.h
File src/kudu/tablet/transactions/write_transaction.h:

PS25, Line 192: pointers to the rpc context, request and response, lifecyle
  :   // is managed by the rpc subsystem.
> Since you're here, mind rewriting this sentence? Start with upper case, the
Done


PS25, Line 194: Request
> nit: 'request_'
Done


-- 
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: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

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

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

-- 
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: 25
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 24: Verified+1

bunch of unrelated failures with no logs. guess Im ddos'ing the dist test system

-- 
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: 24
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 397 insertions(+), 40 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

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

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

-- 
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: 24
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 23: Verified+1

unrelated flakyness, addressed that on a previous patch

-- 
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: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
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 23:

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

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

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

-- 
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: 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: Mike Percy 
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 21:

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

-- 
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: 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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
23 files changed, 374 insertions(+), 37 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 17:

(18 comments)

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

Line 51: #include "kudu/rpc/rpc_header.pb.h"
> unused?
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

Line 182:   optional rpc.RequestIdPB request_id = 100;
> why using such a high id here? seems like it wastes a byte
the idea was that we could keep adding request types (like the optional below). 
changed this to be in sequence.


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

Line 226: bool final_driver = false;
> doc this
removed this (scaffolding code)


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

Line 251:  WriteResponsePB* response);
> mentioned on rev 1, but I dont like the side effect here without an accompa
Done


Line 1243:   if (tracking_results) {
> any way to extract this new bit of code to a separate function?
this is way simpler now with the atomic bit.  please take a new look.


Line 1279:   LOG(FATAL) << "Couldn't change bootstrapping txn to driver 
after 10 attempts for request: "
> this seems like a kind of arbitrary thing... should this be a DFATAL and ke
removed


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_peer.cc
File src/kudu/tablet/tablet_peer.cc:

Line 130: const scoped_refptr 
result_tracker,
> reference
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

Line 212:   if (state()->are_results_tracked()) {
> change to if (!...) return
Done


Line 314: case REPLICATING:
> some spurious changes here
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS17, Line 117: // 1 - When becoming leader, a replica has already prepared all 
the requests that it received
  : // from the previous leader that it will try to 
replicate/commit itself.
  : /
> not quite understanding this sentence
Done


PS17, Line 122: Requests originated on other replicas 
> not sure quite what you mean here. Do you mean operations which were starte
Done


PS17, Line 158: FailAndRespond() 
> hrm, is this FailAndRespond here actually responding to any RPCs in the cur
the design changed and the docs were not updated. change this to reflect the 
new state of affairs.


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/write_transaction.h
File src/kudu/tablet/transactions/write_transaction.h:

PS17, Line 196: c_
> typo
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/remote_bootstrap_session-test.cc
File src/kudu/tserver/remote_bootstrap_session-test.cc:

Line 137:  dummy,
> can just use scoped_refptr(), no?
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

Line 280: const char* ALREADY_PRESENT_ERROR = "There's already an attempt at 
the same operation "
> is this used?
Done


Line 295:   //LOG(INFO) << "Responding error to: " << 
context_->request_pb()->DebugString();
> remove
Done


Line 764:   // attempted elsewhere.
> this probably needs to be moved into tablet_peer
this now is done on the replicate message for all txns. removed


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

Line 123: const char* WRITE_METHOD_NAME = "Write";
> what's this used for? dont see it referenced elsewhere
Done


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

[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] Integrate the result tracker with writes

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 17:

(18 comments)

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

Line 51: #include "kudu/rpc/rpc_header.pb.h"
unused?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

Line 182:   optional rpc.RequestIdPB request_id = 100;
why using such a high id here? seems like it wastes a byte


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

Line 162: completion_record->final_driver = true;
this is a big code smell -- this function should be read-only, not somehow have 
a side effect. If you want it to have a side effect, rename it to 
'BecomeFinalDriver' or something? 'UpgradeToFinalDriver'?

Also, the docs in the header file should reference this concept of being the 
'final driver'


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

Line 226: bool final_driver = false;
doc this

also, weird that this field has a default but the others don't


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

Line 251:  WriteResponsePB* response);
mentioned on rev 1, but I dont like the side effect here without an 
accompanying method rename and doc update


Line 1243:   if (tracking_results) {
any way to extract this new bit of code to a separate function?


Line 1279:   LOG(FATAL) << "Couldn't change bootstrapping txn to driver 
after 10 attempts for request: "
this seems like a kind of arbitrary thing... should this be a DFATAL and keep 
looping?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_peer.cc
File src/kudu/tablet/tablet_peer.cc:

Line 130: const scoped_refptr 
result_tracker,
reference


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

Line 212:   if (state()->are_results_tracked()) {
change to if (!...) return


Line 314: case REPLICATING:
some spurious changes here


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS17, Line 117: // 1 - When becoming leader, a replica has already prepared all 
the requests that it received
  : // from the previous leader that it will try to 
replicate/commit itself.
  : /
not quite understanding this sentence


PS17, Line 122: Requests originated on other replicas 
not sure quite what you mean here. Do you mean operations which were started 
while the node is a follower? Or "originated" meaning "the first attempt"?


PS17, Line 158: FailAndRespond() 
hrm, is this FailAndRespond here actually responding to any RPCs in the current 
design? Can we encapsulate this "precedence" behavior into something like the 
'UpgradeToExclusiveDriver' call I mentioned in the other review?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/write_transaction.h
File src/kudu/tablet/transactions/write_transaction.h:

PS17, Line 196: c_
typo


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/remote_bootstrap_session-test.cc
File src/kudu/tserver/remote_bootstrap_session-test.cc:

Line 137:  dummy,
can just use scoped_refptr(), no?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

Line 280: const char* ALREADY_PRESENT_ERROR = "There's already an attempt at 
the same operation "
is this used?


Line 295:   //LOG(INFO) << "Responding error to: " << 
context_->request_pb()->DebugString();
remove


Line 764:   // attempted elsewhere.
this probably needs to be moved into tablet_peer


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

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

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

-- 
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: 17
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 431 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 17
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 

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

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

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

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

-- 
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: 15
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 423 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 15
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 

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

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

-- 
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: 14
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 397 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 14
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 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
29 files changed, 439 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 13
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 

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

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

-- 
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: 13
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] 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 12:

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

-- 
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: 12
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
29 files changed, 439 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 12
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 

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

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

-- 
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: 11
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
29 files changed, 439 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 11
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 

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

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

-- 
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: 10
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
29 files changed, 433 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 10
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 

[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 9: -Verified

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

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

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

-- 
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: 9
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] 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 8: -Verified

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

-- 
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: 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: Mike Percy 
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 7: -Verified

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

-- 
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: 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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_service.proto
26 files changed, 403 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

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

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

-- 
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: 6
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] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_service.proto
26 files changed, 388 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 1:

this is good to be reviewed, but it's still missing a flag to disable it until 
we've finished garbage collection

-- 
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
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 1:

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

-- 
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No