[kudu-CR] WIP: KUDU-798 (part 3) Replica transactions must start/abort on the consensus update thread

2016-12-02 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: WIP: KUDU-798 (part 3) Replica transactions must start/abort on 
the consensus update thread
..


Patch Set 12:

no, I'll clean this up and run the dist-tests again.
I'll also come up with an additional unit test.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie360e597eea86551c453717d7a1a000848027f4c
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] WIP: KUDU-798 (part 3) Replica transactions must start/abort on the consensus update thread

2016-12-02 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: WIP: KUDU-798 (part 3) Replica transactions must start/abort on 
the consensus update thread
..


Patch Set 13:

(1 comment)

k, i like this approach more. BTW is there a later test in this series that 
validates that this does as it purports?

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

PS13, Line 381:   // Increase our ref count so that we're sure the driver 
survives until after we wait on the latch,
is this still needed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie360e597eea86551c453717d7a1a000848027f4c
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] WIP: KUDU-798 (part 3) Replica transactions must start/abort on the consensus update thread

2016-12-02 Thread David Ribeiro Alves (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: WIP: KUDU-798 (part 3) Replica transactions must start/abort on 
the consensus update thread
..

WIP: KUDU-798 (part 3) Replica transactions must start/abort on the consensus 
update thread

In order for a consensus replica to safely move "safe" time, it needs
to know that all transactions that come before it have at least
started. One way to do this is to call transaction_->Start() on
transaction_driver_->Init(). We know Init() is called by the thread
updating consensus, for non-leader transactions. This should be ok as
the leader has already correctly serialized the transactions.

However, if transactions are now started on Init() then, in the case
they abort (i.e. when transaction_driver_->ReplicationFinished() is
called with a non-ok status, again done by the thread updating
consensus), we must make sure that the transaction is actually
removed from mvcc before that method returns. Otherwise consensus
might call Start() on another transaction with the same timestamp
before the failed transaction is removed from mvcc.

To do this this patch adds a way to release only the mvcc txn,
since its the only thing we care about.

Change-Id: Ie360e597eea86551c453717d7a1a000848027f4c
---
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
5 files changed, 61 insertions(+), 40 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie360e597eea86551c453717d7a1a000848027f4c
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon