github-actions[bot] commented on code in PR #65979:
URL: https://github.com/apache/doris/pull/65979#discussion_r3654165431
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1828,7 +1828,8 @@ int disk_used_percentage(const std::string& path,
std::pair<int, int>* percent)
unsigned long long inode_free = stat.f_ffree;
unsigned long long inode_total = stat.f_files;
- int inode_percentage = cast_set<int>(inode_free * 100 / inode_total);
+ // Some filesystems use dynamic inodes(e.g. btrfs) and stat.f_files are
reported as zero.
Review Comment:
The exact crash input still has no deterministic regression coverage. The
direct `disk_used_percentage` test is commented out, and the active sync point
runs only after this calculation, so existing tests cannot inject `f_files ==
0`; removing or breaking this guard would still leave BE UT green. Please add a
BE unit test that supplies nonzero block capacity with `f_files = f_ffree = 0`
before the calculation and asserts success plus 0% inode usage. A small pure
percentage helper or a BE-test `statfs` hook, like the nearby inode-estimator
hook, would make this deterministic.
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1828,7 +1828,8 @@ int disk_used_percentage(const std::string& path,
std::pair<int, int>* percent)
unsigned long long inode_free = stat.f_ffree;
unsigned long long inode_total = stat.f_files;
- int inode_percentage = cast_set<int>(inode_free * 100 / inode_total);
+ // Some filesystems use dynamic inodes(e.g. btrfs) and stat.f_files are
reported as zero.
+ int inode_percentage = inode_total > 0 ? cast_set<int>(inode_free * 100 /
inode_total) : 100;
Review Comment:
This fixes the inode denominator, but the same monitor still crashes before
reaching it when a successful `statfs` cannot report block capacity:
`nonroot_total` immediately above is zero and is used by both `/` and `%`. This
is reachable with high-level FUSE filesystems that omit the optional `statfs`
callback: libfuse returns success with zero block counts. Doris then accepts
`disk_capacity == 0`, constructs the cache, and starts this monitor; if only
this first division is guarded, `check_need_evict_cache_in_advance()` also
divides by `_capacity == 0`. Please define the zero-capacity contract end to
end—reject/disable such a disk cache, or represent unavailable capacity without
either denominator—and cover the all-zero successful-statfs case.
--
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]
