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


##########
be/src/io/fs/file_reader_options.h:
##########
@@ -38,23 +38,25 @@
 public:
     // path: the path of file which will be cached
     // return value: the cache path of the given file.
-    virtual std::string get_cache_path(const std::string& path) const { return 
""; }
+    virtual std::string get_cache_path() const { return ""; }
+    virtual void set_cache_path(const std::string& cache_path) {}
 };
 
 class NoCachePathPolicy : public CachePathPolicy {
 public:
     NoCachePathPolicy() = default;
-    std::string get_cache_path(const std::string& path) const override { 
return path; }
+    std::string get_cache_path() const override { return ""; }
 };
 
 class SegmentCachePathPolicy : public CachePathPolicy {
 public:
     SegmentCachePathPolicy() = default;
-    std::string get_cache_path(const std::string& path) const override {
-        // the segment file path is 
{rowset_dir}/{schema_hash}/{rowset_id}_{seg_num}.dat
-        // cache path is: {rowset_dir}/{schema_hash}/{rowset_id}_{seg_num}/
-        return path.substr(0, path.size() - 4) + "/";
-    }
+
+    void set_cache_path(const std::string& cache_path) { _cache_path = 
cache_path; }

Review Comment:
   warning: annotate this function with 'override' or (rarely) 'final' 
[modernize-use-override]
   
   ```suggestion
       void set_cache_path(const std::string& cache_path) override { 
_cache_path = cache_path; }
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1717,27 +1735,136 @@ Status Tablet::cooldown() {
     new_rowset_meta->set_resource_id(dest_fs->resource_id());
     new_rowset_meta->set_fs(dest_fs);
     new_rowset_meta->set_creation_time(time(nullptr));
+
+    // write remote tablet meta
+    TabletMetaPB remote_tablet_meta_pb;
+    _tablet_meta->to_meta_pb(true, &remote_tablet_meta_pb);
+    new_rowset_meta->to_rowset_pb(remote_tablet_meta_pb.add_rs_metas());
+    // upload rowset_meta to remote fs.
+    RETURN_IF_ERROR(_write_remote_tablet_meta(dest_fs, remote_tablet_meta_pb));
+
     RowsetSharedPtr new_rowset;
     RowsetFactory::create_rowset(_schema, _tablet_path, new_rowset_meta, 
&new_rowset);
 
     std::vector to_add {std::move(new_rowset)};
     std::vector to_delete {std::move(old_rowset)};
 
-    bool has_shutdown = false;
     {
         std::unique_lock meta_wlock(_meta_lock);
-        has_shutdown = tablet_state() == TABLET_SHUTDOWN;
-        if (!has_shutdown) {
+        if (tablet_state() != TABLET_SHUTDOWN) {
             modify_rowsets(to_add, to_delete);
-            _self_owned_remote_rowsets.insert(to_add.front());
             save_meta();
         }
     }
-    if (has_shutdown) {
-        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();
+}
+
+Status Tablet::_read_remote_tablet_meta(FileSystemSPtr fs, TabletMetaPB* 
tablet_meta_pb) {
+    std::string remote_meta_path =
+            BetaRowset::remote_tablet_meta_path(tablet_id(), 
_tablet_meta->cooldown_replica_id());
+    bool exist = false;
+    RETURN_IF_ERROR(fs->exists(remote_meta_path, &exist));
+    if (exist) {
+        IOContext io_ctx;
+        io::FileReaderSPtr tablet_meta_reader;
+        RETURN_IF_ERROR(fs->open_file(remote_meta_path, &tablet_meta_reader, 
&io_ctx));
+        if (tablet_meta_reader == nullptr) {
+            return Status::InternalError("tablet_meta_reader is null");
+        }
+        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();

Review Comment:
   warning: redefinition of 'io_ctx' [clang-diagnostic-error]
   ```cpp
   Context io_ctx;
           ^
   ```
   **be/src/olap/tablet.cpp:1769:** previous definition is here
   ```cpp
   s(remote_meta_path, &exist));
                                                                    ^
   ```
   



##########
be/src/io/fs/file_reader_options.h:
##########
@@ -38,23 +38,25 @@ class CachePathPolicy {
 public:
     // path: the path of file which will be cached
     // return value: the cache path of the given file.
-    virtual std::string get_cache_path(const std::string& path) const { return 
""; }
+    virtual std::string get_cache_path() const { return ""; }
+    virtual void set_cache_path(const std::string& cache_path) {}
 };
 
 class NoCachePathPolicy : public CachePathPolicy {
 public:
     NoCachePathPolicy() = default;
-    std::string get_cache_path(const std::string& path) const override { 
return path; }
+    std::string get_cache_path() const override { return ""; }
 };
 
 class SegmentCachePathPolicy : public CachePathPolicy {
 public:
     SegmentCachePathPolicy() = default;
-    std::string get_cache_path(const std::string& path) const override {
-        // the segment file path is 
{rowset_dir}/{schema_hash}/{rowset_id}_{seg_num}.dat
-        // cache path is: {rowset_dir}/{schema_hash}/{rowset_id}_{seg_num}/
-        return path.substr(0, path.size() - 4) + "/";
-    }
+
+    void set_cache_path(const std::string& cache_path) { _cache_path = 
cache_path; }

Review Comment:
   warning: 'set_cache_path' overrides a member function but is not marked 
'override' [clang-diagnostic-inconsistent-missing-override]
   ```cpp
       void set_cache_path(const std::string& cache_path) { _cache_path = 
cache_path; }
            ^
   ```
   **be/src/io/fs/file_reader_options.h:41:** overridden virtual function is 
here
   ```cpp
       virtual void set_cache_path(const std::string& cache_path) {}
                    ^
   ```
   



-- 
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-unsubscr...@doris.apache.org

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


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

Reply via email to