github-actions[bot] commented on code in PR #15832:
URL: https://github.com/apache/doris/pull/15832#discussion_r1067804702
##########
be/src/io/fs/file_reader_options.h:
##########
@@ -38,23 +38,29 @@ 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) {
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) {
^
```
**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) {}
^
```
##########
be/src/olap/rowset/beta_rowset.cpp:
##########
@@ -134,7 +139,11 @@ Status
BetaRowset::load_segments(std::vector<segment_v2::SegmentSharedPtr>* segm
for (int seg_id = 0; seg_id < num_segments(); ++seg_id) {
auto seg_path = segment_file_path(seg_id);
std::shared_ptr<segment_v2::Segment> segment;
- auto s = segment_v2::Segment::open(fs, seg_path, seg_id, rowset_id(),
_schema, &segment);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
Review Comment:
warning: 'this' argument to member function 'set_cache_path' has type 'const
doris::io::CachePathPolicy', but function is not marked const
[clang-diagnostic-error]
```cpp
reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
^
```
**be/src/io/fs/file_reader_options.h:41:** 'set_cache_path' declared here
```cpp
virtual void set_cache_path(const std::string& cache_path) {}
^
```
##########
be/src/olap/rowset/beta_rowset.cpp:
##########
@@ -156,7 +165,11 @@
}
auto seg_path = segment_file_path(seg_id);
std::shared_ptr<segment_v2::Segment> segment;
- auto s = segment_v2::Segment::open(fs, seg_path, seg_id, rowset_id(),
_schema, &segment);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
Review Comment:
warning: 'this' argument to member function 'set_cache_path' has type 'const
doris::io::CachePathPolicy', but function is not marked const
[clang-diagnostic-error]
```cpp
reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
^
```
**be/src/io/fs/file_reader_options.h:41:** 'set_cache_path' declared here
```cpp
virtual void set_cache_path(const std::string& cache_path) {}
^
```
##########
be/src/io/fs/file_reader_options.h:
##########
@@ -38,23 +38,29 @@
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) {
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 {
```
##########
be/src/olap/tablet.cpp:
##########
@@ -1438,6 +1439,10 @@
tablet_info->__set_is_in_memory(_tablet_meta->tablet_schema()->is_in_memory());
tablet_info->__set_replica_id(replica_id());
tablet_info->__set_remote_data_size(_tablet_meta->tablet_remote_size());
+ tablet_info->__set_storage_policy(_tablet_meta->storage_policy());
+ if (!tablet_info->storage_policy.empty()) {
+
tablet_info->__set_cooldown_replica_id(_tablet_meta->cooldown_replica_id());
+ }
}
// should use this method to get a copy of current tablet meta
Review Comment:
warning: no member named 'storage_policy' in 'doris::TTabletInfo'
[clang-diagnostic-error]
```cpp
storage_policy.empty()) {
^
```
##########
be/src/olap/tablet.cpp:
##########
@@ -1729,18 +1757,119 @@
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();
}
+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:1781:** previous definition is here
```cpp
s(remote_meta_path, &exist));
^
```
##########
be/src/olap/rowset/beta_rowset.cpp:
##########
@@ -326,7 +339,11 @@
for (int seg_id = 0; seg_id < num_segments(); ++seg_id) {
auto seg_path = segment_file_path(seg_id);
std::shared_ptr<segment_v2::Segment> segment;
- auto s = segment_v2::Segment::open(fs, seg_path, seg_id, rowset_id(),
_schema, &segment);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
Review Comment:
warning: 'this' argument to member function 'set_cache_path' has type 'const
doris::io::CachePathPolicy', but function is not marked const
[clang-diagnostic-error]
```cpp
reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
^
```
**be/src/io/fs/file_reader_options.h:41:** 'set_cache_path' declared here
```cpp
virtual void set_cache_path(const std::string& cache_path) {}
^
```
##########
be/test/io/cache/remote_file_cache_test.cpp:
##########
@@ -141,7 +141,10 @@ class RemoteFileCacheTest : public ::testing::Test {
EXPECT_NE("", writer.min_encoded_key().to_string());
EXPECT_NE("", writer.max_encoded_key().to_string());
- st = segment_v2::Segment::open(fs, path, 0, {}, query_schema, res);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(0));
Review Comment:
warning: use of undeclared identifier 'segment_cache_path'
[clang-diagnostic-error]
```cpp
reader_options.path_policy.set_cache_path(segment_cache_path(0));
^
```
##########
be/src/olap/tablet_meta.cpp:
##########
@@ -521,6 +523,8 @@
}
_storage_policy = tablet_meta_pb.storage_policy();
+ _cooldown_replica_id = tablet_meta_pb.cooldown_replica_id();
+ _cooldown_term = tablet_meta_pb.cooldown_term();
Review Comment:
warning: no member named 'cooldown_term' in 'doris::TabletMetaPB'
[clang-diagnostic-error]
```cpp
_cooldown_term = tablet_meta_pb.cooldown_term();
^
```
##########
be/src/olap/tablet_meta.cpp:
##########
@@ -521,6 +523,8 @@ void TabletMeta::init_from_pb(const TabletMetaPB&
tablet_meta_pb) {
}
_storage_policy = tablet_meta_pb.storage_policy();
+ _cooldown_replica_id = tablet_meta_pb.cooldown_replica_id();
Review Comment:
warning: no member named 'cooldown_replica_id' in 'doris::TabletMetaPB'
[clang-diagnostic-error]
```cpp
_cooldown_replica_id = tablet_meta_pb.cooldown_replica_id();
^
```
##########
be/test/tools/benchmark_tool.cpp:
##########
@@ -363,7 +363,10 @@
writer.finalize(&file_size, &index_size);
file_writer->close();
- Segment::open(fs, path, "", seg_id, {}, &_tablet_schema, res);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
+ Segment::open(fs, path, seg_id, {}, &_tablet_schema, reader_options,
res);
Review Comment:
warning: no viable conversion from 'doris::TabletSchema *' to
'doris::TabletSchemaSPtr' (aka 'shared_ptr<doris::TabletSchema>')
[clang-diagnostic-error]
```cpp
Segment::open(fs, path, seg_id, {}, &_tablet_schema, reader_options,
res);
^
```
**/usr/include/c++/11/bits/shared_ptr.h:149:** candidate constructor not
viable: no known conversion from 'doris::TabletSchema *' to 'const
std::shared_ptr<doris::TabletSchema> &' for 1st argument
```cpp
shared_ptr(const shared_ptr&) noexcept = default; ///< Copy constructor
^
```
**/usr/include/c++/11/bits/shared_ptr.h:303:** candidate constructor not
viable: no known conversion from 'doris::TabletSchema *' to
'std::shared_ptr<doris::TabletSchema> &&' for 1st argument
```cpp
shared_ptr(shared_ptr&& __r) noexcept
^
```
**/usr/include/c++/11/bits/shared_ptr.h:356:** candidate constructor not
viable: no known conversion from 'doris::TabletSchema *' to 'std::nullptr_t'
for 1st argument
```cpp
constexpr shared_ptr(nullptr_t) noexcept : shared_ptr() { }
^
```
**/usr/include/c++/11/bits/shared_ptr.h:295:** candidate template ignored:
could not match 'shared_ptr<_Yp>' against 'doris::TabletSchema *'
```cpp
shared_ptr(const shared_ptr<_Yp>& __r) noexcept
^
```
**/usr/include/c++/11/bits/shared_ptr.h:312:** candidate template ignored:
could not match 'shared_ptr<_Yp>' against 'doris::TabletSchema *'
```cpp
shared_ptr(shared_ptr<_Yp>&& __r) noexcept
^
```
**/usr/include/c++/11/bits/shared_ptr.h:331:** candidate template ignored:
could not match 'auto_ptr<_Yp>' against 'doris::TabletSchema *'
```cpp
shared_ptr(auto_ptr<_Yp>&& __r);
^
```
**/usr/include/c++/11/bits/shared_ptr.h:339:** candidate template ignored:
could not match 'unique_ptr<_Yp, _Del>' against 'doris::TabletSchema *'
```cpp
shared_ptr(unique_ptr<_Yp, _Del>&& __r)
^
```
**/usr/include/c++/11/bits/shared_ptr.h:407:** candidate template ignored:
could not match '_Sp_alloc_shared_tag<_Alloc>' against 'doris::TabletSchema *'
```cpp
shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args)
^
```
**/usr/include/c++/11/bits/shared_ptr.h:159:** explicit constructor is not a
candidate
```cpp
shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
^
```
**/usr/include/c++/11/bits/shared_ptr.h:324:** explicit constructor is not a
candidate
```cpp
explicit shared_ptr(const weak_ptr<_Yp>& __r)
^
```
**be/src/olap/rowset/segment_v2/segment.h:64:** passing argument to
parameter 'tablet_schema' here
```cpp
RowsetId rowset_id, TabletSchemaSPtr tablet_schema,
^
```
##########
be/test/tools/benchmark_tool.cpp:
##########
@@ -363,7 +363,10 @@ class SegmentBenchmark : public BaseBenchmark {
writer.finalize(&file_size, &index_size);
file_writer->close();
- Segment::open(fs, path, "", seg_id, {}, &_tablet_schema, res);
+ io::FileReaderOptions
reader_options(io::cache_type_from_string(config::file_cache_type),
+ io::SegmentCachePathPolicy());
+ reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
Review Comment:
warning: use of undeclared identifier 'segment_cache_path'
[clang-diagnostic-error]
```cpp
reader_options.path_policy.set_cache_path(segment_cache_path(seg_id));
^
```
##########
be/src/olap/tablet.cpp:
##########
@@ -1438,6 +1439,10 @@ void Tablet::build_tablet_report_info(TTabletInfo*
tablet_info,
tablet_info->__set_is_in_memory(_tablet_meta->tablet_schema()->is_in_memory());
tablet_info->__set_replica_id(replica_id());
tablet_info->__set_remote_data_size(_tablet_meta->tablet_remote_size());
+ tablet_info->__set_storage_policy(_tablet_meta->storage_policy());
+ if (!tablet_info->storage_policy.empty()) {
+
tablet_info->__set_cooldown_replica_id(_tablet_meta->cooldown_replica_id());
Review Comment:
warning: no member named '__set_storage_policy' in 'doris::TTabletInfo'
[clang-diagnostic-error]
```cpp
^
```
--
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
