You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2022/07/20 13:44:36 UTC

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18761


Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................

KUDU-3384: Handling IncrementEncodedKey failure

While using lower/upper of a DRS to optimize scan at DRS level, we
should handle the cases where a upper bound key cannot be incremented.
This patch fixes it, enables the reproduction scenario and also adds
a regression test.

Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
3 files changed, 58 insertions(+), 6 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................


Patch Set 2: Verified+1

unrelated test failure (ASAN): ReplicatedAlterTableTest.AlterReplicationFactorWhileWriting


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 14:36:38 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................

KUDU-3384: Handling IncrementEncodedKey failure

While using lower/upper of a DRS to optimize scan at DRS level, we
should handle the cases where a upper bound key cannot be incremented.
This patch fixes it, enables the reproduction scenario and also adds
a regression test.

Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
3 files changed, 61 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/18761/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18761
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................

KUDU-3384: Handling IncrementEncodedKey failure

While using lower/upper of a DRS to optimize scan at DRS level, we
should handle the cases where a upper bound key cannot be incremented.
This patch fixes it, enables the reproduction scenario and also adds
a regression test.

Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Reviewed-on: http://gerrit.cloudera.org:8080/18761
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
3 files changed, 61 insertions(+), 6 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................


Patch Set 1: Code-Review+1

(3 comments)

Thank you for fixing this!

Overall looks good, just a few nits.

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc
File src/kudu/tablet/cfile_set-test.cc:

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc@655
PS1, Line 655: Regression
nit: Regression test


http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc@657
PS1, Line 657: const
nit: could be constexpr?


http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set.cc@446
PS1, Line 446: EncodedKey::IncrementEncodedKey
This might return Status::RuntimeError() due to out-of-memory condition.  Could you add an explanation why it's possible to continue in such case even if the upper bound is something less that maximum possible value for the primary key and still get correct results for the scan?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 20 Jul 2022 19:06:29 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc
File src/kudu/tablet/cfile_set-test.cc:

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc@655
PS1, Line 655: Regression
> nit: Regression test
Done


http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set-test.cc@657
PS1, Line 657: const
> nit: could be constexpr?
Done


http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:

http://gerrit.cloudera.org:8080/#/c/18761/1/src/kudu/tablet/cfile_set.cc@446
PS1, Line 446: e exclusive_upper_bound_key onl
> This might return Status::RuntimeError() due to out-of-memory condition.  C
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 04:14:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

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

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................


Patch Set 2: Code-Review+2

Thank you for the fix!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 14:36:52 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3384: Handling IncrementEncodedKey failure

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: KUDU-3384: Handling IncrementEncodedKey failure
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/18761
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I22c48f9893364c3fad5597431e4acfbcb2a4b43d
Gerrit-Change-Number: 18761
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>