Lchangliang commented on code in PR #29537:
URL: https://github.com/apache/doris/pull/29537#discussion_r1442467448


##########
be/src/olap/options.cpp:
##########
@@ -244,18 +268,18 @@ Status parse_conf_cache_paths(const std::string& 
config_path, std::vector<CacheP
     return Status::OK();
 }
 
-io::FileCacheSettings CachePath::init_settings() const {
+io::FileCacheSettings CachePath::init_settings(const std::vector<size_t>& 
percents) const {

Review Comment:
   The percents is member variable. Not need the input param.



##########
be/test/io/cache/file_block_cache_test.cpp:
##########
@@ -118,7 +118,7 @@ TEST(LRUFileCache, init) {
     EXPECT_TRUE(parse_conf_cache_paths(string, cache_paths));
     EXPECT_EQ(cache_paths.size(), 2);
     for (const auto& cache_path : cache_paths) {
-        io::FileCacheSettings settings = cache_path.init_settings();
+        io::FileCacheSettings settings = 
cache_path.init_settings(cache_path.percents);

Review Comment:
   Add some ut to cover it.



##########
be/src/olap/options.cpp:
##########
@@ -235,7 +236,30 @@ Status parse_conf_cache_paths(const std::string& 
config_path, std::vector<CacheP
             return Status::InvalidArgument(
                     "total_size or query_limit should not less than or equal 
to zero");
         }
-        paths.emplace_back(std::move(path), total_size, query_limit_bytes);
+
+        // percent
+        std::vector<size_t> percents;
+        if (map.HasMember(CACHE_PERCENTS.c_str())) {
+            auto& percentArray = map.FindMember(CACHE_PERCENTS.c_str())->value;
+            if (percentArray.IsArray()) {
+                for (auto& v : percentArray.GetArray()) {
+                    if (v.IsUint()) {
+                        percents.push_back(v.GetUint());
+                    } else {
+                        return Status::InvalidArgument("percent should be 
uint");
+                    }
+                }
+            } else {
+                return Status::InvalidArgument("percents should be array");
+            }
+        } else {
+            percents.assign({85, 10, 5});
+        }
+        if (percents.size() != 3 || percents[0] + percents[1] + percents[2] != 
100) {
+            return Status::InvalidArgument("wrong percents value");

Review Comment:
   More detail. The array size is not equal to 3 or the sum is not equal to 100.



##########
be/src/olap/options.cpp:
##########
@@ -200,7 +201,7 @@ void parse_conf_broken_store_paths(const string& 
config_path, std::set<std::stri
  *  [
  *    {"path": "storage1", "total_size":53687091200,"query_limit": 
"10737418240"},
  *    {"path": "storage2", "total_size":53687091200},
- *    {"path": "storage3", "total_size":53687091200},
+ *    {"path": "storage3", "total_size":53687091200, percents: [85,10,5]}

Review Comment:
   Modify the annotation in "config.h" too



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to