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/12/30 09:24:52 UTC

[GitHub] [doris] github-actions[bot] commented on a diff in pull request #14390: [Feature](remote)Remote Storage support single file when Cooldown.(FE)

github-actions[bot] commented on code in PR #14390:
URL: https://github.com/apache/doris/pull/14390#discussion_r1059307721


##########
be/src/agent/agent_server.cpp:
##########
@@ -183,6 +184,15 @@ void AgentServer::submit_tasks(TAgentResult& agent_result,
                         signature);
             }
             break;
+        case TTaskType::PUSH_COOLDOWN_CONF:
+            if (task.__isset.push_cooldown_conf || task.__isset.push_cooldown_conf) {

Review Comment:
   warning: both sides of operator are equivalent [misc-redundant-expression]
   ```cpp
               if (task.__isset.push_cooldown_conf || task.__isset.push_cooldown_conf) {
                                                   ^
   ```
   



##########
be/src/runtime/exec_env.h:
##########
@@ -182,6 +182,8 @@ class ExecEnv {
     HeartbeatFlags* heartbeat_flags() { return _heartbeat_flags; }
     doris::vectorized::ScannerScheduler* scanner_scheduler() { return _scanner_scheduler; }
 
+    int64_t be_start_time_sec() { return _be_start_time_sec; }

Review Comment:
   warning: method 'be_start_time_sec' can be made const [readability-make-member-function-const]
   
   ```suggestion
       int64_t be_start_time_sec() const { return _be_start_time_sec; }
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1762,18 +1807,153 @@ Status Tablet::cooldown() {
         has_shutdown = tablet_state() == TABLET_SHUTDOWN;
         if (!has_shutdown) {
             modify_rowsets(to_add, to_delete);
-            _self_owned_remote_rowsets.insert(to_add.front());
+            if (!config::cooldown_single_remote_file) {
+                _self_owned_remote_rowsets.insert(to_add.front());
+            }
             save_meta();
         }
     }
-    if (has_shutdown) {
+    if (has_shutdown && !config::cooldown_single_remote_file) {
         record_unused_remote_rowset(new_rowset_id, dest_fs->resource_id(),
                                     to_add.front()->num_segments());
         return Status::Aborted("tablet {} has shutdown", tablet_id());
     }
     return Status::OK();
 }
 
+// if 2 files exist, get tablet_meta from remote_meta_path
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, TabletMetaPB* tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id());
+    bool exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    }
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id());
+    exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    LOG(INFO) << "No tablet meta file founded, init needed. tablet_id: " << tablet_id();
+    return Status::OK();
+}
+
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                        TabletMetaPB* tablet_meta_pb) {
+    io::FileReaderSPtr tablet_meta_reader;
+    RETURN_IF_ERROR(fs->open_file(meta_path, &tablet_meta_reader));
+    DCHECK(tablet_meta_reader != nullptr);
+    auto file_size = tablet_meta_reader->size();
+    size_t bytes_read;
+    uint8_t* buf = new uint8_t[file_size];
+    Slice slice(buf, file_size);
+    IOContext io_ctx;
+    Status st = tablet_meta_reader->read_at(0, slice, io_ctx, &bytes_read);
+    if (!st.ok()) {
+        tablet_meta_reader->close();
+        return st;
+    }
+    tablet_meta_reader->close();
+    if (!tablet_meta_pb->ParseFromString(slice.to_string())) {
+        LOG(WARNING) << "parse tablet meta failed";
+        return Status::OLAPInternalError(OLAP_ERR_INIT_FAILED);
+    }
+    return Status::OK();
+}
+
+// Remote tablet meta file need to write 2 files, and then delete remote_meta_bak_path file.

Review Comment:
   warning: no member named 'OLAPInternalError' in 'doris::Status' [clang-diagnostic-error]
   ```cpp
    tablet meta failed";
                                                ^
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1762,18 +1807,153 @@
         has_shutdown = tablet_state() == TABLET_SHUTDOWN;
         if (!has_shutdown) {
             modify_rowsets(to_add, to_delete);
-            _self_owned_remote_rowsets.insert(to_add.front());
+            if (!config::cooldown_single_remote_file) {
+                _self_owned_remote_rowsets.insert(to_add.front());
+            }
             save_meta();
         }
     }
-    if (has_shutdown) {
+    if (has_shutdown && !config::cooldown_single_remote_file) {
         record_unused_remote_rowset(new_rowset_id, dest_fs->resource_id(),
                                     to_add.front()->num_segments());
         return Status::Aborted("tablet {} has shutdown", tablet_id());
     }
     return Status::OK();
 }
 
+// if 2 files exist, get tablet_meta from remote_meta_path
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, TabletMetaPB* tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id());
+    bool exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    }
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id());
+    exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    LOG(INFO) << "No tablet meta file founded, init needed. tablet_id: " << tablet_id();
+    return Status::OK();
+}
+
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                        TabletMetaPB* tablet_meta_pb) {
+    io::FileReaderSPtr tablet_meta_reader;
+    RETURN_IF_ERROR(fs->open_file(meta_path, &tablet_meta_reader));
+    DCHECK(tablet_meta_reader != nullptr);
+    auto file_size = tablet_meta_reader->size();
+    size_t bytes_read;
+    uint8_t* buf = new uint8_t[file_size];
+    Slice slice(buf, file_size);
+    IOContext io_ctx;
+    Status st = tablet_meta_reader->read_at(0, slice, io_ctx, &bytes_read);
+    if (!st.ok()) {
+        tablet_meta_reader->close();
+        return st;
+    }
+    tablet_meta_reader->close();
+    if (!tablet_meta_pb->ParseFromString(slice.to_string())) {
+        LOG(WARNING) << "parse tablet meta failed";
+        return Status::OLAPInternalError(OLAP_ERR_INIT_FAILED);
+    }
+    return Status::OK();
+}
+
+// Remote tablet meta file need to write 2 files, and then delete remote_meta_bak_path file.
+// When an error occurs, if 1 file is deleted just now, the other file will still exist.
+Status Tablet::_write_remote_tablet_meta(FileSystemSPtr fs, int64_t tablet_id,
+                                         const TabletMetaPB& tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id);
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id);
+    bool exist = false;
+    bool bak_exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &bak_exist));
+    bool bak_file_need_upload = !bak_exist;
+    if (exist && bak_exist) {
+        // It means bak file is not deleted, and meta file exist, bak file is not needed.
+        RETURN_IF_ERROR(fs->delete_file(remote_meta_bak_path));
+        bak_file_need_upload = true;
+    }
+    if (bak_file_need_upload) {
+        RETURN_IF_ERROR(_write_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    if (exist) {
+        RETURN_IF_ERROR(fs->delete_file(remote_meta_path));
+    }
+    RETURN_IF_ERROR(_write_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    RETURN_IF_ERROR(fs->delete_file(remote_meta_bak_path));
+    return Status::OK();
+}
+
+Status Tablet::_write_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                         const TabletMetaPB& tablet_meta_pb) {
+    io::FileWriterPtr tablet_meta_writer;
+    RETURN_IF_ERROR(fs->create_file(meta_path, &tablet_meta_writer));
+    DCHECK(tablet_meta_writer != nullptr);
+    string value;
+    tablet_meta_pb.SerializeToString(&value);
+    DCHECK(tablet_meta_writer != nullptr);
+    uint8_t* buf = new uint8_t[value.size()];
+    memcpy(buf, value.c_str(), value.size());
+    Slice slice(buf, value.size());
+    Status st = tablet_meta_writer->appendv(&slice, 1);
+    if (!st.ok()) {
+        tablet_meta_writer->close();
+        return st;
+    }
+    tablet_meta_writer->close();
+    return Status::OK();
+}
+
+Status Tablet::_cooldown_use_remote_data() {
+    auto dest_fs = io::FileSystemMap::instance()->get(storage_policy());
+    if (!dest_fs) {
+        return Status::OLAPInternalError(OLAP_ERR_NOT_INITED);
+    }
+    DCHECK(dest_fs->type() == io::FileSystemType::S3);
+    TabletMetaPB remote_tablet_meta_pb;

Review Comment:
   warning: no member named 'OLAPInternalError' in 'doris::Status' [clang-diagnostic-error]
   ```cpp
       return Status::OLAPInternalError(OLAP_ERR_NOT_INITED);
                      ^
   ```
   



##########
be/src/olap/tablet_meta.cpp:
##########
@@ -866,6 +872,7 @@ bool operator==(const TabletMeta& a, const TabletMeta& b) {
     if (a._in_restore_mode != b._in_restore_mode) return false;
     if (a._preferred_rowset_type != b._preferred_rowset_type) return false;
     if (a._storage_policy != b._storage_policy) return false;
+    if (a._cooldown_type != b._cooldown_type) return false;

Review Comment:
   warning: statement should be inside braces [readability-braces-around-statements]
   
   ```suggestion
       if (a._cooldown_type != b._cooldown_type) { return false;
   }
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1762,18 +1807,153 @@
         has_shutdown = tablet_state() == TABLET_SHUTDOWN;
         if (!has_shutdown) {
             modify_rowsets(to_add, to_delete);
-            _self_owned_remote_rowsets.insert(to_add.front());
+            if (!config::cooldown_single_remote_file) {
+                _self_owned_remote_rowsets.insert(to_add.front());
+            }
             save_meta();
         }
     }
-    if (has_shutdown) {
+    if (has_shutdown && !config::cooldown_single_remote_file) {
         record_unused_remote_rowset(new_rowset_id, dest_fs->resource_id(),
                                     to_add.front()->num_segments());
         return Status::Aborted("tablet {} has shutdown", tablet_id());
     }
     return Status::OK();
 }
 
+// if 2 files exist, get tablet_meta from remote_meta_path
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, TabletMetaPB* tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id());
+    bool exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    }
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id());
+    exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    LOG(INFO) << "No tablet meta file founded, init needed. tablet_id: " << tablet_id();
+    return Status::OK();
+}
+
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                        TabletMetaPB* tablet_meta_pb) {
+    io::FileReaderSPtr tablet_meta_reader;
+    RETURN_IF_ERROR(fs->open_file(meta_path, &tablet_meta_reader));
+    DCHECK(tablet_meta_reader != nullptr);
+    auto file_size = tablet_meta_reader->size();
+    size_t bytes_read;
+    uint8_t* buf = new uint8_t[file_size];
+    Slice slice(buf, file_size);
+    IOContext io_ctx;
+    Status st = tablet_meta_reader->read_at(0, slice, io_ctx, &bytes_read);
+    if (!st.ok()) {
+        tablet_meta_reader->close();
+        return st;
+    }
+    tablet_meta_reader->close();
+    if (!tablet_meta_pb->ParseFromString(slice.to_string())) {
+        LOG(WARNING) << "parse tablet meta failed";
+        return Status::OLAPInternalError(OLAP_ERR_INIT_FAILED);
+    }
+    return Status::OK();
+}
+
+// Remote tablet meta file need to write 2 files, and then delete remote_meta_bak_path file.
+// When an error occurs, if 1 file is deleted just now, the other file will still exist.
+Status Tablet::_write_remote_tablet_meta(FileSystemSPtr fs, int64_t tablet_id,
+                                         const TabletMetaPB& tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id);
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id);
+    bool exist = false;
+    bool bak_exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &bak_exist));
+    bool bak_file_need_upload = !bak_exist;
+    if (exist && bak_exist) {
+        // It means bak file is not deleted, and meta file exist, bak file is not needed.
+        RETURN_IF_ERROR(fs->delete_file(remote_meta_bak_path));
+        bak_file_need_upload = true;
+    }
+    if (bak_file_need_upload) {
+        RETURN_IF_ERROR(_write_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    if (exist) {
+        RETURN_IF_ERROR(fs->delete_file(remote_meta_path));
+    }
+    RETURN_IF_ERROR(_write_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    RETURN_IF_ERROR(fs->delete_file(remote_meta_bak_path));
+    return Status::OK();
+}
+
+Status Tablet::_write_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                         const TabletMetaPB& tablet_meta_pb) {
+    io::FileWriterPtr tablet_meta_writer;
+    RETURN_IF_ERROR(fs->create_file(meta_path, &tablet_meta_writer));
+    DCHECK(tablet_meta_writer != nullptr);
+    string value;
+    tablet_meta_pb.SerializeToString(&value);
+    DCHECK(tablet_meta_writer != nullptr);
+    uint8_t* buf = new uint8_t[value.size()];
+    memcpy(buf, value.c_str(), value.size());
+    Slice slice(buf, value.size());
+    Status st = tablet_meta_writer->appendv(&slice, 1);
+    if (!st.ok()) {
+        tablet_meta_writer->close();
+        return st;
+    }
+    tablet_meta_writer->close();
+    return Status::OK();
+}
+
+Status Tablet::_cooldown_use_remote_data() {
+    auto dest_fs = io::FileSystemMap::instance()->get(storage_policy());
+    if (!dest_fs) {
+        return Status::OLAPInternalError(OLAP_ERR_NOT_INITED);
+    }
+    DCHECK(dest_fs->type() == io::FileSystemType::S3);
+    TabletMetaPB remote_tablet_meta_pb;

Review Comment:
   warning: use of undeclared identifier 'OLAP_ERR_NOT_INITED' [clang-diagnostic-error]
   ```cpp
       return Status::OLAPInternalError(OLAP_ERR_NOT_INITED);
                                        ^
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1762,18 +1807,153 @@
         has_shutdown = tablet_state() == TABLET_SHUTDOWN;
         if (!has_shutdown) {
             modify_rowsets(to_add, to_delete);
-            _self_owned_remote_rowsets.insert(to_add.front());
+            if (!config::cooldown_single_remote_file) {
+                _self_owned_remote_rowsets.insert(to_add.front());
+            }
             save_meta();
         }
     }
-    if (has_shutdown) {
+    if (has_shutdown && !config::cooldown_single_remote_file) {
         record_unused_remote_rowset(new_rowset_id, dest_fs->resource_id(),
                                     to_add.front()->num_segments());
         return Status::Aborted("tablet {} has shutdown", tablet_id());
     }
     return Status::OK();
 }
 
+// if 2 files exist, get tablet_meta from remote_meta_path
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, TabletMetaPB* tablet_meta_pb) {
+    std::string remote_meta_path = BetaRowset::remote_tablet_meta_path(tablet_id());
+    bool exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_path, tablet_meta_pb));
+    }
+    std::string remote_meta_bak_path = BetaRowset::remote_tablet_meta_bak_path(tablet_id());
+    exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_bak_path, &exist));
+    if (exist) {
+        RETURN_IF_ERROR(_read_remote_tablet_meta(fs, remote_meta_bak_path, tablet_meta_pb));
+    }
+    LOG(INFO) << "No tablet meta file founded, init needed. tablet_id: " << tablet_id();
+    return Status::OK();
+}
+
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, const std::string& meta_path,
+                                        TabletMetaPB* tablet_meta_pb) {
+    io::FileReaderSPtr tablet_meta_reader;
+    RETURN_IF_ERROR(fs->open_file(meta_path, &tablet_meta_reader));
+    DCHECK(tablet_meta_reader != nullptr);
+    auto file_size = tablet_meta_reader->size();
+    size_t bytes_read;
+    uint8_t* buf = new uint8_t[file_size];
+    Slice slice(buf, file_size);
+    IOContext io_ctx;
+    Status st = tablet_meta_reader->read_at(0, slice, io_ctx, &bytes_read);
+    if (!st.ok()) {
+        tablet_meta_reader->close();
+        return st;
+    }
+    tablet_meta_reader->close();
+    if (!tablet_meta_pb->ParseFromString(slice.to_string())) {
+        LOG(WARNING) << "parse tablet meta failed";
+        return Status::OLAPInternalError(OLAP_ERR_INIT_FAILED);
+    }
+    return Status::OK();
+}
+
+// Remote tablet meta file need to write 2 files, and then delete remote_meta_bak_path file.

Review Comment:
   warning: use of undeclared identifier 'OLAP_ERR_INIT_FAILED' [clang-diagnostic-error]
   ```cpp
    tablet meta failed";
                                                                  ^
   ```
   



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