KeDeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/17917 )
Change subject: KUDU-3326 [delete]: Add soft delete table supports ...................................................................... Patch Set 26: (6 comments) http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h@714 PS5, Line 714: /// Whether to apply the deletion to external catalogs, such as the Hive Metastore, : /// which the Kudu master has been configured to integrate with. : /// @param [in] force_on_trashed_table : /// Whether to force to delete a trashed table. : /// @param [in] reserve_seconds : /// Reserve seconds after being deleted. : /// @return Operation status. > What if there were many tables with the same name created and then deleted? The main reason for designing this interface is in some scenarios?tables may be deleted by mistake. At this time, a method is needed to recover the deleted tables. Now the design of the system should require table name unique, so it should not delete tables with the same name at the same time. If a table is deleted first, then a new table with the same name is created and then deleted, the trash table names obtained from the two operations are different, so the current design can meet the conditions of deleting tables with the same name successively. For the recall table operation, you need to ensure that there is no table with the same name, otherwise it will fail.Of course, we can also add parameters to specify the table name after recalled, but this may deviate from the original design intention of the recall interface. http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/catalog_manager.cc@1505 PS22, Line 1505: > How is this repopulated? I'd expect we need to check the system catalog ent Done http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/catalog_manager.cc@6214 PS22, Line 6214: resp, MasterErrorPB::EVEN_REPLICATION_FACTOR); > What happens if 'trash_table_names_map_' already has a trashed table of thi I think we need to ensure the uniqueness of table name in the system, even if the table is in trash state. http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/master_service.cc@581 PS22, Line 581: > I don't think this is doing the right thing. If we have requested to modify Done http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/master_service.cc@578 PS22, Line 578: atus s = server_->catalog_manager()->AlterTableRpc( : *req, resp, rpc); : CheckRespErrorOrSetUnknown(s, resp); : rpc->RespondSuccess(); : } : : void MasterServiceImpl::IsAlterTableDone(const IsAlterTableDoneRequestPB* req, : IsAlterTableDoneResponsePB* resp, : rpc::RpcContext* rpc) { : CatalogManager::ScopedLeaderSharedLock l(server_->catalog_manager()); : if (!l.CheckIsInitializedAndIsLeaderOrRespond(resp, rpc)) { : return; : } : : auto s = server_->catalog_manager()->IsAlterTableDone( : req, resp, rpc->remote_user().username()); : CheckRespErrorOrSetUnknown(s, resp); : rpc->RespondSuccess(); : } : : void > The failure scenarios here are difficult to reason about. What if there's a Done http://gerrit.cloudera.org:8080/#/c/17917/22/src/kudu/master/master_service.cc@616 PS22, Line 616: sInitializedAnd > Could we move more of the business logic into the CatalogManager, rather th Done -- 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: 26 Gerrit-Owner: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 27 Jan 2022 06:12:42 +0000 Gerrit-HasComments: Yes
