You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2018/07/12 19:28:26 UTC

[kudu-CR] memrowset: support iteration with omit deleted rowslse 

Hello Mike Percy, Grant Henke, Todd Lipcon,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................

memrowset: support iteration with omit_deleted_rows=false

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 "was 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/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: newchange
Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1
Gerrit-Change-Number: 10929
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <to...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 24 Jul 2018 18:00:57 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10929/2/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:

http://gerrit.cloudera.org:8080/#/c/10929/2/src/kudu/tablet/memrowset.h@562
PS2, Line 562: should_be_excluded
> how about should_be_included?
Made the change, though not sure if it's any more or less clear.



-- 
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: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 19 Jul 2018 18:52:47 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 20 Jul 2018 19:38:22 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 20 Jul 2018 00:00:28 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Jul 2018 02:30:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Jul 2018 02:47:21 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


Patch Set 2:

(1 comment)

overall looks good

http://gerrit.cloudera.org:8080/#/c/10929/2/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:

http://gerrit.cloudera.org:8080/#/c/10929/2/src/kudu/tablet/memrowset.h@562
PS2, Line 562: should_be_excluded
how about should_be_included?



-- 
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: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 19 Jul 2018 18:37:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 19 Jul 2018 22:11:01 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


Patch Set 2: Verified+1

Overriding Jenkins, unrelated test failure.


-- 
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: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 18 Jul 2018 02:23:42 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10929/3/src/kudu/tablet/memrowset-test.cc
File src/kudu/tablet/memrowset-test.cc:

http://gerrit.cloudera.org:8080/#/c/10929/3/src/kudu/tablet/memrowset-test.cc@618
PS3, Line 618: omit
maybe include instead of omit to avoid setting true for a negative?



-- 
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: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 19 Jul 2018 19:05:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10929/3/src/kudu/tablet/memrowset-test.cc
File src/kudu/tablet/memrowset-test.cc:

http://gerrit.cloudera.org:8080/#/c/10929/3/src/kudu/tablet/memrowset-test.cc@618
PS3, Line 618: omit
> maybe include instead of omit to avoid setting true for a negative?
Done



-- 
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: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 19 Jul 2018 20:36:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 (#3).

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................

memrowset: support iteration with omit_deleted_rows=false

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 "was 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/3
-- 
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: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with omit deleted rowslse

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/10929 )

Change subject: memrowset: support iteration with omit_deleted_rows=false
......................................................................


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: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Jul 2018 20:14:47 +0000
Gerrit-HasComments: No

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 24 Jul 2018 22:42:45 +0000
Gerrit-HasComments: Yes

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] memrowset: support iteration with include deleted rows

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>