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 2019/06/06 08:04:14 UTC

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

Hello Will Berkeley, Mike Percy, Grant Henke,

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

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

to review the following change.


Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................

KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

The counterpart to the MRS patch, this one makes an equivalent change to the
DRS diff scanning logic. It's far more complicated than the MRS version, for
several reasons:
1. A row's history is fragmented across multiple delta stores.
2. UNDOs' view of time is inverted, complicating attempts at tracing a row's
   liveness across stores.

To implement this, we need to reconsider how the DeltaApplier initializes
its selection vector. The overall structure remains the same:
1. The DeltaApplier sets up data structure to capture selectivity info.
2. It invokes a method on each store to add that store's selectivity info
   to the structure.
3. Finally, converts the structure into the selection vector.

Before this patch, it was sufficient for the data structure to be the
selection vector itself. But now the selection vector is insufficient.
Instead we use a new data structure which tracks, for each row, the row's
oldest and newest selected deltas. Armed with this information, we can omit
unobservable rows at conversion time: if a row was dead before the oldest
delta and after the newest delta, its lifetime exists entirely within the
diff scan and it should be omitted from the results.

Much of the complexity here lies with the need to totally order all deltas,
regardless of whether they're UNDOs or REDOs, even if they share the same
timestamps (fuzz-itest is a super effective way of testing this). The
ordering starts with timestamps, but then orders deltas in some potentially
non-obvious ways based on Kudu invariants:
1. Timestamps (obviously)
2. UNDOs, then REDOs. Within a rowset all UNDOs come before REDOs.
3. The order in which the deltas were observed in the store. This is
   equivalent to the order in which they are applied.

Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_iterator_merger.cc
M src/kudu/tablet/delta_iterator_merger.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
15 files changed, 402 insertions(+), 60 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/13536/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 3:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.h@133
PS2, Line 133:   // Comparator that establishes a total ordering amongst Deltas.
Obvious detail worth mentioning: "a total ordering amongst Deltas for the same row"


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@114
PS2, Line 114:     // delta type    | oldest | newest
Thanks for including this helpful block comment.


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@156
PS2, Line 156: existing->newest
Why include newest? When could existing->oldest not be less than existing->newest?


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@158
PS2, Line 158: existing->oldest
same


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@278
PS2, Line 278:                                           Traits::kType,
nit: Seems worth writing a constructor to guarantee all fields get set so we don't have to worry about it at the call sites.


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@281
PS2, Line 281: decoder.get_type() };
Should be:

  reinterpret_cast<size_t>(this)

or similar; also maybe consider using uintptr_t



-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 07:02:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 4:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.h@133
PS2, Line 133:   // Comparator that establishes a total ordering amongst Deltas.
> Obvious detail worth mentioning: "a total ordering amongst Deltas for the s
Done


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@114
PS2, Line 114:     // delta type    | oldest | newest
> Thanks for including this helpful block comment.
Ack


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@156
PS2, Line 156: existing->newest
> Why include newest? When could existing->oldest not be less than existing->
Good question. Looking at it again I agree that it can be simplified.


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@158
PS2, Line 158: existing->oldest
> same
Done


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@278
PS2, Line 278:                                           Traits::kType,
> nit: Seems worth writing a constructor to guarantee all fields get set so w
Gonna pass. There's only this one construction site, and I get a compiler error if I remove any of the fields except the last one. Plus adding a constructor means I need to rename all of the fields in order to disambiguate vs. the constructor args.


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@281
PS2, Line 281: decoder.get_type() };
> Should be:
Hmm, but within Delta I do want to treat these as int64_t (i.e. a 64-bit integer that uniquely identifies a delta store). That is, I want to hide the fact that this is a pointer to something. And I don't think size_t makes sense as this isn't a byte or length quantity.



-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 07:49:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 4:

Just so you don't miss it: I posted comments on rev 2 right before you pushed the rev 4 rebase.


-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 07:19:38 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Grant Henke, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#7).

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................

KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

The counterpart to the MRS patch, this one makes an equivalent change to the
DRS diff scanning logic. It's far more complicated than the MRS version, for
several reasons:
1. A row's history is fragmented across multiple delta stores.
2. UNDOs' view of time is inverted, complicating attempts at tracing a row's
   liveness across stores.

To implement this, we need to reconsider how the DeltaApplier initializes
its selection vector. The overall structure remains the same:
1. The DeltaApplier sets up data structure to capture selectivity info.
2. It invokes a method on each store to add that store's selectivity info
   to the structure.
3. Finally, converts the structure into the selection vector.

Before this patch, it was sufficient for the data structure to be the
selection vector itself. But now the selection vector is insufficient.
Instead we use a new data structure which tracks, for each row, the row's
oldest and newest selected deltas. Armed with this information, we can omit
unobservable rows at conversion time: if a row was dead before the oldest
delta and after the newest delta, its lifetime exists entirely within the
diff scan and it should be omitted from the results.

Much of the complexity here lies with the need to totally order all deltas,
regardless of whether they're UNDOs or REDOs, even if they share the same
timestamps (fuzz-itest is a super effective way of testing this). The
ordering starts with timestamps, but then orders deltas in some potentially
non-obvious ways based on Kudu invariants:
1. Timestamps (obviously)
2. UNDOs, then REDOs. Within a rowset all UNDOs come before REDOs.
3. The order in which the deltas were observed in the store. This is
   equivalent to the order in which they are applied.

Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_iterator_merger.cc
M src/kudu/tablet/delta_iterator_merger.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
15 files changed, 387 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/13536/7
-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13536 )

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................

KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

The counterpart to the MRS patch, this one makes an equivalent change to the
DRS diff scanning logic. It's far more complicated than the MRS version, for
several reasons:
1. A row's history is fragmented across multiple delta stores.
2. UNDOs' view of time is inverted, complicating attempts at tracing a row's
   liveness across stores.

To implement this, we need to reconsider how the DeltaApplier initializes
its selection vector. The overall structure remains the same:
1. The DeltaApplier sets up data structure to capture selectivity info.
2. It invokes a method on each store to add that store's selectivity info
   to the structure.
3. Finally, converts the structure into the selection vector.

Before this patch, it was sufficient for the data structure to be the
selection vector itself. But now the selection vector is insufficient.
Instead we use a new data structure which tracks, for each row, the row's
oldest and newest selected deltas. Armed with this information, we can omit
unobservable rows at conversion time: if a row was dead before the oldest
delta and after the newest delta, its lifetime exists entirely within the
diff scan and it should be omitted from the results.

Much of the complexity here lies with the need to totally order all deltas,
regardless of whether they're UNDOs or REDOs, even if they share the same
timestamps (fuzz-itest is a super effective way of testing this). The
ordering starts with timestamps, but then orders deltas in some potentially
non-obvious ways based on Kudu invariants:
1. Timestamps (obviously)
2. UNDOs, then REDOs. Within a rowset all UNDOs come before REDOs.
3. The order in which the deltas were observed in the store. This is
   equivalent to the order in which they are applied.

Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Reviewed-on: http://gerrit.cloudera.org:8080/13536
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_iterator_merger.cc
M src/kudu/tablet/delta_iterator_merger.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
15 files changed, 387 insertions(+), 60 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
Gerrit-PatchSet: 8
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Grant Henke, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................

KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

The counterpart to the MRS patch, this one makes an equivalent change to the
DRS diff scanning logic. It's far more complicated than the MRS version, for
several reasons:
1. A row's history is fragmented across multiple delta stores.
2. UNDOs' view of time is inverted, complicating attempts at tracing a row's
   liveness across stores.

To implement this, we need to reconsider how the DeltaApplier initializes
its selection vector. The overall structure remains the same:
1. The DeltaApplier sets up data structure to capture selectivity info.
2. It invokes a method on each store to add that store's selectivity info
   to the structure.
3. Finally, converts the structure into the selection vector.

Before this patch, it was sufficient for the data structure to be the
selection vector itself. But now the selection vector is insufficient.
Instead we use a new data structure which tracks, for each row, the row's
oldest and newest selected deltas. Armed with this information, we can omit
unobservable rows at conversion time: if a row was dead before the oldest
delta and after the newest delta, its lifetime exists entirely within the
diff scan and it should be omitted from the results.

Much of the complexity here lies with the need to totally order all deltas,
regardless of whether they're UNDOs or REDOs, even if they share the same
timestamps (fuzz-itest is a super effective way of testing this). The
ordering starts with timestamps, but then orders deltas in some potentially
non-obvious ways based on Kudu invariants:
1. Timestamps (obviously)
2. UNDOs, then REDOs. Within a rowset all UNDOs come before REDOs.
3. The order in which the deltas were observed in the store. This is
   equivalent to the order in which they are applied.

Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_iterator_merger.cc
M src/kudu/tablet/delta_iterator_merger.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
15 files changed, 387 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/13536/5
-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@278
PS2, Line 278: 
> Gonna pass. There's only this one construction site, and I get a compiler e
Ack


http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@281
PS2, Line 281:  false;
> Hmm, but within Delta I do want to treat these as int64_t (i.e. a 64-bit in
Well, that's fair that the type doesn't really matter. But the cast is wrong, it should be reinterpret_cast<int64_t>(this) -- the way it is right now, it will read the first 64 bytes of the object which is opts_.projection as a 64-bit int, but we're trying to read the pointer as an int.



-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 17:30:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 22:09:46 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2809 (4/6): skip unobservable rows when iterating a DRS

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

Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc
File src/kudu/tablet/delta_store.cc:

http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@281
PS2, Line 281:  false;
> Well, that's fair that the type doesn't really matter. But the cast is wron
Ah good point. Will change to reinterpret_cast<int64_t>.



-- 
To view, visit http://gerrit.cloudera.org:8080/13536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226
Gerrit-Change-Number: 13536
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 (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 21:34:16 +0000
Gerrit-HasComments: Yes