Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18864 )
Change subject: KUDU-3326 [master] add flag FLAGS_enable_soft_delete_on_master to enable soft-delete on master ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/18864/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18864/7//COMMIT_MSG@9 PS7, Line 9: This patch helps to change the default behavior of the DeleteTable RPC at the master side. : If the flag is enabled, the delete request with 'reserve_seconds' set to 0 would make no sense, : the table will do soft-delete instead of purge immediately. Could we make it a bit simpler? I guess the idea is to have a flag to change the cluster-wide behavior of the DeleteTable() RPC. That's to make all DeleteTable requests to actually turn into SoftDeletTable() with the specified grace period at the server side. The use case: there is a bunch of already deployed Kudu applications that call DeleteTable since they are not aware of SoftDeleteTable and it's not possible to modify those applications, but it's necessary to treat each delete table request with some grace period, so it's possible to recall dropped tables for a limited time after DeleteTable request arrives. And it's necessary to do so cluster-wide. As for the explicit calls of the SoftDeleteTable() RPC, I think it should always be treated as-is with the exact grace/reserve period for the dropped tables. If such RPC calls arrives, that means the application is aware of SoftDeleteTable() RPC, so it wants to have the table soft-deleted with the exact specified grace/reserve time. With that, I think it's enough to introduce just a single flag --deleted_table_reserve_seconds, with the default value of 0. That's the flag to control the cluster-wide behavior of the DeleteTable() RPC: if the flag is set to 0, then DeleteTable() works the regular/legacy way, i.e. dropping the table and purging its data immediately. If it's set to anything greater than 0, then all DeleteTable() RPCs are turned into SoftDeleteTable(..., FLAGS_deleted_table_reserve_seconds). I don't think we need to introduce a public kill-switch flag like --enable_soft_delete_on_master unless we want a way to disable soft-delete table support in newer servers. We might add that just for testing purposes, though. What we actually need to do is to introduce a new feature (TABLE_SOFT_DELETION) for masters, and the clients should required that feature when sending DeleteTable() RPCs with non-zero 'reserve_seconds' field. That way the client will know that an old server not just ignore the 'reserve_seconds' field it doesn't recognize, but instead will get a proper error response to realize that the server doesn't support the soft-delete functionality for tables. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/18864 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7cf3021fa0e9a425d5f8091ea3336c55a3c14bbc Gerrit-Change-Number: 18864 Gerrit-PatchSet: 7 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 01 Sep 2022 19:17:39 +0000 Gerrit-HasComments: Yes
