Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20830 )
Change subject: Do not enter ALTERING state just for replication factor changed. ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Thank you for the fix! http://gerrit.cloudera.org:8080/#/c/20830/9/src/kudu/integration-tests/alter_table-test.cc File src/kudu/integration-tests/alter_table-test.cc: http://gerrit.cloudera.org:8080/#/c/20830/9/src/kudu/integration-tests/alter_table-test.cc@2642 PS9, Line 2642: TEST_F(ReplicatedAlterTableTest, CheckTableStateAfterReplicatedAlter) { > This probably needs to add some codes to catalog_manager.cc to enable or di AFAIK, for the majority of tests in the Kudu codebase it's been enough to make sure the test pass with the fix, and just manually checking that it was failing before the fix. Adding variations on fix/no-fix is an overkill. The idea is to make sure the fix works as intended and to catch future regressions. That's already enough. -- To view, visit http://gerrit.cloudera.org:8080/20830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d20da7c0dd5912790aaa46e9fff366b2973d7a4 Gerrit-Change-Number: 20830 Gerrit-PatchSet: 10 Gerrit-Owner: Song Jiacheng <songjiach...@thinkingdata.cn> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Song Jiacheng <songjiach...@thinkingdata.cn> Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Comment-Date: Thu, 04 Jan 2024 17:15:13 +0000 Gerrit-HasComments: Yes