You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2022/08/27 01:00:42 UTC

[kudu-CR] [test] instantiate TsRecoveryITestDeathTest

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18920


Change subject: [test] instantiate TsRecoveryITestDeathTest
......................................................................

[test] instantiate TsRecoveryITestDeathTest

As it turned out, the parameterized TsRecoveryITestDeathTest was not
instantiated.  Digging into the history of changes, I found that [1]
introduced TEST_F --> TEST_P change without adding corresponding
instantiations for the parameterized test suite.

I fixed the test by adapting it for the parameterization it went through
and added the missing instantiation macro.  In addition, [2] introduced
an extra DCHECK() into TabletReplica::FinishConsensusOnlyRound().  Once
instantiated, the test started hitting the DCHECK().  To address that
issue, I updated the test-only function AppendNoOpsToLogSync() in
log-test-base.h to fill in the 'noop_request' field along with setting
the 'op_type' field to NO_OP.  With that, since I added some extra data
into a NO_OP log record, I also updated the reference size of the WAL
segment in Raft consensus log test.

[1] https://github.com/apache/kudu/commit/371a00b7aeba244aa63d92bf479cbb356b4dfbca
[2] https://github.com/apache/kudu/commit/bc817a44867c586bf4e0539aa564b282c666a49d

Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
---
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
3 files changed, 8 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
Gerrit-Change-Number: 18920
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>

[kudu-CR] [test] instantiate TsRecoveryITestDeathTest

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

Change subject: [test] instantiate TsRecoveryITestDeathTest
......................................................................

[test] instantiate TsRecoveryITestDeathTest

As it turned out, the parameterized TsRecoveryITestDeathTest was not
instantiated.  Digging into the history of changes, I found that [1]
introduced TEST_F --> TEST_P change without adding corresponding
instantiations for the parameterized test suite.

I fixed the test by adapting it for the parameterization it went through
and added the missing instantiation macro.  In addition, [2] introduced
an extra DCHECK() into TabletReplica::FinishConsensusOnlyRound().  Once
instantiated, the test started hitting the DCHECK().  To address that
issue, I updated the test-only function AppendNoOpsToLogSync() in
log-test-base.h to fill in the 'noop_request' field along with setting
the 'op_type' field to NO_OP.  With that, since I added some extra data
into a NO_OP log record, I also updated the reference size of the WAL
segment in Raft consensus log test.

[1] https://github.com/apache/kudu/commit/371a00b7aeba244aa63d92bf479cbb356b4dfbca
[2] https://github.com/apache/kudu/commit/bc817a44867c586bf4e0539aa564b282c666a49d

Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
Reviewed-on: http://gerrit.cloudera.org:8080/18920
Tested-by: Kudu Jenkins
Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
3 files changed, 8 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Yingchun Lai: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
Gerrit-Change-Number: 18920
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [test] instantiate TsRecoveryITestDeathTest

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

Change subject: [test] instantiate TsRecoveryITestDeathTest
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
Gerrit-Change-Number: 18920
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Aug 2022 07:44:53 +0000
Gerrit-HasComments: No