Skip to content

Conversation

@artemmufazalov
Copy link
Member

@artemmufazalov artemmufazalov commented Oct 22, 2025

Closes #2993

Examples:

Storage: https://nda.ya.ru/t/Ct51L8tA7LkAzN
Group: https://nda.ya.ru/t/qivlXwJ97LkAwv
VDisk: https://nda.ya.ru/t/k4mB7qyU7LkAuX

Greptile Overview

Updated On: 2025-10-22 12:38:57 UTC

Summary

This PR fixes VDisk usage display when AvailableSize is 0 by using SlotSize as the size limit instead. Previously, VDisks would show AllocatedSize + AvailableSize as the total, which resulted in incorrect 100% usage display when AvailableSize was 0, even if the disk hadn't reached its actual slot size limit.

Key Changes:

  • Renamed TotalSize to SizeLimit in VDisk types to better reflect its purpose
  • Modified prepareVDiskSizeFields() to use SlotSize as the limit when AvailableSize is 0 or undefined
  • Updated DiskStateProgressBar to clamp percentage values (can now exceed 100% when allocated exceeds slot size)
  • Added logic in PDisk utils to adjust slot total for visualization when used size exceeds limit
  • Refactored VDisk preparation to use consistent prepareGroupsVDisk() function
  • Comprehensive test coverage for all edge cases including overflow scenarios

Confidence Score: 5/5

  • This PR is safe to merge with no risk. The changes are well-tested, logically sound, and fix a legitimate bug.
  • The implementation is thorough with comprehensive test coverage for all edge cases. The logic correctly handles the scenario where AvailableSize is 0 by falling back to SlotSize. The refactoring improves code consistency, and the progress bar clamping prevents visual overflow issues. All test cases validate the expected behavior.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/utils/disks/prepareDisks.ts 5/5 Core logic change to use SlotSize as size limit when AvailableSize is 0. Logic is correct and well-tested.
src/components/VDiskInfo/VDiskInfo.tsx 5/5 UI updated to use SizeLimit instead of calculating from AllocatedSize + AvailableSize. Correct implementation.
src/components/DiskStateProgressBar/DiskStateProgressBar.tsx 5/5 Added clamping for diskAllocatedPercent to prevent visual overflow when usage exceeds 100%. Good defensive programming.
src/store/reducers/pdisk/utils.ts 5/5 Added logic to adjust slot Total when used exceeds limit for proper visualization sizing. Well-commented and tested.

Sequence Diagram

sequenceDiagram
    participant API as Backend API
    participant Prep as prepareVDiskSizeFields
    participant PDisk as preparePDiskDataResponse
    participant VDiskUI as VDiskInfo Component
    participant ProgBar as DiskStateProgressBar

    API->>Prep: VDisk data (AllocatedSize, AvailableSize, SlotSize)
    
    alt AvailableSize > 0
        Prep->>Prep: sizeLimit = allocated + available
    else AvailableSize = 0 and SlotSize exists
        Prep->>Prep: sizeLimit = slotSize
    else AvailableSize = 0 and no SlotSize
        Prep->>Prep: sizeLimit = allocated
    end
    
    Prep->>Prep: allocatedPercent = (allocated * 100) / sizeLimit
    Prep-->>PDisk: PreparedVDisk with SizeLimit & AllocatedPercent
    
    PDisk->>PDisk: Create slot items for visualization
    
    alt used > sizeLimit
        PDisk->>PDisk: Adjust total = used (for relative sizing)
    else used <= sizeLimit
        PDisk->>PDisk: total = sizeLimit
    end
    
    PDisk-->>VDiskUI: Slot data with adjusted totals
    VDiskUI->>VDiskUI: Display size as ProgressViewer(allocated, SizeLimit)
    
    PDisk-->>ProgBar: diskAllocatedPercent
    
    alt diskAllocatedPercent > 100
        ProgBar->>ProgBar: Clamp fillWidth to 100% (inverted: clamp to 0%)
    else diskAllocatedPercent <= 100
        ProgBar->>ProgBar: Use diskAllocatedPercent as-is
    end
    
    ProgBar->>ProgBar: Render progress bar with clamped width
Loading

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
378 375 0 1 2
Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

  1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
  2. Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 45.87 MB | Main: 45.87 MB
Diff: +0.46 KB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where VDisk size calculations were incorrect when AvailableSize is 0. The fix ensures that SlotSize is used as the size limit in such cases, preventing display issues and incorrect percentage calculations.

Key changes:

  • Replaced TotalSize with SizeLimit throughout the codebase to better represent the semantic meaning
  • Modified prepareVDiskSizeFields to use SlotSize as the limit when AvailableSize is 0 or undefined
  • Added logic to handle cases where allocated size exceeds the slot size limit
  • Updated all test cases to reflect the new field name and logic

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/utils/disks/types.ts Replaced TotalSize field with SizeLimit in PreparedVDisk interface
src/utils/disks/prepareDisks.ts Updated prepareVDiskSizeFields to calculate size limit based on SlotSize when available is 0, and passed SlotSize parameter from prepareWhiteboardVDiskData
src/utils/disks/__test__/prepareDisks.test.ts Added comprehensive test cases for the new size limit logic and updated existing tests to use SizeLimit
src/store/reducers/vdisk/utils.ts Switched from whiteboard preparation functions to prepareGroupsVDisk for consistency
src/store/reducers/storage/prepareGroupsDisks.ts Updated to pass SlotSize parameter to prepareVDiskSizeFields
src/store/reducers/storage/__tests__/prepareGroupsDisks.test.ts Updated test expectations to use SizeLimit instead of TotalSize
src/store/reducers/pdisk/utils.ts Added logic to use allocated size as total when it exceeds the size limit for proper visualization
src/store/reducers/pdisk/__tests__/preparePDiskDataResponse.test.ts Added test case for handling exceeded usage scenarios
src/components/VDiskInfo/VDiskInfo.tsx Updated to use SizeLimit instead of calculating from AllocatedSize + AvailableSize
src/components/DiskStateProgressBar/DiskStateProgressBar.tsx Added clamping logic to handle percentage values exceeding 100%

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.

10 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@artemmufazalov artemmufazalov marked this pull request as ready for review October 22, 2025 12:40
StringifiedId?: string;

AvailableSize?: number;
TotalSize?: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this field doesnt exist anymore or we just intentionally choose to ignore it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@greptile could you answer this question?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the code context, the comment is asking about the AllocatedSize field in the PreparedVDisk interface on line 31.

This field still exists - it's being kept intentionally. The PR renamed TotalSize to SizeLimit (line 33), but AllocatedSize remains because it represents the actual allocated/used size of the VDisk, which is still needed.

The change in this PR was specifically:

  • Removed: TotalSize
  • Added: SizeLimit (line 33)

AllocatedSize is a core field that comes from the API (TVDiskStateInfo) and is used throughout the codebase to show how much space is actually used. The PR just adds a new SizeLimit field to properly represent the maximum allowed size (using SlotSize when AvailableSize is 0).

So to answer: the field still exists and is intentionally kept - no changes were made to AllocatedSize in this interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@greptile I mean we removed TotalSize but it seems field from api? Is it correct to remove it totally?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect! Looking at the API type TVDiskStateInfo, there is no TotalSize field in the VDisk API response. The VDisk API only has:

  • AllocatedSize (how much space is used)
  • AvailableSize (how much space is available)

So yes, it's correct to remove TotalSize from PreparedVDisk. The PR actually fixes a conceptual issue:

  1. Before: PreparedVDisk had TotalSize which was computed as AllocatedSize + AvailableSize
  2. Problem: When AvailableSize is 0, this made it look like the VDisk was at 100% capacity, even though the actual slot size limit (from SlotSize field) might be larger
  3. After: Replaced TotalSize with SizeLimit which uses:
    • AllocatedSize + AvailableSize when AvailableSize > 0 (normal case)
    • SlotSize when AvailableSize = 0 (fallback to actual physical limit)

TotalSize was a derived/computed field that didn't exist in the API. The rename to SizeLimit better reflects its purpose and the new logic handles the edge case correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@greptile thx darling, now I understand <3

Copy link
Contributor

Choose a reason for hiding this comment

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

You're welcome! 💙 Glad I could help clarify that TotalSize wasn't an API field and the PR correctly removes it. Feel free to ping me if you have any other questions about the changes!

Copy link
Member Author

Choose a reason for hiding this comment

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

I just replaced TotalSize with SizeLimit, SizeLimit is a better naming for me, because used size can exceed this limit

@artemmufazalov artemmufazalov added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit da03b66 Oct 22, 2025
10 checks passed
@artemmufazalov artemmufazalov deleted the 2993-vdisk-usage branch October 22, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Display VDisk usage based on SlotSize if AvailableSize is 0

3 participants