[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. memrowset: support iteration with include_deleted_rows This is another piece in the incremental backup puzzle. The idea is that when taking an incremental backup, the scan will include rows that have been deleted. The backup will figure out which rows were deleted by including a special "is deleted" virtual column in the projection. This commit introduces a new iterator option that can be used to include deleted rows, and uses it in the MemRowSet. Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Reviewed-on: http://gerrit.cloudera.org:8080/10929 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h 4 files changed, 80 insertions(+), 16 deletions(-) Approvals: Todd Lipcon: Looks good to me, but someone else must approve Kudu Jenkins: Verified Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 25 Jul 2018 20:14:47 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 25 Jul 2018 02:47:21 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc File src/kudu/tablet/memrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc@585 PS5, Line 585: INSTANTIATE_TEST_CASE_P(MrsConfigurations, ParameterizedTestMemRowSet, : ::testing::Bool()); > mind naming this parameterization something more useful to indicate what th Done http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc@665 PS5, Line 665: ASSERT_OK(InsertRow(mrs.get(), "row 1", 0)); > I think its' worth adding a third row which has been inserted, deleted, and Done http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset.h File src/kudu/tablet/memrowset.h: http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset.h@557 PS5, Line 557: // The 'was_deleted' parameter will be set to true if the last mutation walked > ah, this is starting to look a bit closer to my proposal to add an enum out Technically they're all possible, but I think this is still cleaner with your enum suggestion from the first patch. Take a look. -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 25 Jul 2018 02:30:33 + Gerrit-HasComments: Yes
[kudu-CR] memrowset: support iteration with include deleted rows
Hello Mike Percy, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10929 to look at the new patch set (#6). Change subject: memrowset: support iteration with include_deleted_rows .. memrowset: support iteration with include_deleted_rows This is another piece in the incremental backup puzzle. The idea is that when taking an incremental backup, the scan will include rows that have been deleted. The backup will figure out which rows were deleted by including a special "is deleted" virtual column in the projection. This commit introduces a new iterator option that can be used to include deleted rows, and uses it in the MemRowSet. Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h 4 files changed, 80 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/10929/6 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc File src/kudu/tablet/memrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc@585 PS5, Line 585: INSTANTIATE_TEST_CASE_P(MrsConfigurations, ParameterizedTestMemRowSet, : ::testing::Bool()); mind naming this parameterization something more useful to indicate what the value of the bool means? and perhaps add a comment? http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset-test.cc@665 PS5, Line 665: ASSERT_OK(InsertRow(mrs.get(), "row 1", 0)); I think its' worth adding a third row which has been inserted, deleted, and reinserted and maybe some other rows which include some updates in there, too? http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset.h File src/kudu/tablet/memrowset.h: http://gerrit.cloudera.org:8080/#/c/10929/5/src/kudu/tablet/memrowset.h@557 PS5, Line 557: // The 'was_deleted' parameter will be set to true if the last mutation walked ah, this is starting to look a bit closer to my proposal to add an enum out-param instead of the two bools. Are all four combinations of these two bools possible? If so, maybe two bools is better than one enum. -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 24 Jul 2018 22:42:45 + Gerrit-HasComments: Yes
[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 5: Verified+1 Overriding Jenkins, unrelated Java test failures. -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 24 Jul 2018 18:00:57 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Hello Mike Percy, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10929 to look at the new patch set (#5). Change subject: memrowset: support iteration with include_deleted_rows .. memrowset: support iteration with include_deleted_rows This is another piece in the incremental backup puzzle. The idea is that when taking an incremental backup, the scan will include rows that have been deleted. The backup will figure out which rows were deleted by including a special "is deleted" virtual column in the projection. This commit introduces a new iterator option that can be used to include deleted rows, and uses it in the MemRowSet. Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h 5 files changed, 77 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/10929/5 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 20 Jul 2018 19:38:22 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 4: Verified+1 Overriding Jenkins, unrelated test failures. -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 20 Jul 2018 00:00:28 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 ) Change subject: memrowset: support iteration with include_deleted_rows .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 19 Jul 2018 22:11:01 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with include deleted rows
Hello Mike Percy, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10929 to look at the new patch set (#4). Change subject: memrowset: support iteration with include_deleted_rows .. memrowset: support iteration with include_deleted_rows This is another piece in the incremental backup puzzle. The idea is that when taking an incremental backup, the scan will include rows that have been deleted. The backup will figure out which rows were deleted by including a special "is deleted" virtual column in the projection. This commit introduces a new iterator option that can be used to include deleted rows, and uses it in the MemRowSet. Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h 5 files changed, 77 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/10929/4 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon