Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23844 )
Change subject: [tools] Add --preserve_table_ids flag to unsafe_rebuild ...................................................................... Patch Set 6: Code-Review+1 (3 comments) Thanks a lot for addressing the deficiency in the 'kudu master unsafe_rebuild' CLI tool! Overall this patch looks good to me, just a few nits. http://gerrit.cloudera.org:8080/#/c/23844/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23844/6//COMMIT_MSG@1 PS6, Line 1: Parent: 242b465f ([codegen] relax memory ordering for metric updates) > Maybe this is just a hypothetical scenario, consider a security incident > happened followed by a 'master unsafe_rebuild' with preserve flag i.e. all > the old table ids remain intact and any subsequent scan from the previous > source with old token succeeds when it should have been rejected. The solution for the described scenario is trivial -- simply restart all the tablet servers. Since the rebuilt system catalog has a newly generated CA key, all the authz tokens signed by prior CA key become invalid. The public part of token signing keys are ephemeral at tablet servers, and they are re-fetched from the system catalog when tablet servers start. FWIW, if there is a scenario when it's necessary to reject a request issued by a bearer of previously valid authz token, that's not about rebuilding the system catalog. Invalidation of authz tokens is a completely orthogonal issue -- it's not related to rebuilding system catalog, at all. http://gerrit.cloudera.org:8080/#/c/23844/6/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/23844/6/src/kudu/tools/kudu-admin-test.cc@4093 PS6, Line 4093: LOG(INFO) << "Original table_id before rebuild: " << original_table_id; nit for here and below: since this log line doesn't help with the automated verification, consider dropping it after debugging since tests are quite chatty already http://gerrit.cloudera.org:8080/#/c/23844/6/src/kudu/tools/master_rebuilder.cc File src/kudu/tools/master_rebuilder.cc: http://gerrit.cloudera.org:8080/#/c/23844/6/src/kudu/tools/master_rebuilder.cc@61 PS6, Line 61: false Simply from common sense and from the description of this new flag, I'd rather switch this to 'true' for the default value. I can hardly imagine a use case when it would be necessary to avoid keeping same UUIDs for the tablets/tables upon rebuilding the system catalog. IMO, the fact that rebuilding the system catalog behaved differently before this patch is rather a bug, not a feature. -- To view, visit http://gerrit.cloudera.org:8080/23844 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ae4353564922312d646f0323271d804e32e3b0d Gerrit-Change-Number: 23844 Gerrit-PatchSet: 6 Gerrit-Owner: Yan-Daojiang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yan-Daojiang <[email protected]> Gerrit-Comment-Date: Fri, 13 Mar 2026 02:52:04 +0000 Gerrit-HasComments: Yes
