github-actions[bot] commented on code in PR #33519:
URL: https://github.com/apache/doris/pull/33519#discussion_r1560368968
##########
be/src/vec/spill/spill_stream_manager.cpp:
##########
@@ -271,18 +275,68 @@ Status SpillDataDir::init() {
RETURN_NOT_OK_STATUS_WITH_WARN(Status::IOError("opendir failed,
path={}", _path),
"check file exist failed");
}
-
+ return update_capacity();
+}
+Status SpillDataDir::update_capacity() {
+ std::lock_guard<std::mutex> l(_mutex);
+ RETURN_IF_ERROR(io::global_local_filesystem()->get_space_info(_path,
&_disk_capacity_bytes,
+
&_available_bytes));
+ if (_shared_with_storage_path) {
+ _limit_bytes = (size_t)(_disk_capacity_bytes *
+ (config::storage_flood_stage_usage_percent /
100.0) *
+ (config::spill_storage_usage_percent / 100.0));
+ } else {
+ _limit_bytes =
+ (size_t)(_disk_capacity_bytes *
(config::spill_storage_usage_percent / 100.0));
+ }
return Status::OK();
}
bool SpillDataDir::reach_capacity_limit(int64_t incoming_data_size) {
- double used_pct = get_usage(incoming_data_size);
- int64_t left_bytes = _available_bytes - incoming_data_size;
- if (used_pct >= config::storage_flood_stage_usage_percent / 100.0 &&
- left_bytes <= config::storage_flood_stage_left_capacity_bytes) {
- LOG(WARNING) << "reach capacity limit. used pct: " << used_pct
- << ", left bytes: " << left_bytes << ", path: " << _path;
- return true;
+ std::lock_guard<std::mutex> l(_mutex);
+ if (_shared_with_storage_path) {
+ VLOG_DEBUG << fmt::format(
+ "spill data path: {}, limit: {}, used: {}, available: {}, "
+ "incoming "
+ "bytes: {}",
+ _path, PrettyPrinter::print_bytes(_limit_bytes),
+ PrettyPrinter::print_bytes(_used_bytes),
+ PrettyPrinter::print_bytes(_available_bytes),
+ PrettyPrinter::print_bytes(incoming_data_size));
+ int64_t left_bytes = _available_bytes - incoming_data_size;
+ if (_used_bytes + incoming_data_size > _limit_bytes ||
+ left_bytes <= config::storage_flood_stage_left_capacity_bytes) {
+ LOG(WARNING) << fmt::format(
+ "spill data reach limit, path: {}, limit: {}, used: {},
available: {}, "
+ "incoming "
+ "bytes: {}",
+ _path, PrettyPrinter::print_bytes(_limit_bytes),
+ PrettyPrinter::print_bytes(_used_bytes),
+ PrettyPrinter::print_bytes(_available_bytes),
+ PrettyPrinter::print_bytes(incoming_data_size));
+ return true;
Review Comment:
warning: redundant boolean literal in conditional return statement
[readability-simplify-boolean-expr]
be/src/vec/spill/spill_stream_manager.cpp:305:
```diff
- if (_used_bytes + incoming_data_size > _limit_bytes ||
- left_bytes <= config::storage_flood_stage_left_capacity_bytes)
{
- LOG(WARNING) << fmt::format(
- "spill data reach limit, path: {}, limit: {}, used:
{}, available: {}, "
- "incoming "
- "bytes: {}",
- _path, PrettyPrinter::print_bytes(_limit_bytes),
- PrettyPrinter::print_bytes(_used_bytes),
- PrettyPrinter::print_bytes(_available_bytes),
- PrettyPrinter::print_bytes(incoming_data_size));
- return true;
- }
- return false;
+ return _used_bytes + incoming_data_size > _limit_bytes ||
+ left_bytes <= config::storage_flood_stage_left_capacity_bytes;
```
##########
be/src/vec/spill/spill_stream_manager.cpp:
##########
@@ -271,18 +275,68 @@
RETURN_NOT_OK_STATUS_WITH_WARN(Status::IOError("opendir failed,
path={}", _path),
"check file exist failed");
}
-
+ return update_capacity();
+}
+Status SpillDataDir::update_capacity() {
+ std::lock_guard<std::mutex> l(_mutex);
+ RETURN_IF_ERROR(io::global_local_filesystem()->get_space_info(_path,
&_disk_capacity_bytes,
+
&_available_bytes));
+ if (_shared_with_storage_path) {
+ _limit_bytes = (size_t)(_disk_capacity_bytes *
+ (config::storage_flood_stage_usage_percent /
100.0) *
+ (config::spill_storage_usage_percent / 100.0));
+ } else {
+ _limit_bytes =
+ (size_t)(_disk_capacity_bytes *
(config::spill_storage_usage_percent / 100.0));
+ }
return Status::OK();
}
bool SpillDataDir::reach_capacity_limit(int64_t incoming_data_size) {
- double used_pct = get_usage(incoming_data_size);
- int64_t left_bytes = _available_bytes - incoming_data_size;
- if (used_pct >= config::storage_flood_stage_usage_percent / 100.0 &&
- left_bytes <= config::storage_flood_stage_left_capacity_bytes) {
- LOG(WARNING) << "reach capacity limit. used pct: " << used_pct
- << ", left bytes: " << left_bytes << ", path: " << _path;
- return true;
+ std::lock_guard<std::mutex> l(_mutex);
+ if (_shared_with_storage_path) {
+ VLOG_DEBUG << fmt::format(
+ "spill data path: {}, limit: {}, used: {}, available: {}, "
+ "incoming "
+ "bytes: {}",
+ _path, PrettyPrinter::print_bytes(_limit_bytes),
+ PrettyPrinter::print_bytes(_used_bytes),
+ PrettyPrinter::print_bytes(_available_bytes),
+ PrettyPrinter::print_bytes(incoming_data_size));
+ int64_t left_bytes = _available_bytes - incoming_data_size;
+ if (_used_bytes + incoming_data_size > _limit_bytes ||
+ left_bytes <= config::storage_flood_stage_left_capacity_bytes) {
+ LOG(WARNING) << fmt::format(
+ "spill data reach limit, path: {}, limit: {}, used: {},
available: {}, "
+ "incoming "
+ "bytes: {}",
+ _path, PrettyPrinter::print_bytes(_limit_bytes),
+ PrettyPrinter::print_bytes(_used_bytes),
+ PrettyPrinter::print_bytes(_available_bytes),
+ PrettyPrinter::print_bytes(incoming_data_size));
+ return true;
+ }
+ return false;
+ } else {
+ double used_pct = _disk_capacity_bytes == 0
+ ? 0
+ : (_disk_capacity_bytes - _available_bytes +
incoming_data_size) /
+ (double)_disk_capacity_bytes;
+ VLOG_DEBUG << fmt::format(
+ "spill data path: {}, capacity: {}, available: {}, used pct:
{}, incoming bytes: "
+ "{}",
+ _path, PrettyPrinter::print_bytes(_disk_capacity_bytes),
+ PrettyPrinter::print_bytes(_available_bytes), used_pct,
+ PrettyPrinter::print_bytes(incoming_data_size));
+ if (used_pct >= config::spill_storage_usage_percent / 100.0) {
+ LOG(WARNING) << fmt::format(
+ "spill data reach limit, path: {}, capacity: {},
available: {}, incoming "
+ "bytes: {}",
+ _path, PrettyPrinter::print_bytes(_disk_capacity_bytes),
+ PrettyPrinter::print_bytes(_available_bytes),
+ PrettyPrinter::print_bytes(incoming_data_size));
+ return true;
Review Comment:
warning: redundant boolean literal in conditional return statement
[readability-simplify-boolean-expr]
be/src/vec/spill/spill_stream_manager.cpp:329:
```diff
- if (used_pct >= config::spill_storage_usage_percent / 100.0) {
- LOG(WARNING) << fmt::format(
- "spill data reach limit, path: {}, capacity: {},
available: {}, incoming "
- "bytes: {}",
- _path,
PrettyPrinter::print_bytes(_disk_capacity_bytes),
- PrettyPrinter::print_bytes(_available_bytes),
- PrettyPrinter::print_bytes(incoming_data_size));
- return true;
- }
- return false;
+ return used_pct >= config::spill_storage_usage_percent / 100.0;
```
##########
be/src/vec/spill/spill_stream_manager.h:
##########
@@ -54,25 +55,61 @@
Status update_capacity();
- double get_usage(int64_t incoming_data_size) const {
- return _disk_capacity_bytes == 0
- ? 0
- : (_disk_capacity_bytes - _available_bytes +
incoming_data_size) /
- (double)_disk_capacity_bytes;
+ void update_usage(int64_t incoming_data_size) {
+ if (_shared_with_storage_path) {
+ std::lock_guard<std::mutex> l(_mutex);
+ _used_bytes += incoming_data_size;
+ }
+ }
+
+ int64_t get_used_bytes() {
+ std::lock_guard<std::mutex> l(_mutex);
+ if (_shared_with_storage_path) {
+ return _used_bytes;
+ } else {
+ return _disk_capacity_bytes - _available_bytes;
+ }
+ }
+
+ double get_usage(int64_t incoming_data_size) {
Review Comment:
warning: method 'get_usage' can be made const
[readability-make-member-function-const]
```suggestion
double get_usage(int64_t incoming_data_size) const {
```
##########
be/src/vec/spill/spill_stream_manager.h:
##########
@@ -54,25 +55,61 @@ class SpillDataDir {
Status update_capacity();
- double get_usage(int64_t incoming_data_size) const {
- return _disk_capacity_bytes == 0
- ? 0
- : (_disk_capacity_bytes - _available_bytes +
incoming_data_size) /
- (double)_disk_capacity_bytes;
+ void update_usage(int64_t incoming_data_size) {
+ if (_shared_with_storage_path) {
+ std::lock_guard<std::mutex> l(_mutex);
+ _used_bytes += incoming_data_size;
+ }
+ }
+
+ int64_t get_used_bytes() {
Review Comment:
warning: method 'get_used_bytes' can be made const
[readability-make-member-function-const]
```suggestion
int64_t get_used_bytes() const {
```
##########
be/src/vec/spill/spill_stream_manager.h:
##########
@@ -54,25 +55,61 @@
Status update_capacity();
- double get_usage(int64_t incoming_data_size) const {
- return _disk_capacity_bytes == 0
- ? 0
- : (_disk_capacity_bytes - _available_bytes +
incoming_data_size) /
- (double)_disk_capacity_bytes;
+ void update_usage(int64_t incoming_data_size) {
+ if (_shared_with_storage_path) {
+ std::lock_guard<std::mutex> l(_mutex);
+ _used_bytes += incoming_data_size;
+ }
+ }
+
+ int64_t get_used_bytes() {
+ std::lock_guard<std::mutex> l(_mutex);
+ if (_shared_with_storage_path) {
+ return _used_bytes;
+ } else {
+ return _disk_capacity_bytes - _available_bytes;
+ }
+ }
+
+ double get_usage(int64_t incoming_data_size) {
+ std::lock_guard<std::mutex> l(_mutex);
+ if (_shared_with_storage_path) {
+ return _limit_bytes == 0 ? 0 : _used_bytes + incoming_data_size /
(double)_limit_bytes;
+
+ } else {
+ return _disk_capacity_bytes == 0
+ ? 0
+ : (_disk_capacity_bytes - _available_bytes +
incoming_data_size) /
+ (double)_disk_capacity_bytes;
+ }
+ }
+
+ int64_t storage_limit() {
Review Comment:
warning: method 'storage_limit' can be made const
[readability-make-member-function-const]
```suggestion
int64_t storage_limit() const {
```
--
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
