Skip to content

Conversation

@hengfeiyang
Copy link
Contributor

@hengfeiyang hengfeiyang commented Dec 5, 2025

This PR fixed the case there are some logs keep output
6f124e4b694dfd6882acc0632f43682b

The reason is the ZO_DISK_CACHE_RELEASE_SIZE default is 10% of ZO_DISK_CACHE_MAX_SIZE, and ZO_DISK_RESULT_CACHE_MAX_SIZE also is 10% of it. so even there is no data, but curr_size + release >= result_max_size then it start gc.

@github-actions github-actions bot added the ☢️ Bug Something isn't working label Dec 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

Failed to generate code suggestions for PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 5, 2025

Greptile Overview

Greptile Summary

This PR fixes excessive GC logging by proportionally adjusting the release_size threshold for RESULT_FILES and AGGREGATION_FILES based on their respective max sizes.

Key changes:

  • Calculated proportional release_size for RESULT_FILES using the ratio of max_size to result_max_size
  • Calculated proportional release_size for AGGREGATION_FILES using the ratio of max_size to aggregation_max_size
  • Added minimum threshold of 10MB to prevent too-small release sizes

Issues found:

  • Potential division by zero panic if result_max_size or aggregation_max_size is greater than max_size (integer division yields 0 denominator)
  • Should add zero-check guards before performing division

Confidence Score: 3/5

  • This PR fixes the immediate logging issue but introduces a potential division by zero bug that could cause runtime panics
  • The fix correctly addresses the root cause (mismatched release_size for different cache types), but the division logic lacks safety checks for edge cases where result_max_size or aggregation_max_size could be larger than max_size, leading to integer division by zero
  • src/infra/src/cache/file_data/disk.rs requires attention to add division by zero guards

Important Files Changed

File Analysis

Filename Score Overview
src/infra/src/cache/file_data/disk.rs 3/5 Fixed proportional release_size calculation for RESULT_FILES and AGGREGATION_FILES to prevent constant GC runs, but potential division by zero exists if result_max_size > max_size

Sequence Diagram

sequenceDiagram
    participant GC as GC Task
    participant Config as Config
    participant FILES as FILES Cache
    participant RESULT as RESULT_FILES Cache
    participant AGG as AGGREGATION_FILES Cache
    
    GC->>Config: get_config()
    Config-->>GC: cfg
    
    Note over GC: Process FILES
    loop for each file in FILES
        GC->>FILES: read()
        FILES-->>GC: r (cur_size, max_size)
        alt cur_size + release_size < max_size
            Note over GC: Skip GC (cache not full)
        else cache full
            GC->>FILES: write().gc(gc_size)
            FILES-->>GC: release bytes
        end
    end
    
    Note over GC: Calculate proportional release_size
    GC->>GC: release_size = max(10MB, release_size / (max_size / result_max_size))
    
    Note over GC: Process RESULT_FILES
    loop for each file in RESULT_FILES
        GC->>RESULT: read()
        RESULT-->>GC: r (cur_size, max_size)
        alt cur_size + release_size < max_size
            Note over GC: Skip GC (cache not full)
        else cache full
            GC->>RESULT: write().gc(gc_size)
            RESULT-->>GC: release bytes
        end
    end
    
    Note over GC: Calculate proportional release_size
    GC->>GC: release_size = max(10MB, release_size / (max_size / aggregation_max_size))
    
    Note over GC: Process AGGREGATION_FILES
    loop for each file in AGGREGATION_FILES
        GC->>AGG: read()
        AGG-->>GC: r (cur_size, max_size)
        alt cur_size + release_size < max_size
            Note over GC: Skip GC (cache not full)
        else cache full
            GC->>AGG: write().gc(gc_size)
            AGG-->>GC: release bytes
        end
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@hengfeiyang hengfeiyang merged commit 3c986f1 into main Dec 8, 2025
39 of 41 checks passed
@hengfeiyang hengfeiyang deleted the fix/disk-cache-gc branch December 8, 2025 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants