Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17917 )
Change subject: KUDU-3326 [delete]: Add soft delete table supports ...................................................................... Patch Set 66: Code-Review-1 (1 comment) Thank you very much for working on this useful feature! I apologize for long response times. With everything looking good enough, I still think it's better to separate this initial work and HMS-related part. I added more details on the reasoning in the corresponding comment. To prevent accidental push of this patch before that's discussed and we meet consensus on that topic, I added -1 since Yingchun Lai gave +2 and it would be possible to submit this patch without corresponding updates. http://gerrit.cloudera.org:8080/#/c/17917/59/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/17917/59/src/kudu/master/catalog_manager.cc@2408 PS59, Line 2408: rpc::RpcCo > In my opinion, this is not blocked. When HMS integration is enabled, it's used as a central catalog to contain information about table's structure and availability. If a table is soft-deleted in Kudu but HMS doesn't know that, other applications using the HMS would try to work with the table as it would be readily available. And such a behavior does look consistent to me since an attempt to read/write into such the table would result in an error. What I propose here is to cleanly separate two parts of the work -- the initial support for the soft delete feature and follow-up work to make the soft-delete feature consistent with the HMS integration. As of now, those don't work together in a consistent manner. Even if thinking that soft-deleted table could be "equal to normal status in HMS", I don't see any tests in this patch that would cover a single scenario involving soft-deleted tables when the HMS integration is enabled. If I missed it, please kindly point me at those. -- To view, visit http://gerrit.cloudera.org:8080/17917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d1dddfbca55a5c4bcac4028157325ad618ea665 Gerrit-Change-Number: 17917 Gerrit-PatchSet: 66 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 19 Jul 2022 19:27:29 +0000 Gerrit-HasComments: Yes
