David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-798 (part 1) Unify leader/follower mvcc behavior
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5055/9/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

PS9, Line 124:  Finish the Prepare phase of a write transaction.
I also removed this since this will change in the future (using Todd's 
suggestion of start()ing the txn before acquiring locks)


PS9, Line 126: assigns a timestamp for the transaction
> this is inaccurate, right?
Done


PS9, Line 127: // This also snapshots the current set of tablet components into 
the transaction
             :   // state.
> this is also out of date as of be1e476df107786732373850047db378636a9f5a. mi
Done


PS9, Line 130:  // This should always be done _after_ any relevant row locks 
are acquired
             :   // (using CreatePreparedInsert/CreatePreparedMutate). This 
ensures that,
             :   // within each row, timestamps only move forward. If we took a 
timestamp before
             :   // getting the row lock, we could have the following situation:
             :   //
             :   //   Thread 1         |  Thread 2
             :   //   ----------------------
             :   //   Start tx 1       |
             :   //                    |  Start tx 2
             :   //                    |  Obtain row lock
             :   //                    |  Update row
             :   //                    |  Commit tx 2
             :   //   Obtain row lock  |
             :   //   Delete row       |
             :   //   Commit tx 1
             :   //
             :   // This would cause the mutation list to look like: @t1: 
DELETE, @t2: UPDATE
             :   // which is invalid, since we expect to be able to be able to 
replay mutations
             :   // in increasing timestamp order on a given row.
             :   //
             :   // This requirement is basically two-phase-locking: the order 
in which row locks
             :   // are acquired for transactions determines their 
serialization order. If/when
             :   // we support multi-node serializable transactions, we'll have 
to acquire _all_
             :   // row locks (across all nodes) before obtaining a timestamp.
             :   //
> does all of this belong elsewhere now?
I updated and moved this to transaction_driver.h. I know this is most relevant 
for writes but I didn't want to have it be lost in write_transaction.h, we 
already have type of comments on transaction_driver.h


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to