You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org> on 2019/10/14 18:20:24 UTC
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14428
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
[util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
RLE encoding may encode "literally" when it doesn't find sufficient
repeated values.
When the number of values are not multiple of 8, literal count is
rounded up to multiple of 8. After reading the last value,
another attempt is made to read the value in GetNextRun() method.
This is beyond the max_bytes in the BitReader which returns false
and causes DCHECK() failure in GetNextRun() method.
For further details see JIRA KUDU-2968.
Fix is to check for the return value of BitReader::_GetValue()
and return without modifying further internal state of RleDecoder.
Similarly if an attempt is made by the caller of
RleDecoder::GetNextRun() to read after having read the last value
then BitReader::GetValue() would also return false again
leading to DCHECK() failure.
Tests:
- Added a test case for various int data types
that inserts wide range of non-repeated values
including non-multiple-of-8 and verfies GetNextRun() method.
Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Reviewed-on: http://gerrit.cloudera.org:8080/14405
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
(cherry picked from commit 25d92848ad7dc0852b23b9e967aa695005b8b374)
---
M src/kudu/util/rle-encoding.h
M src/kudu/util/rle-test.cc
2 files changed, 52 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/14428/1
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14428 )
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
[util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
RLE encoding may encode "literally" when it doesn't find sufficient
repeated values.
When the number of values are not multiple of 8, literal count is
rounded up to multiple of 8. After reading the last value,
another attempt is made to read the value in GetNextRun() method.
This is beyond the max_bytes in the BitReader which returns false
and causes DCHECK() failure in GetNextRun() method.
For further details see JIRA KUDU-2968.
Fix is to check for the return value of BitReader::_GetValue()
and return without modifying further internal state of RleDecoder.
Similarly if an attempt is made by the caller of
RleDecoder::GetNextRun() to read after having read the last value
then BitReader::GetValue() would also return false again
leading to DCHECK() failure.
Tests:
- Added a test case for various int data types
that inserts wide range of non-repeated values
including non-multiple-of-8 and verfies GetNextRun() method.
Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Reviewed-on: http://gerrit.cloudera.org:8080/14405
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
(cherry picked from commit 25d92848ad7dc0852b23b9e967aa695005b8b374)
Reviewed-on: http://gerrit.cloudera.org:8080/14428
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/util/rle-encoding.h
M src/kudu/util/rle-test.cc
2 files changed, 52 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: merged
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14428 )
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
Patch Set 1: Code-Review+1
Unrelated test failure in:
* raft_consensus_election-itest.0
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: comment
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Oct 2019 19:13:38 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14428 )
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: comment
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Oct 2019 19:13:49 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/14428 )
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
[kudu-CR](branch-1.11.x) [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14428 )
Change subject: [util] KUDU-2968 Fix RleDecoder::GetNextRun() in case of literal runs
......................................................................
Patch Set 1: Verified+1 -Code-Review
--
To view, visit http://gerrit.cloudera.org:8080/14428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.11.x
Gerrit-MessageType: comment
Gerrit-Change-Id: If1703efd5307c66e282a2d2d7600dc4658c252df
Gerrit-Change-Number: 14428
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Oct 2019 19:13:46 +0000
Gerrit-HasComments: No