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/03 11:12:21 UTC
[kudu-CR] WIP: KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
David Ribeiro Alves has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4929
Change subject: WIP: KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
......................................................................
WIP: KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
Previously this method would set a bool to true if a row was
deleted and left it unchanged if it wasn't. This is no longer enough
with reinserts.
For this particular case we go through the redo deltas in reverse
order, meaning we might find a reinsert in the last redo deltas. In
this case just not setting the bool is not enough, we need to be
able to tell the caller unequivocally that the row is alive.
This patch does this by introducing an enum: DeletionStatus. This
enum has three possible values: DELETED, if the last mutation we
found for the row was a DELETE; NOT_DELETED if the last mutation
we found for the row was a REINSERT; UNKNOWN if we didn't find
any mutation for the row.
This also adds a new visitor to DeltaFileIterator to set the new
enum and simplifies this path. We were previously using the same
path as when we apply deletes to a whole column going through the
updates in order. Not only is this not enough with reinserts when
traversing the redos in reverse order, but this path was also overly
complex. For instance it continuouly checked the validity of the
redo regarding the snapshot, which is not needed since we use an
"include all mutations" snapshhot in this case.
WIP as this needs a directed test.
Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
---
M src/kudu/common/row_changelist-test.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
7 files changed, 105 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/4929/1
--
To view, visit http://gerrit.cloudera.org:8080/4929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
......................................................................
Patch Set 3:
This will be updated but is no longer part of the REINSERT sequence
--
To view, visit http://gerrit.cloudera.org:8080/4929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
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/4929
to look at the new patch set (#2).
Change subject: WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
......................................................................
WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
This makes it so that CheckRowDeleted() returns an enum instead of
just setting a bool. The new enum allows to stop looking for a delete
delta if we find an update for a row.
This also adds a new visitor to DeltaFileIterator to set the new
enum and simplifies this path, which was overly complex and branchy.
For instance it continuouly checked the validity of the redo regarding
the snapshot, which is not needed since we use an "include all mutations"
snapshhot in this case.
WIP This likely needs a simple test that asserts we stop looking when we
find an update.
Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
---
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
6 files changed, 104 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/4929/2
--
To view, visit http://gerrit.cloudera.org:8080/4929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
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] WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
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/4929
to look at the new patch set (#3).
Change subject: WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
......................................................................
WIP KUDU-237 (part 2) - Make DeltaStore::CheckRowDeleted() return an enum
This makes it so that CheckRowDeleted() returns an enum instead of
just setting a bool. The new enum allows to stop looking for a delete
delta if we find an update for a row.
This also adds a new visitor to DeltaFileIterator to set the new
enum and simplifies this path, which was overly complex and branchy.
For instance it continuouly checked the validity of the redo regarding
the snapshot, which is not needed since we use an "include all mutations"
snapshhot in this case.
WIP This likely needs a simple test that asserts we stop looking when we
find an update.
Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
---
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
6 files changed, 109 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/4929/3
--
To view, visit http://gerrit.cloudera.org:8080/4929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
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] NOT FOR REVIEW - Make DeltaStore::CheckRowDeleted() return an enum
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/4929
to look at the new patch set (#4).
Change subject: NOT FOR REVIEW - Make DeltaStore::CheckRowDeleted() return an enum
......................................................................
NOT FOR REVIEW - Make DeltaStore::CheckRowDeleted() return an enum
This makes it so that CheckRowDeleted() returns an enum instead of
just setting a bool. The new enum allows to stop looking for a delete
delta if we find an update for a row.
This also adds a new visitor to DeltaFileIterator to set the new
enum and simplifies this path, which was overly complex and branchy.
For instance it continuouly checked the validity of the redo regarding
the snapshot, which is not needed since we use an "include all mutations"
snapshhot in this case.
WIP This likely needs a simple test that asserts we stop looking when we
find an update.
Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
---
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
6 files changed, 109 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/4929/4
--
To view, visit http://gerrit.cloudera.org:8080/4929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5680c0c719cc7d367ea870b2ccf61b81e5fb309
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot