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

2018-07-25 Thread Adar Dembo (Code Review)
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

2018-07-25 Thread Mike Percy (Code Review)
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

2018-07-24 Thread Todd Lipcon (Code Review)
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

2018-07-24 Thread Adar Dembo (Code Review)
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

2018-07-24 Thread Adar Dembo (Code Review)
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

2018-07-24 Thread Todd Lipcon (Code Review)
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

2018-07-24 Thread Adar Dembo (Code Review)
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

2018-07-24 Thread Adar Dembo (Code Review)
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

2018-07-23 Thread Adar Dembo (Code Review)
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

2018-07-20 Thread Grant Henke (Code Review)
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

2018-07-19 Thread Adar Dembo (Code Review)
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

2018-07-19 Thread Adar Dembo (Code Review)
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

2018-07-19 Thread Mike Percy (Code Review)
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

2018-07-19 Thread Adar Dembo (Code Review)
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