You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/10 05:38:08 UTC

[GitHub] [doris] platoneko opened a new pull request, #10731: [Enhancement] Garbage collection of unused data on remote storage backend

platoneko opened a new pull request, #10731:
URL: https://github.com/apache/doris/pull/10731

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem Summary:
   
   This PR will record remote unused rowset info in rocksdb, and remove remote data in  background gc thread.
   There are still some scenes that may generate garbage:
   1. Kill BE while BE is uploading data to remote.
   2. Although we have used `self_ owned_ remote_ rowsets` to record  which remote rowsets can be safely deleted, BE replica scheduling still generated some garbage data in our fuzzy test. Fortunately, garbage data accounts for a small proportion(~90GB data, only ~80MB garbage in our fuzzy test).
   A simple solution to scenario 2: delete all remote data with tablet prefix when drop table/partition. Although remote garbage data will still exist during tables' lifetime, we can ensure that there is no garbage data after dropping table/partition.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   4. Has unit tests been added: (Yes/No/No Need)
   5. Has document been added or modified: (Yes/No/No Need)
   6. Does it need to update dependencies: (Yes/No)
   7. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] platoneko commented on a diff in pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
platoneko commented on code in PR #10731:
URL: https://github.com/apache/doris/pull/10731#discussion_r931123085


##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -438,14 +438,18 @@ void TaskWorkerPool::_drop_tablet_worker_thread_callback() {
                 error_msgs.push_back("drop table failed!");
                 status_code = TStatusCode::RUNTIME_ERROR;
             }
-            // if tablet is dropped by fe, then the related txn should also be removed
-            StorageEngine::instance()->txn_manager()->force_rollback_tablet_related_txns(
-                    dropped_tablet->data_dir()->get_meta(), drop_tablet_req.tablet_id,
-                    drop_tablet_req.schema_hash, dropped_tablet->tablet_uid());
-            // We remove remote rowset directly.
-            // TODO(cyx): do remove in background
-            if (drop_tablet_req.is_drop_table_or_partition) {
-                dropped_tablet->remove_all_remote_rowsets();
+            if (!drop_status.is_aborted()) {
+                // if tablet is dropped by fe, then the related txn should also be removed
+                StorageEngine::instance()->txn_manager()->force_rollback_tablet_related_txns(
+                        dropped_tablet->data_dir()->get_meta(), drop_tablet_req.tablet_id,
+                        drop_tablet_req.schema_hash, dropped_tablet->tablet_uid());
+                if (drop_tablet_req.is_drop_table_or_partition) {
+                    // We remove remote rowset directly.

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] pengxiangyu commented on a diff in pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on code in PR #10731:
URL: https://github.com/apache/doris/pull/10731#discussion_r922963765


##########
be/src/olap/data_dir.cpp:
##########
@@ -804,4 +805,41 @@ Status DataDir::move_to_trash(const std::string& tablet_path) {
     return Status::OK();
 }
 
+void DataDir::perform_remote_gc() {

Review Comment:
   DataDir::perform_path_gc_by_tablet() is used to delete unused file. Maybe rowset has remote file.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] gavinchou commented on pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
gavinchou commented on PR #10731:
URL: https://github.com/apache/doris/pull/10731#issuecomment-1198930712

   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] platoneko commented on a diff in pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
platoneko commented on code in PR #10731:
URL: https://github.com/apache/doris/pull/10731#discussion_r932420975


##########
be/src/olap/base_tablet.h:
##########
@@ -59,10 +60,10 @@ class BaseTablet : public std::enable_shared_from_this<BaseTablet> {
     int16_t shard_id() const;
     bool equal(int64_t tablet_id, int32_t schema_hash);
 
-    const io::ResourceId& cooldown_resource() const { return _tablet_meta->cooldown_resource(); }
+    const std::string& storage_policy() const { return _tablet_meta->storage_policy(); }
 
-    void set_cooldown_resource(io::ResourceId resource) {
-        _tablet_meta->set_cooldown_resource(std::move(resource));
+    void set_storage_policy(std::string policy) {

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] yiguolei merged pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #10731:
URL: https://github.com/apache/doris/pull/10731


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] pengxiangyu commented on a diff in pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on code in PR #10731:
URL: https://github.com/apache/doris/pull/10731#discussion_r922954365


##########
be/src/olap/base_tablet.h:
##########
@@ -59,10 +60,10 @@ class BaseTablet : public std::enable_shared_from_this<BaseTablet> {
     int16_t shard_id() const;
     bool equal(int64_t tablet_id, int32_t schema_hash);
 
-    const io::ResourceId& cooldown_resource() const { return _tablet_meta->cooldown_resource(); }
+    const std::string& storage_policy() const { return _tablet_meta->storage_policy(); }
 
-    void set_cooldown_resource(io::ResourceId resource) {
-        _tablet_meta->set_cooldown_resource(std::move(resource));
+    void set_storage_policy(std::string policy) {

Review Comment:
   const std::string& is better.



##########
be/src/olap/data_dir.cpp:
##########
@@ -804,4 +805,41 @@ Status DataDir::move_to_trash(const std::string& tablet_path) {
     return Status::OK();
 }
 
+void DataDir::perform_remote_gc() {
+    std::vector<std::pair<std::string, std::string>> gc_kvs;
+    auto traverse_remote_rowset_func = [&gc_kvs](const std::string& key,
+                                                 const std::string& value) -> bool {
+        gc_kvs.emplace_back(key, value);
+        return true;
+    };
+    _meta->iterate(META_COLUMN_FAMILY_INDEX, REMOTE_ROWSET_GC_PREFIX, traverse_remote_rowset_func);
+    std::vector<std::string> deleted_keys;
+    for (auto& [key, val] : gc_kvs) {
+        auto rowset_id = key.substr(REMOTE_ROWSET_GC_PREFIX.size());
+        RemoteRowsetGcPB gc_pb;
+        gc_pb.ParseFromString(val);
+        auto fs = io::FileSystemMap::instance()->get(gc_pb.resource_id());
+        if (!fs) {
+            LOG(WARNING) << "Cannot get file system: " << gc_pb.resource_id();
+            continue;
+        }
+        DCHECK(fs->type() != io::FileSystemType::LOCAL);
+        Status st;
+        for (int i = 0; i < gc_pb.num_segments(); ++i) {
+            auto seg_path = BetaRowset::remote_segment_path(gc_pb.tablet_id(), rowset_id, i);

Review Comment:
   delete file directly?It is dangerous, move it to trash is better。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] pengxiangyu commented on a diff in pull request #10731: [Enhancement] Garbage collection of unused data on remote storage backend

Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on code in PR #10731:
URL: https://github.com/apache/doris/pull/10731#discussion_r923059632


##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -438,14 +438,18 @@ void TaskWorkerPool::_drop_tablet_worker_thread_callback() {
                 error_msgs.push_back("drop table failed!");
                 status_code = TStatusCode::RUNTIME_ERROR;
             }
-            // if tablet is dropped by fe, then the related txn should also be removed
-            StorageEngine::instance()->txn_manager()->force_rollback_tablet_related_txns(
-                    dropped_tablet->data_dir()->get_meta(), drop_tablet_req.tablet_id,
-                    drop_tablet_req.schema_hash, dropped_tablet->tablet_uid());
-            // We remove remote rowset directly.
-            // TODO(cyx): do remove in background
-            if (drop_tablet_req.is_drop_table_or_partition) {
-                dropped_tablet->remove_all_remote_rowsets();
+            if (!drop_status.is_aborted()) {
+                // if tablet is dropped by fe, then the related txn should also be removed
+                StorageEngine::instance()->txn_manager()->force_rollback_tablet_related_txns(
+                        dropped_tablet->data_dir()->get_meta(), drop_tablet_req.tablet_id,
+                        drop_tablet_req.schema_hash, dropped_tablet->tablet_uid());
+                if (drop_tablet_req.is_drop_table_or_partition) {
+                    // We remove remote rowset directly.

Review Comment:
   Local file and OlapMeta are deleted in TabletManager::start_trash_sweep(); remote file need to be deleted in start_trash_sweep() too, otherwise, it will cause remote file remains but OlapMeta is removed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org