You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/02/24 20:05:49 UTC

[kudu-CR] test: add more natural test for KUDU-2233 

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17114


Change subject: test: add more natural test for KUDU-2233
......................................................................

test: add more natural test for KUDU-2233

I have a patch in-flight that touches an area of the code around where
we expect the infamous KUDU-2233 crash. Before merging it, I'd like to
ensure the graceful handling of this corruption is unaffected,
especially in cases where data has previously been corrupted and we've
just upgraded to a newer version of Kudu.

This patch adds such a test case, where data is corrupted but does not
result in a crash, and the tserver is restarted with bits that handle
corruption gracefully. In doing so, this patch also adds an off switch
to all of the guardrails we installed for KUDU-2233.

Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_replica.cc
7 files changed, 156 insertions(+), 17 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: DEFINE_bool(dcheck_on_kudu_2233_invariants, true
> Yes, this one different as I can see: it's not quite used in NDEBUG case, w
I am fine with the current code, but if Alexey thinks differentiate the flag to be not exposed in NDEBUG case can help with the performance of execution. Then I am also good with that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 20:03:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

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

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

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

Change subject: test: add more natural test for KUDU-2233
......................................................................

test: add more natural test for KUDU-2233

I have a patch in-flight that touches an area of the code around where
we expect the infamous KUDU-2233 crash. Before merging it, I'd like to
ensure the graceful handling of this corruption is unaffected,
especially in cases where data has previously been corrupted and we've
just upgraded to a newer version of Kudu.

This patch adds such a test case, where data is corrupted but does not
result in a crash, and the tserver is restarted with bits that handle
corruption gracefully. In doing so, this patch also adds an off switch
to all of the guardrails we installed for KUDU-2233.

Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_replica.cc
7 files changed, 173 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/17114/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 4:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: #ifndef NDEBUG
> I am fine with the current code, but if Alexey thinks differentiate the fla
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 20:59:58 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 4: Code-Review+2

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: #ifndef NDEBUG
> I am fine with the current code, but if Alexey thinks differentiate the fla
Thank you!


http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: #ifndef NDEBUG
> Done
Apart from performance concern (I guess most likely the performance difference is negligible), I was not sure whether we want to keep a flag which does nothing.  It was not a major point anyways, so I'm also good with either keeping it or removing.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 21:25:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 3:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: DEFINE_bool(dcheck_on_kudu_2233_invariants, true
> I can surround this with some ifndef sure, but we do have several test-only
Left this as is for now.


http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@78
PS2, Line 78: caused by KUDU-2233
> nit: maybe specify this is test only as well.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 00:53:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................

test: add more natural test for KUDU-2233

I have a patch in-flight that touches an area of the code around where
we expect the infamous KUDU-2233 crash. Before merging it, I'd like to
ensure the graceful handling of this corruption is unaffected,
especially in cases where data has previously been corrupted and we've
just upgraded to a newer version of Kudu.

This patch adds such a test case, where data is corrupted but does not
result in a crash, and the tserver is restarted with bits that handle
corruption gracefully. In doing so, this patch also adds an off switch
to all of the guardrails we installed for KUDU-2233.

Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Reviewed-on: http://gerrit.cloudera.org:8080/17114
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_replica.cc
7 files changed, 173 insertions(+), 22 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Andrew Wong: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: add more natural test for KUDU-2233

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

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

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

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

Change subject: test: add more natural test for KUDU-2233
......................................................................

test: add more natural test for KUDU-2233

I have a patch in-flight that touches an area of the code around where
we expect the infamous KUDU-2233 crash. Before merging it, I'd like to
ensure the graceful handling of this corruption is unaffected,
especially in cases where data has previously been corrupted and we've
just upgraded to a newer version of Kudu.

This patch adds such a test case, where data is corrupted but does not
result in a crash, and the tserver is restarted with bits that handle
corruption gracefully. In doing so, this patch also adds an off switch
to all of the guardrails we installed for KUDU-2233.

Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_replica.cc
7 files changed, 160 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/17114/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: DEFINE_bool(dcheck_on_kudu_2233_invariants, true
> I can surround this with some ifndef sure, but we do have several test-only
Yes, this one different as I can see: it's not quite used in NDEBUG case, while other test-only flags are used in both debug and NDEBUG case.

At lines 308-311 and 316-319 the usage of this flag reduces to an empty if() clause in NDEBUG mode.  Why to keep such a flag then in NDEBUG case?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 16:56:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: add more natural test for KUDU-2233

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: test: add more natural test for KUDU-2233
......................................................................

test: add more natural test for KUDU-2233

I have a patch in-flight that touches an area of the code around where
we expect the infamous KUDU-2233 crash. Before merging it, I'd like to
ensure the graceful handling of this corruption is unaffected,
especially in cases where data has previously been corrupted and we've
just upgraded to a newer version of Kudu.

This patch adds such a test case, where data is corrupted but does not
result in a crash, and the tserver is restarted with bits that handle
corruption gracefully. In doing so, this patch also adds an off switch
to all of the guardrails we installed for KUDU-2233.

Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_replica.cc
7 files changed, 157 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: DEFINE_bool(dcheck_on_kudu_2233_invariants, true
> Since this is a test-only flag (correct me if I'm wrong), maybe put the def
I can surround this with some ifndef sure, but we do have several test-only flags today. Is this one different?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 24 Feb 2021 22:37:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

LGTM, thanks a lot for providing this patch test upgrade cases.

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@78
PS2, Line 78: caused by KUDU-2233
nit: maybe specify this is test only as well.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 25 Feb 2021 18:45:36 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 4: Verified+1

Unrelated failure of LocationAwareRebalancingParamTest.Rebalance/idx8_rf4_t8_l_3_1_1_1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 21:56:50 +0000
Gerrit-HasComments: No

[kudu-CR] test: add more natural test for KUDU-2233

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

Change subject: test: add more natural test for KUDU-2233
......................................................................


Patch Set 2:

(1 comment)

I quickly looked through the change.  I didn't get to the essence yet, but posted a question w.r.t. defining and using the newly introducing test flags.

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

http://gerrit.cloudera.org:8080/#/c/17114/2/src/kudu/tablet/compaction.cc@77
PS2, Line 77: DEFINE_bool(dcheck_on_kudu_2233_invariants, true
Since this is a test-only flag (correct me if I'm wrong), maybe put the definition of this flag and its usages in the code under #ifndef correspondingly?  Otherwise, why to have such flags in a release binary?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac3ad0a1b6bb9b17d5b6a901dc5bba79c0840fa
Gerrit-Change-Number: 17114
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 24 Feb 2021 21:53:07 +0000
Gerrit-HasComments: Yes