Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18496 )
Change subject: [tools] update schema if needed when rebuild master ...................................................................... Patch Set 5: Code-Review+1 (4 comments) Thank you for working on this! Looks good overall! Just a few nits in the commit message and in the test scenario. http://gerrit.cloudera.org:8080/#/c/18496/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18496/5//COMMIT_MSG@7 PS5, Line 7: rebuild nit: rebuilding http://gerrit.cloudera.org:8080/#/c/18496/5//COMMIT_MSG@9 PS5, Line 9: When use 'kudu master unsafe_rebuild' tool to rebuild : master, if the cluster is not health, for example, : some tservers holds outdated schemas (with lower schema : version), we may build tables with outdated schemas. nit: how about Before this patch, if tservers had outdated schemas with lower version for some tables (e.g. when the cluster wasn't healthy), the 'kudu master unsafe_rebuild' tool might rebuild the system catalog with outdated schemas for the unhealthy tables. http://gerrit.cloudera.org:8080/#/c/18496/5//COMMIT_MSG@14 PS5, Line 14: This patch optmizes the rebuild logic, will use newer : schema (with higher version) to replace schema with : lower version. nit: how about This patch optimizes the rebuild logic, so the tool now picks up schema of the highest version for an unhealthy table when rebuilding the system catalog. http://gerrit.cloudera.org:8080/#/c/18496/5/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/18496/5/src/kudu/tools/kudu-admin-test.cc@3244 PS5, Line 3244: nit: does it make sense to check that the client sees the right number of columns in the KuduSchema object before writing/reading into the table? That something about calling KuduClient::GetTableSchema() and then KuduSchema::num_columns(), correspondingly. -- To view, visit http://gerrit.cloudera.org:8080/18496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec99d57115228b521ba645b8e19c7057a4bb5d3d Gerrit-Change-Number: 18496 Gerrit-PatchSet: 5 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Mon, 16 May 2022 18:07:12 +0000 Gerrit-HasComments: Yes
