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 56:

(3 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");
> Yeah, but is there any place where this flag is ever overridden?  If not, w
Oh, I see what you mean. I'll implement it another way.


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)
> Right: you can post a separate changelist with just this small refactor, an
OK?I'll take this part out right away.


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:   /// @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.
> OK, thank you for the clarification.
In the initial implementation, I expect that deleting a table with the same 
name can be reentrant by adding a timestamp to the table name.
After discussing with Andrew, we simplified these functions and focused on the 
stability after soft deletion. The table name no longer has a timestamp, and it 
is not allowed to soft delete the reentry of the table with the same name at 
the same time.
So, it is not allowed to perform create/soft-delete cycle multiple time for a 
table with the same name



--
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: 56
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: Mon, 25 Apr 2022 02:16:17 +0000
Gerrit-HasComments: Yes

Reply via email to