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 54: (4 comments) http://gerrit.cloudera.org:8080/#/c/17917/54/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/17917/54/src/kudu/client/client-internal.cc@73 PS54, Line 73: DEFINE_uint32(trash_table_reserved_seconds, 60 * 60 * 24 * 7, : "Default reserved time for deleted table." : "A value of 0 disables the reserved trash table and delete table directly"); > Yes, used to set default parameters in function DeleteTable. Yeah, but is there any place where this flag is ever overridden? If not, why to keep it at all? I guess my question is why to have this as a flag at all. The issue with gflags in a client library is that there isn't a good way to use it in an application that uses the library. http://gerrit.cloudera.org:8080/#/c/17917/54/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/17917/54/src/kudu/client/client-test.cc@715 PS54, Line 715: Status CreateTable(const string& table_name, : int num_replicas, : vector<unique_ptr<KuduPartialRow>> split_rows, : vector<pair<unique_ptr<KuduPartialRow>, : unique_ptr<KuduPartialRow>>> range_bounds, : shared_ptr<KuduTable>* table) > If it's a function code, I think it can be disassembled. However, this func Right: you can post a separate changelist with just this small refactor, and rebase this patch on top of that. Thanks! 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@690 PS5, Line 690: /// @return Operation status. : Status DeleteTable(const std::string& table_name) > Sorry, I forgot to reply. This has been modified in the latest code.Because Ah, no problem -- indeed, the issue has been resolved in the recent patchsets. Sure, I'll take a closer look, hopefully next week. Thank you very much for working on this useful new feature! http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h@714 PS5, Line 714: /// @param [in] modify_external_catalogs : /// 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. > Allowing the creation of the same table name as the trash table is outdated OK, thank you for the clarification. Just to double-check: with the updated design, it is now possible to perform create/soft-delete cycle multiple time for a table with the same name (of course, their table identifiers are different), right? -- 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: 54 Gerrit-Owner: KeDeng <[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: Fri, 22 Apr 2022 14:33:48 +0000 Gerrit-HasComments: Yes
