You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/11/12 00:11:38 UTC

[kudu-CR] KUDU-768 (part 1) - Move timestamp assignement out of Tablet

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: KUDU-768 (part 1) - Move timestamp assignement out of Tablet
......................................................................

KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Currently tablets are responsible for timestamp assignement. For safe
time to be correct consensus leaders need to be able to tell other
replicas what the safe time is and for that this to be correct
timestamp assignment needs to happen on Consensus::Replicate().
This because we need to avoid the race where the leader will read the
clock thinking that was the safe time but there was actually already
an operation in-flight with a lower timestamp that just haden't been
submitted to Consensus::Replicate().

This patch is a first step in that direction. It doesn't move timestamp
assignment into consensus yet, but it moves it from Tablet into the
TransactionDriver, which performs it just before Consensus::Replicate().

The main behavior change is that (outside of tests) all mvcc
transactions now have a pre-assigned timestamp, whereas before only the
follower was had that behavior. By making this change this patch proves
that this stragey is feasible.

This doesn't actually gut MvccManager to remove the multiple modes of
starting a ScopedTransaction as some tests still require that. Follow
patches will remove that test dependency.

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 56 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-768 (part 1) - Move timestamp assignement out of Tablet
......................................................................

KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Currently tablets are responsible for timestamp assignement. For safe
time to be correct consensus leaders need to be able to tell other
replicas what the safe time is and for that this to be correct
timestamp assignment needs to happen on Consensus::Replicate().
This because we need to avoid the race where the leader will read the
clock thinking that was the safe time but there was actually already
an operation in-flight with a lower timestamp that just haden't been
submitted to Consensus::Replicate().

This patch is a first step in that direction. It doesn't move timestamp
assignment into consensus yet, but it moves it from Tablet into the
TransactionDriver, which performs it just before Consensus::Replicate().

The main behavior change is that (outside of tests) all mvcc
transactions now have a pre-assigned timestamp, whereas before only the
follower was had that behavior. By making this change this patch proves
that this stragey is feasible.

This doesn't actually gut MvccManager to remove the multiple modes of
starting a ScopedTransaction as some tests still require that. Follow
patches will remove that test dependency.

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 49 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

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


Patch Set 9:

(3 comments)

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

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


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. mind removing it?


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?


-- 
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 <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Safe time is a timestamp such that all transactions before it are
known and either completed or in-flight. Waiting for the Mvcc
snapshot at "safe time" to be "clean" allows to yield repeatable
reads: scans of a tablet at a snapshot defined by a timestamp
that will always return the same results. Proper "safe time"
advancement also allows to improve load balancing: A scan at a clean
timestamp that is lower that "safe time" on a replica is guaranteed
to yield the same results as the same scan on the leader replica
(though maybe with a lantency penalty).

Currently this timestamp is advanced within Mvcc but this is not
natural as in conflates the consensus state (all the operations
that are being replicated and/or replayed) and the mvcc state
(all the operations that have been consensus committed and are
being applied). Furthermore, there is a confusing mixing of
concepts in Mvcc between "safe time" and "clean time" where the
latter means a timestamp such that all operation have been
completed, whereas the former also includes the operations that
are in-flight, even if they haven't started being applied to
the tablet.

This patch series aims at separating the two concepts and fixing
safe time advancement:
a) - Safe time advancement will be handled by consensus: The leader
can easily establish which timestamps are safe for a replica by
looking at which operations that replica knows and what the
timestamp of the last committed operation is.
b) - Mvcc will only take care of monitoring "clean time" advancement.
This makes it simpler to wait for a timestamp to be "safe" and "clean"
the caller will first wait for a timestamp to be "safe" meaning all
operations are known and in-flight and then wait for it to be "clean"
in mvcc meaning all the in-flight operations before have completed.

This patch in particular takes the first two steps in this direction:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 51 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-768 (part 1) - Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-768 (part 1) - Unify leader/follower mvcc behavior

Safe time is a timestamp such that all transactions before it are
known and either completed or in-flight. Waiting for the Mvcc
snapshot at "safe time" to be "clean" allows to yield repeatable
reads: scans of a tablet at a snapshot defined by a timestamp
that will always return the same results. Proper "safe time"
advancement also allows to improve load balancing: A scan at a clean
timestamp that is lower that "safe time" on a replica is guaranteed
to yield the same results as the same scan on the leader replica
(though maybe with a lantency penalty).

Currently this timestamp is advanced within Mvcc but this is not
natural as in conflates the consensus state (all the operations
that are being replicated and/or replayed) and the mvcc state
(all the operations that have been consensus committed and are
being applied). Furthermore, there is a confusing mixing of
concepts in Mvcc between "safe time" and "clean time" where the
latter means a timestamp such that all operation have been
completed, whereas the former also includes the operations that
are in-flight, even if they haven't started being applied to
the tablet.

This patch series aims at separating the two concepts and fixing
safe time advancement:
a) - Safe time advancement will be handled by consensus: The leader
can easily establish which timestamps are safe for a replica by
looking at which operations that replica knows and what the
timestamp of the last committed operation is.
b) - Mvcc will only take care of monitoring "clean time" advancement.
This makes it simpler to wait for a timestamp to be "safe" and "clean"
the caller will first wait for a timestamp to be "safe" meaning all
operations are known and in-flight and then wait for it to be "clean"
in mvcc meaning all the in-flight operations before have completed.

This patch in particular takes the first two steps in this direction:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 51 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-768 (part 1) - Move timestamp assignement out of Tablet
......................................................................

KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Safe time is a timestamp such that all transactions before it are
known and either completed or in-flight. Waiting for the Mvcc
snapshot at "safe time" to be "clean" allows to yield repeatable
reads: scans of a tablet at a snapshot defined by a timestamp
that will always return the same results. Proper "safe time"
advancement also allows to improve load balancing: A scan at a clean
timestamp that is lower that "safe time" on a replica is guaranteed
to yield the same results as the same scan on the leader replica
(though maybe with a lantency penalty).

Currently this timestamp is advanced within Mvcc but this is not
natural as in conflates the consensus state (all the operations
that are being replicated and/or replayed) and the mvcc state
(all the operations that have been consensus committed and are
being applied). Furthermore, there is a confusing mixing of
concepts in Mvcc between "safe time" and "clean time" where the
latter means a timestamp such that all operation have been
completed, whereas the former also includes the operations that
are in-flight, even if they haven't started being applied to
the tablet.

This patch series aims at separating the two concepts and fixing
safe time advancement:
a) - Safe time advancement will be handled by consensus: The leader
can easily establish which timestamps are safe for a replica by
looking at which operations that replica knows and what the
timestamp of the last committed operation is.
b) - Mvcc will only take care of monitoring "clean time" advancement.
This makes it simpler to wait for a timestamp to be "safe" and "clean"
the caller will first wait for a timestamp to be "safe" meaning all
operations are known and in-flight and then wait for it to be "clean"
in mvcc meaning all the in-flight operations before have completed.

This patch in particular takes the first two steps in this direction:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication.
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 51 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-798 (part 1) Unify leader/follower mvcc behavior

This patch does the following:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 56 insertions(+), 27 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

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


Patch Set 11:

(1 comment)

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

Line 126:   // TODO: rename this to something like "FinishPrepare" or "StartApply", since
> warning: missing username/bug in TODO [google-readability-todo]
Done


-- 
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: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.

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


KUDU-798 (part 1) Unify leader/follower mvcc behavior

This patch does the following:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Reviewed-on: http://gerrit.cloudera.org:8080/5055
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 93 insertions(+), 58 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

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


Patch Set 6:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/5055/6//COMMIT_MSG
Commit Message:

Line 9: Safe time is a timestamp such that all transactions before it are
> would be great to have the definitions of 'safe' and 'clean' times somewher
this is in progress. I started to dump this info on docs/design-docs/repeatable-reads.md but theres still a some work to make that minimally coherent


PS6, Line 17: lantency
> nit: typo
removed


PS6, Line 19: this timestamp
> you mean the safe time, right?
removed


PS6, Line 21: mvcc
> nit: can you consistetly write it as 'MVCC' for better readability?
removed


http://gerrit.cloudera.org:8080/#/c/5055/6/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

PS6, Line 393:   mvcc_tx.reset(new ScopedTransaction(&mvcc_, tx_state->timestamp()));
             :   tx_state->SetMvccTxAndTimestamp(std::move(mvcc_tx));
             : }
> strikes me that this implementation is the same as the 'ForTests' method ab
Done


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

Line 160:   void StartTransactionForTests(WriteTransactionState* tx_state);
> rename this to AssignTimestampAndStartTransactionForTests or somesuch? othe
Done


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

Line 264:       RETURN_NOT_OK(transaction_->Start());
> I'm not 100% sure here, but it strikes me as strange that the order of tran
I don't think it matters but the reversal was not on purpose. Changed this back.


PS6, Line 284:  // This is a placeholder since in the near future the timestamp will be assigned.
             :       // within consensus.
> can you add a TODO(dralves) and be more specific about the JIRA that will r
Done


Line 286:       if (!transaction_->state()->has_timestamp()) {
> could this be a CHECK? when would we be in this case and not already have a
good catch. Done


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

Line 193:                     const scoped_refptr<server::Clock>& clock);
> I think this should just be a 'scoped_refptr' and then assign it with std::
Done


Line 322:   // Temporatily have the clock on the driver so that we can assign timestamps to
> typo: temporarily
yeah, thats that I meant. Done


-- 
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-798 (part 1) Unify leader/follower mvcc behavior

This patch does the following:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 93 insertions(+), 58 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

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


Patch Set 12: Code-Review+2

-- 
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-768 (part 1) - Move timestamp assignement out of Tablet
......................................................................

KUDU-768 (part 1) - Move timestamp assignement out of Tablet

Currently tablets are responsible for timestamp assignement. For safe
time to be correct consensus leaders need to be able to tell other
replicas what the safe time is and for that this to be correct
timestamp assignment needs to happen on Consensus::Replicate().
This because we need to avoid the race where the leader will read the
clock thinking that was the safe time but there was actually already
an operation in-flight with a lower timestamp that just haden't been
submitted to Consensus::Replicate().

This patch is a first step in that direction. It doesn't move timestamp
assignment into consensus yet, but it moves it from Tablet into the
TransactionDriver, which performs it just before Consensus::Replicate().

The main behavior change is that (outside of tests) all mvcc
transactions now have a pre-assigned timestamp, whereas before only the
follower was had that behavior. By making this change this patch proves
that this stragey is feasible.

This doesn't actually gut MvccManager to remove the multiple modes of
starting a ScopedTransaction as some tests still require that. Follow
patches will remove that test dependency.

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 56 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-798 (part 1) Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-798 (part 1) Unify leader/follower mvcc behavior

This patch does the following:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 92 insertions(+), 57 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

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


Patch Set 6:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/5055/6//COMMIT_MSG
Commit Message:

Line 9: Safe time is a timestamp such that all transactions before it are
would be great to have the definitions of 'safe' and 'clean' times somewhere in docs/design-docs/ or a block comment in one of the headers. Maybe this information can be moved into such a place and the commit message updated to just refer to the newly-added docs?


PS6, Line 17: lantency
nit: typo


PS6, Line 19: this timestamp
you mean the safe time, right?


PS6, Line 21: mvcc
nit: can you consistetly write it as 'MVCC' for better readability?


PS6, Line 30: This patch series aims at separating the two concepts and fixing
            : safe time advancement:
            : a
I like the general direction here.

I think the key thing is that we used to sort of think of safe/clean as having a "subset" relationship and now that's not necessarily the case, right?

One concern, having not yet read the patch yet, but just thinking about this: when we construct the 'current MvccSnapshot' for internal-facing purposes like compaction, we need to make sure we don't prematurely use a simple 'TS < <clean timestamp>' snapshot when that snapshot is not also safe, right? Otherwise if we're a follower and a new write arrives with a lower timestamp, it can show up in the middle of a compaction and wreak havoc.

How will snapshots chosen for compaction/flush work now, given they also need to be repeatable?


http://gerrit.cloudera.org:8080/#/c/5055/6/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

PS6, Line 393:   mvcc_tx.reset(new ScopedTransaction(&mvcc_, tx_state->timestamp()));
             :   tx_state->SetMvccTxAndTimestamp(std::move(mvcc_tx));
             : }
strikes me that this implementation is the same as the 'ForTests' method above in the case that the tx already has the timestamp assigned. Will the above one be phased out? Maybe we can change the above one to CHECK that !tx_state->has_timestamp() and any cases where that check would fire we should be using this new method?


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

Line 160:   void StartTransactionForTests(WriteTransactionState* tx_state);
rename this to AssignTimestampAndStartTransactionForTests or somesuch? otherwise it's a bit confusing at call sites.


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

Line 264:       RETURN_NOT_OK(transaction_->Start());
I'm not 100% sure here, but it strikes me as strange that the order of transaction_->Start() and the ResultTracker registration is getting inverted by this patch. Does that have the potential to cause some issue?


PS6, Line 284:  // This is a placeholder since in the near future the timestamp will be assigned.
             :       // within consensus.
can you add a TODO(dralves) and be more specific about the JIRA that will remove it?


Line 286:       if (!transaction_->state()->has_timestamp()) {
could this be a CHECK? when would we be in this case and not already have a timestamp?


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

Line 193:                     const scoped_refptr<server::Clock>& clock);
I think this should just be a 'scoped_refptr' and then assign it with std::move in the initializer list


Line 322:   // Temporatily have the clock on the driver so that we can assign timestamps to
typo: temporarily

also, what do you mean by temporarily here? If you mean that this is transitional and you plan to remove it in a later patch in this series, can you leave a TODO(dralves) with a specific call-out to which JIRA will remove it? Otherwise I think we'll just forget about this forever.


-- 
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

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


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5055/6//COMMIT_MSG
Commit Message:

PS6, Line 30: This patch series aims at separating the two concepts and fixing
            : safe time advancement:
            : a
> I like the general direction here.
Not sure about this problem. It's still a subset: all "clean" timestamps are "safe", that is we won't advance clean past safe (otherwise we might get in trouble, like you hinted at) it just that we can't only rely on "clean" to advance when waiting for a repeatable read.

For instance say that a follower has a clean timestamp of 5, with no writes going on. The leader then updates the safe time to 10. We can't move "clean" to 10 since another leader might come up with operations that come before that (well not until we have leader leases anyway) so what we do is we advance safe time to 10 and keep clean time where it is.

From another angle, until we have leader leases, safe time might wobble back and forth, we need to make sure that clean time never does that. so we keep clean time at the time of the last committed txn.

When we have leader leases we can actually trust that safe time won't ever go back but having this separation of concerns is still very useful. For instance the time manager (the safe time keeping entity) is knowledgeable about some consensus information like lag and who the leader is, which we can use to better inform clients on what to do when they try snapshot scans on a replica.


-- 
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

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

KUDU-798 (part 1) - Unify leader/follower mvcc behavior

This patch does the following:
1) It moves timestamp assignment from tablet and into the
TransactionDriver to be done prior to pushing the operation to
consensus for replication. Follow up patches will move it to be done
within consensus itself (though not necessarily managed by any of the
consensus classes).
2) It makes all operations be "operations at a timestamp", making
all operations have the same behavior within mvcc independently of
whether they were started at the leader or at a follower.

Follow up patches will completely remove the Mvcc APIs for automatic
safe time advancement and timestamp assignment and will introduce
the new entity responsible for "safe time".

Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
---
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
7 files changed, 56 insertions(+), 27 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ba7212f9211f585d4bef00e5ccfc24d5eece224
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-798 (part 1) - Unify leader/follower mvcc behavior

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

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


Patch Set 6:

I reviewed this, don't have anything to add to Todd's comments

-- 
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No