[Feature] Support for cleaning the trash actively by BiteTheDDDDt · Pull Request #6323 · apache/doris

BiteTheDDDDt · 2021-07-26T02:50:32Z

Proposed changes

Support for cleaning the trash actively.
User can use 'CLEAN TRASH' to clean trash.

Types of changes

What types of changes does your code introduce to Doris?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)
Code refactor (Modify the code structure, format the code, etc...)
Optimization. Including functional usability improvements and performance improvements.
Dependency. Such as changes related to third-party components.
Other.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have created an issue on (Fix [Feature] Support for cleaning the trash actively #6322) and described the bug/feature there in detail
Compiling and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
If these changes need document changes, I have updated the document
Any dependent changes have been merged

EmmyMiao87 · 2021-07-26T02:55:18Z

Please enrich your commit msg

fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java

EmmyMiao87 · 2021-07-26T03:00:46Z

docs/zh-CN/sql-reference/sql-statements/Administration/CLEAN TRASH.md

+
+# CLEAN TRASH
+## description
+    该语句用于清理 backend 内的垃圾数据。


Will this statement clean up both trash and snapshot?
Will cleaning up the snapshot involve the snapshot being restored?

Will this statement clean up both trash and snapshot?
Will cleaning up the snapshot involve the snapshot being restored?

Yes, this statement will clean up both trash and snapshot.

This statement will call StorageEngine::start_trash_sweep.
It will only clean up expired data (define at config::snapshot_expire_time_sec/config::trash_file_expire_time_sec).
And this function will be automatically called periodically, so I think its cleanup is harmless.

EmmyMiao87 · 2021-07-26T03:22:28Z

docs/.vuepress/sidebar/en.js

              "SHOW MIGRATIONS",
              "SHOW PLUGINS",
              "SHOW TABLE STATUS",
+              "CLEAN TRASH",


You need to add both guide and sql reference

You need to add both guide and sql reference

c31dd3e
I add description at /administrator-guide/operation/disk-capacity.md .
At the same time, I found that there is no corresponding English version of this document.

nimuyuhan · 2021-07-27T05:38:21Z

fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java

+            TNetworkAddress address = null;
+            boolean ok = false;
+            try {
+                long start = System.currentTimeMillis();


The variable start is not used later.

The variable start is not used later.

fixed

EmmyMiao87

LGTM

github-actions · 2021-07-27T06:39:53Z

PR approved by at least one committer and no changes requested.

github-actions · 2021-07-27T06:39:54Z

PR approved by anyone and no changes requested.

nimuyuhan

LGTM

morningman

How to force sweep trash?

BiteTheDDDDt · 2021-07-28T06:23:44Z

How to force sweep trash?

I do some change at 08df4ce

EmmyMiao87 · 2021-08-03T02:25:01Z

be/src/service/backend_service.cpp


 void BackendService::clean_trash() {
-    StorageEngine::instance()->start_trash_sweep(nullptr); // do not update usage
+    StorageEngine::instance()->start_trash_sweep(nullptr, true); // do not update usage, ignore guard_space


Suggested change

StorageEngine::instance()->start_trash_sweep(nullptr, true); // do not update usage, ignore guard_space

StorageEngine::instance()->start_trash_sweep(nullptr, true); // update usage, ignore guard_space

EmmyMiao87 · 2021-08-03T02:25:34Z

docs/en/administrator-guide/operation/disk-capacity.md

    **This operation will affect [Restore data from BE Recycle Bin](./tablet-restore-tool.md).**

-    If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort)` to actively clean up temporary files. There are two situations as follows: 
+    If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. **all trash files** and expired snapshot files will be cleaned up, **This will affect the operation of restoring data from the trash bin**.


Suggested change

If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. **all trash files** and expired snapshot files will be cleaned up, **This will affect the operation of restoring data from the trash bin**.

If the BE can still be started, you can use `ADMIN CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. **all trash files** and expired snapshot files will be cleaned up, **This will affect the operation of restoring data from the trash bin**.

EmmyMiao87 · 2021-08-03T02:25:45Z

docs/en/administrator-guide/operation/disk-capacity.md

+    If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. **all trash files** and expired snapshot files will be cleaned up, **This will affect the operation of restoring data from the trash bin**.
+
+
+    If you do not manually execute `CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows: 


Suggested change

If you do not manually execute `CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows:

If you do not manually execute `ADMIN CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows:

EmmyMiao87 · 2021-08-03T02:26:16Z

docs/zh-CN/administrator-guide/operation/disk-capacity.md

-    如果BE还能够启动，则可以使用`CLEAN TRASH ON(BackendHost:BackendHeartBeatPort)`来主动清理临时文件，这里分为如下两种情况：
-    * 如果磁盘占用未达到 **危险水位(Flood Stage)** 的90%，则会清理过期trash文件和过期snapshot文件，此时会保留一些近期文件而不影响恢复数据。
-    * 如果磁盘占用已达到 **危险水位(Flood Stage)** 的90%，则会清理 **所有** trash文件和过期snapshot文件， **此时也会影响从回收站恢复数据的操作** 。
+    如果BE还能够启动，则可以使用`CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);`来主动清理临时文件，会清理 **所有** trash文件和过期snapshot文件，**这将影响从回收站恢复数据的操作** 。


same as above

EmmyMiao87 · 2021-08-03T02:26:26Z

docs/zh-CN/administrator-guide/operation/disk-capacity.md

+    如果BE还能够启动，则可以使用`CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);`来主动清理临时文件，会清理 **所有** trash文件和过期snapshot文件，**这将影响从回收站恢复数据的操作** 。

-    如果不手动执行`CLEAN TRASH`，系统仍将会在几分钟至几十分钟内自动执行清理。
+    如果不手动执行`CLEAN TRASH`，系统仍将会在几分钟至几十分钟内自动执行清理，这里分为两种情况：


same as above

same as above

I fix these problems at 3c899f6

morningman · 2021-08-04T16:39:40Z

fe/fe-core/src/main/java/org/apache/doris/analysis/AdminCleanTrashStmt.java

+
+    @Override
+    public RedirectStatus getRedirectStatus() {
+        return RedirectStatus.FORWARD_WITH_SYNC;


Do we need to forward this stmt to master?

Do we need to forward this stmt to master?

This does not seem to modify the metadata, so I change it to NO_FORWARD at 9765d59

morningman · 2021-08-04T16:41:20Z

be/src/service/backend_service.cpp

 }

+void BackendService::clean_trash() {
+    StorageEngine::instance()->start_trash_sweep(nullptr, true);


It may takes a very long time to clean the trash. So I suggest to use a async call.

It may takes a very long time to clean the trash. So I suggest to use a async call.

I think this is already async, because of I use oneway to define the function at thrift file.
gensrc/thrift/BackendService.thrift
oneway void clean_trash();

morningman · 2021-08-07T14:39:03Z

gensrc/thrift/BackendService.thrift


    TStreamLoadRecordResult get_stream_load_record(1: i64 last_stream_record_time);

+    oneway void clean_trash();


Is it safe to be called multi times using oneway?
And is the method start_trash_sweep() thread safe?

author BiteTheDDDDt <952130278@qq.com> 1626945340 +0800 committer BiteTheDDDDt <952130278@qq.com> 1628491167 +0800 support for clean trash used on backends && add document of clean trash fix wrong format on CleanTrashStmt toSql Update fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java fix format Co-authored-by: EmmyMiao87 <522274284@qq.com> add description about 'clean trash' at disk-capacity.md Translate document /administrator-guide/operation/disk-capacity.md to english add more document description about clean trash && remove unused variable fix blank in markdown 1. Ignore guard space when clean trash. 2. Change query format from 'clean trash' to 'admin clean trash'. 3. Update document about clean trash. 1. improve comments. 2. remove useless function (AdminCleanTrashStmt.toSql()). 3. fix document. change 'FORWARD_WITH_SYNC' to 'NO_FORWARD' at AdminCleanTrashStmt support for clean trash used on backends && add document of clean trash fix wrong format on CleanTrashStmt toSql 1. Ignore guard space when clean trash. 2. Change query format from 'clean trash' to 'admin clean trash'. 3. Update document about clean trash. 1. improve comments. 2. remove useless function (AdminCleanTrashStmt.toSql()). 3. fix document. change 'FORWARD_WITH_SYNC' to 'NO_FORWARD' at AdminCleanTrashStmt

morningman · 2021-08-10T11:03:57Z

be/src/olap/storage_engine.cpp

    // clean unused rowset metas in OlapMeta
    _clean_unused_rowset_metas();

+    _trash_sweep_lock.unlock();


The method may be returned before you unlock this lock.
you can use src//util/mutex.h to unlock automatically when deconstructing.

The method may be returned before you unlock this lock.
you can use src//util/mutex.h to unlock automatically when deconstructing.

I use unique_lock to fix it.

This reverts commit 9174151.

…doris into dev_clear

morningman

LGTM

github-actions · 2021-08-11T13:57:52Z

PR approved by at least one committer and no changes requested.

EmmyMiao87 reviewed Jul 26, 2021

View reviewed changes

EmmyMiao87 added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 26, 2021

EmmyMiao87 self-assigned this Jul 26, 2021

EmmyMiao87 reviewed Jul 26, 2021

View reviewed changes

nimuyuhan reviewed Jul 27, 2021

View reviewed changes

EmmyMiao87 previously approved these changes Jul 27, 2021

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 27, 2021

github-actions bot added the reviewed label Jul 27, 2021

nimuyuhan previously approved these changes Jul 27, 2021

View reviewed changes

morningman reviewed Jul 27, 2021

View reviewed changes

BiteTheDDDDt dismissed stale reviews from nimuyuhan and EmmyMiao87 via 08df4ce July 28, 2021 06:17

github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jul 28, 2021

EmmyMiao87 reviewed Aug 3, 2021

View reviewed changes

morningman reviewed Aug 4, 2021

View reviewed changes

morningman reviewed Aug 7, 2021

View reviewed changes

BiteTheDDDDt added 2 commits August 9, 2021 14:43

add lock at start_trash_sweep for make it can re-entrant

ce1ea45

BiteTheDDDDt force-pushed the dev_clear branch from a28a05a to ce1ea45 Compare August 9, 2021 06:45

BiteTheDDDDt added 2 commits August 9, 2021 14:59

Fix the bug generated when resolving conflicts.

d920b0a

Change lock_guard to return directly when locked.

126f821

morningman reviewed Aug 10, 2021

View reviewed changes

BiteTheDDDDt added 4 commits August 10, 2021 20:08

use unique_lock

9174151

Merge branch 'master' into dev_clear

50907b7

Revert "use unique_lock"

7ea044c

This reverts commit 9174151.

Merge branch 'dev_clear' of http://github.com/BiteTheDDDDt/incubator-…

7508f1c

…doris into dev_clear

use unique_lock to avoid deadlock

2d0cfa4

morningman approved these changes Aug 11, 2021

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 11, 2021

yangzhg merged commit 8a267f1 into apache:master Aug 12, 2021

morningman mentioned this pull request Oct 10, 2021

Release note 0.15.0 #6806

Closed

BiteTheDDDDt deleted the dev_clear branch January 20, 2025 06:45

	StorageEngine::instance()->start_trash_sweep(nullptr, true); // do not update usage, ignore guard_space
	StorageEngine::instance()->start_trash_sweep(nullptr, true); // update usage, ignore guard_space

	If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. all trash files and expired snapshot files will be cleaned up, This will affect the operation of restoring data from the trash bin.
	If the BE can still be started, you can use `ADMIN CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. all trash files and expired snapshot files will be cleaned up, This will affect the operation of restoring data from the trash bin.

		If the BE can still be started, you can use `CLEAN TRASH ON(BackendHost:BackendHeartBeatPort);` to actively clean up temporary files. all trash files and expired snapshot files will be cleaned up, This will affect the operation of restoring data from the trash bin.


		If you do not manually execute `CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows:

	If you do not manually execute `CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows:
	If you do not manually execute `ADMIN CLEAN TRASH`, the system will still automatically execute the cleanup within a few minutes to tens of minutes.There are two situations as follows:


		TStreamLoadRecordResult get_stream_load_record(1: i64 last_stream_record_time);

		oneway void clean_trash();

Conversation

BiteTheDDDDt commented Jul 26, 2021

Proposed changes

Types of changes

Checklist

Uh oh!

EmmyMiao87 commented Jul 26, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmmyMiao87 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 27, 2021

Uh oh!

github-actions bot commented Jul 27, 2021

Uh oh!

nimuyuhan left a comment

Choose a reason for hiding this comment

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

BiteTheDDDDt commented Jul 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BiteTheDDDDt Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

BiteTheDDDDt Aug 5, 2021 •

edited

Loading