You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/10/15 21:53:42 UTC
[kudu-CR](branch-1.8.x) KUDU-2463 pt 3: don't scan if MVCC hasn't moved
Hello Mike Percy, Kudu Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11690
to review the following change.
Change subject: KUDU-2463 pt 3: don't scan if MVCC hasn't moved
......................................................................
KUDU-2463 pt 3: don't scan if MVCC hasn't moved
In cases when a tablet bootstrap yields an MvccManager whose safe time
has not been advanced (e.g. if there are no write ops in the WAL), and
the tablet has otherwise not bumped its MVCC timestamps (e.g. if it has
not yet elected a leader), a scan (whose correctness depends on the
MvccManager to determine what transactions have been applied) will
return incorrect results.
In the same way that we prevent compactions from occuring if MVCC's
timestamps have not been moved, this patch prevents incorrect results
from being returend from a scan by returning an error that can be
retried elsewhere.
New tests are added to attempt to scan in this state, verifying that we
get an error. A couple of tests that use the mock clock are also updated
so the initial timestamp assigned to a no-op is a more organic, non-zero
timestamp.
Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Reviewed-on: http://gerrit.cloudera.org:8080/11428
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_service.cc
12 files changed, 231 insertions(+), 62 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/11690/1
--
To view, visit http://gerrit.cloudera.org:8080/11690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.8.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Gerrit-Change-Number: 11690
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR](branch-1.8.x) KUDU-2463 pt 3: don't scan if MVCC hasn't moved
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11690 )
Change subject: KUDU-2463 pt 3: don't scan if MVCC hasn't moved
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.8.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Gerrit-Change-Number: 11690
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 15 Oct 2018 22:16:26 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.8.x) KUDU-2463 pt 3: don't scan if MVCC hasn't moved
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11690 )
Change subject: KUDU-2463 pt 3: don't scan if MVCC hasn't moved
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.8.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Gerrit-Change-Number: 11690
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 16 Oct 2018 00:01:32 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.8.x) KUDU-2463 pt 3: don't scan if MVCC hasn't moved
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11690 )
Change subject: KUDU-2463 pt 3: don't scan if MVCC hasn't moved
......................................................................
KUDU-2463 pt 3: don't scan if MVCC hasn't moved
In cases when a tablet bootstrap yields an MvccManager whose safe time
has not been advanced (e.g. if there are no write ops in the WAL), and
the tablet has otherwise not bumped its MVCC timestamps (e.g. if it has
not yet elected a leader), a scan (whose correctness depends on the
MvccManager to determine what transactions have been applied) will
return incorrect results.
In the same way that we prevent compactions from occuring if MVCC's
timestamps have not been moved, this patch prevents incorrect results
from being returend from a scan by returning an error that can be
retried elsewhere.
New tests are added to attempt to scan in this state, verifying that we
get an error. A couple of tests that use the mock clock are also updated
so the initial timestamp assigned to a no-op is a more organic, non-zero
timestamp.
Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Reviewed-on: http://gerrit.cloudera.org:8080/11428
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/11690
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_service.cc
12 files changed, 231 insertions(+), 62 deletions(-)
Approvals:
Grant Henke: Looks good to me, approved
Kudu Jenkins: Verified
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.8.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Idc0f77673e1f04a34ab1f5c1930bbaa2498b39bf
Gerrit-Change-Number: 11690
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>