zclllyybb commented on issue #65545: URL: https://github.com/apache/doris/issues/65545#issuecomment-4956785081
Breakwater-GitHub-Analysis-Slot: slot_da6fafffe61f This content is generated by AI for reference only. Initial analysis for apache/doris#65545: The stack trace points to a real BE bug in the file-cache disk monitor path, not only an environment/configuration issue. In the reported commit `7126cf65d96ebc43fce0906f51e92c1a2ccf24a6` (`4.1.3-rc02`), `be/src/io/cache/block_file_cache.cpp::disk_used_percentage()` reads `stat.f_ffree` and `stat.f_files`, then computes: ```cpp int inode_percentage = cast_set<int>(inode_free * 100 / inode_total); ``` There is no guard for `inode_total == 0`. The issue's GDB output shows `stat.f_files = 0` and `stat.f_ffree = 0`, so the SIGFPE at that line is expected from the current code. I also checked refreshed upstream refs: the same unguarded expression is still present on `upstream/branch-4.1` (`598749afd1d`) and `upstream/master` (`8b52996cd9a`). Impact: - The crash occurs in the background file-cache monitor: `BlockFileCache::run_background_monitor()` -> `check_disk_resource_limit()` -> `disk_used_percentage()`. - It can happen when the cache path is on a filesystem that reports block capacity but does not expose inode accounting through `statfs()` (`f_files/f_ffree == 0`). - `statfs()` itself succeeds in this case, so the existing `ret != 0` error handling is bypassed. Suggested fix: - Treat `stat.f_files == 0` as "inode usage unavailable" in `disk_used_percentage()` instead of dividing by it. - A practical behavior is to keep the existing disk-capacity calculation and set inode used percent to `0` or otherwise skip inode-pressure decisions when total inode count is unavailable. This keeps space-based protection active and avoids falsely entering inode-pressure mode on filesystems that do not report inodes. - This matches nearby file-cache behavior: `FSFileCacheStorage::estimate_file_count_from_inode()` already checks `vfs.f_files == 0` and skips inode estimation instead of using it. - If the helper is being hardened more generally, it is also worth guarding `nonroot_total == 0`, although the evidence in this issue specifically proves the inode denominator problem. Suggested tests: - Add a BE unit test for `disk_used_percentage()` or for a factored statfs-result helper with `f_files = 0` and `f_ffree = 0`, asserting no SIGFPE and a bounded returned percentage pair. - The existing `BlockFileCache::disk_used_percentage:1` sync point runs after the calculation, so it cannot reproduce this crash by itself; a small test hook or a factored helper would make the zero-inode case testable. - Keep the existing `check_disk_resource_limit()` / `check_need_evict_cache_in_advance()` coverage for high space and high inode percentages, since those paths should still trigger resource-limit or early-eviction behavior when inode statistics are available. Missing information that would help define the affected environment scope: - Filesystem type and mount options for the file-cache path, for example `statfs.f_type`, `df -T`, and the configured file cache directory. - Whether the same cache path/filesystem is used by all BEs or only by the crashing BE. - Which target branches are needed for the fix: `master`, `branch-4.1`, and/or the affected cloud build branch/tag. -- 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]
