feat(config): add tmo max reclaim size - #208
Merged
Merged
Conversation
|
|
📝 WalkthroughWalkthroughAdds an optional ChangesTMO reclaim-size configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yaml`:
- Around line 113-117: Update all three CRD schema entries for maxReclaimSize to
add a minimum constraint of 0, while preserving the existing int64 type and
description. This ensures negative values are rejected at the CRD boundary and
matches the *uint64 configuration field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e866f26d-941e-4368-befc-93f0b28d07de
📒 Files selected for processing (3)
config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yamlpkg/apis/config/v1alpha1/tmo.gopkg/apis/config/v1alpha1/zz_generated.deepcopy.go
Comment on lines
+113
to
+117
| maxReclaimSize: | ||
| description: MaxReclaimSize limits the bytes that can | ||
| be reclaimed in one TMO cycle. Zero means unlimited. | ||
| format: int64 | ||
| type: integer |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A5 'maxReclaimSize:' \
config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yamlRepository: kubewharf/katalyst-api
Length of output: 1261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the Go field definitions and any validation markers for maxReclaimSize.
rg -n -A4 -B4 'maxReclaimSize|MaxReclaimSize' api pkg config -g '!**/vendor/**' || true
# Show the exact CRD sections around the three occurrences.
sed -n '108,122p' config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yaml
printf '\n---\n'
sed -n '251,265p' config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yaml
printf '\n---\n'
sed -n '331,345p' config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yamlRepository: kubewharf/katalyst-api
Length of output: 9087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the generated CRD schema source and any kubebuilder validation on the field.
rg -n -A4 -B4 'MaxReclaimSize|maxReclaimSize' api pkg config -g '*.go' -g '*.yaml' || trueRepository: kubewharf/katalyst-api
Length of output: 6515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat > /tmp/uint64_decode_check.go <<'EOF'
package main
import (
"encoding/json"
"fmt"
)
func main() {
var v struct {
MaxReclaimSize *uint64 `json:"maxReclaimSize"`
}
err := json.Unmarshal([]byte(`{"maxReclaimSize":-1}`), &v)
if err != nil {
fmt.Println("ERR:", err)
return
}
if v.MaxReclaimSize == nil {
fmt.Println("OK: nil")
return
}
fmt.Println("OK:", *v.MaxReclaimSize)
}
EOF
go run /tmp/uint64_decode_check.goRepository: kubewharf/katalyst-api
Length of output: 250
Reject negative maxReclaimSize values at the CRD boundary.
maxReclaimSize is a *uint64, so a negative value will fail typed decoding even though the CRD currently accepts it as int64. Add minimum: 0 to all three schema entries.
Proposed schema fix
maxReclaimSize:
format: int64
+ minimum: 0
type: integer📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| maxReclaimSize: | |
| description: MaxReclaimSize limits the bytes that can | |
| be reclaimed in one TMO cycle. Zero means unlimited. | |
| format: int64 | |
| type: integer | |
| maxReclaimSize: | |
| description: MaxReclaimSize limits the bytes that can | |
| be reclaimed in one TMO cycle. Zero means unlimited. | |
| format: int64 | |
| minimum: 0 | |
| type: integer |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yaml`
around lines 113 - 117, Update all three CRD schema entries for maxReclaimSize
to add a minimum constraint of 0, while preserving the existing int64 type and
description. This ensures negative values are rejected at the CRD boundary and
matches the *uint64 configuration field.
liunxaa
force-pushed
the
dev/refactor-tmo
branch
from
July 17, 2026 07:38
9fc1c6e to
4797f56
Compare
liunxaa
force-pushed
the
dev/refactor-tmo
branch
from
July 17, 2026 07:53
4797f56 to
d053664
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/apis/config/v1alpha1/tmo.go`:
- Around line 140-143: Update the MaxReclaimSize field validation in the TMO API
type to reject quantities with a leading negative sign while continuing to allow
zero and positive values. Use the CRD validation marker pattern used by adjacent
fields so the generated CRD enforces this constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e7aae4a1-b545-4fab-be60-448cb6329d36
📒 Files selected for processing (3)
config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yamlpkg/apis/config/v1alpha1/tmo.gopkg/apis/config/v1alpha1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (2)
- config/crd/bases/config.katalyst.kubewharf.io_transparentmemoryoffloadingconfigurations.yaml
- pkg/apis/config/v1alpha1/zz_generated.deepcopy.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer:
Summary by CodeRabbit
maxReclaimSizesetting to control the maximum number of bytes that can be reclaimed in a single TMO cycle.0now means “unlimited”.