github-actions[bot] commented on code in PR #67498:
URL: https://github.com/apache/doris/pull/67498#discussion_r4226542050
##########
be/src/exec/spill/spill_file_manager.cpp:
##########
@@ -410,68 +411,96 @@ void
SpillFileManager::_retry_pending_query_spill_directories() {
}
void SpillFileManager::gc(int32_t max_work_time_ms) {
- bool exists = true;
bool has_work = false;
int64_t max_work_time_ns = max_work_time_ms * 1000L * 1000L;
MonotonicStopWatch watch;
watch.start();
+ // One summary line per spill store, printed together with the inode usage
of each store so
+ // that an inode leak or a growing gc backlog can be diagnosed from the BE
log alone.
+ std::vector<std::string> store_summaries;
Defer defer {[&]() {
if (has_work) {
std::string msg(
fmt::format("spill gc time: {}",
PrettyPrinter::print(watch.elapsed_time(),
TUnit::TIME_NS)));
msg += ", spill storage:\n";
- for (const auto& [path, store_dir] : _spill_store_map) {
- msg += " " + store_dir->debug_string();
+ for (const auto& summary : store_summaries) {
+ msg += " " + summary;
msg += "\n";
}
LOG(INFO) << msg;
}
}};
_retry_pending_query_spill_directories();
for (const auto& [path, store_dir] : _spill_store_map) {
- std::string gc_root_dir = store_dir->get_spill_data_gc_path();
+ SpillGcStats stats;
Review Comment:
[P2] Include pending query deletions in the GC diagnostics. When query
teardown cannot remove an active `spill/<query>` directory,
`_retry_pending_query_spill_directories()` retains it, but this new
`SpillGcStats` starts only after that retry and counts only `spill_gc` entries.
With an empty `spill_gc`, persistent retry failures or a successful retry
produce no GC summary; if another store triggers the summary, this store still
reports `failed: 0`. Please include the pending queue's backlog and retry
outcomes in the per-store summary and its log gate.
`QueryContextRetriesSpillDirectoryDeletionUntilSuccess` exercises this path but
does not check diagnostics.
##########
be/src/exec/spill/spill_file_manager.cpp:
##########
@@ -577,6 +626,32 @@ Status SpillDataDir::update_capacity() {
return Status::OK();
}
+// Inode statistics are for monitoring only, so a failure to read them never
fails
+// update_capacity(); the previous gauge values are kept and a throttled
warning is logged.
+void SpillDataDir::_update_inode_usage() {
+ size_t inode_total = 0;
+ size_t inode_available = 0;
+ auto st = io::global_local_filesystem()->get_inode_info(_path,
&inode_total, &inode_available);
+ if (!st.ok()) {
+ LOG_EVERY_T(WARNING, 60) << fmt::format("failed to get inode info of
spill path {}: {}",
+ _path, st.to_string());
+ return;
+ }
+ spill_disk_inode_total->set_value(inode_total);
+ spill_disk_inode_available->set_value(inode_available);
+
+ if (inode_total == 0) {
+ return;
+ }
+ if (inode_usage(inode_total, inode_available) >=
+ config::storage_flood_stage_usage_percent / 100.0) {
+ LOG_EVERY_T(WARNING, 60) << fmt::format(
Review Comment:
[P2] Rate-limit high-inode warnings per spill store.
`_spill_gc_thread_callback()` refreshes every store in a stable order, but this
`LOG_EVERY_T(WARNING, 60)` has one call-site timer shared by all `SpillDataDir`
instances. If two stores stay above the threshold, the first consumes every
interval and the second path never appears in the warning log. Use a per-path
throttle or emit one periodic warning that lists every affected store; the
inode-read failure warning above has the same multi-store suppression.
--
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]
