Add maxInflightMountCalls and volumeAttachLimit args #1706
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package driver | ||
|
|
||
| import ( | ||
| "sync" | ||
|
|
||
| "k8s.io/klog/v2" | ||
| ) | ||
|
|
||
| type InFlightMountTracker struct { | ||
| mux sync.Mutex | ||
| count int64 | ||
| maxCount int64 | ||
| } | ||
|
|
||
| func NewInFlightMountTracker(maxCount int64) *InFlightMountTracker { | ||
| if maxCount <= 0 { | ||
| klog.V(4).InfoS("InFlightMountTracker is disabled") | ||
| return nil | ||
| } | ||
| return &InFlightMountTracker{ | ||
| maxCount: maxCount, | ||
| } | ||
| } | ||
|
|
||
| func (checker *InFlightMountTracker) increment() bool { | ||
| checker.mux.Lock() | ||
| defer checker.mux.Unlock() | ||
|
|
||
| if checker.count >= checker.maxCount { | ||
| return false | ||
| } | ||
|
|
||
| checker.count++ | ||
| return true | ||
| } | ||
|
|
||
| func (checker *InFlightMountTracker) decrement() bool { | ||
| checker.mux.Lock() | ||
| defer checker.mux.Unlock() | ||
| if checker.count == 0 { | ||
| klog.Error("InFlightMountTracker: trying to decrement count when it is already 0") | ||
| return false | ||
| } | ||
|
|
||
| checker.count-- | ||
| return true | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package driver | ||
|
|
||
| import ( | ||
| "sync" | ||
| "testing" | ||
| ) | ||
|
|
||
| func assertEqual[T comparable](t *testing.T, actual, expected T, description string) { | ||
| if expected != actual { | ||
| t.Errorf("%s: expected %v != actual %v", description, expected, actual) | ||
| } | ||
| } | ||
|
|
||
| func TestNewInFlightMountTracker(t *testing.T) { | ||
| checker := NewInFlightMountTracker(5) | ||
| assertEqual(t, checker.maxCount, 5, "Max inflight count") | ||
| assertEqual(t, checker.count, 0, "Inflight count") | ||
|
|
||
| checker = NewInFlightMountTracker(UnsetMaxInflightMountCounts) | ||
| assertEqual(t, checker, nil, "Nil checker for negative max inflight mount counts") | ||
|
|
||
| checker = NewInFlightMountTracker(0) | ||
| assertEqual(t, checker, nil, "Nil checker for zero max inflight mount counts") | ||
| } | ||
|
|
||
| func TestIncrement(t *testing.T) { | ||
| maxFlightCount := int64(2) | ||
| checker := NewInFlightMountTracker(maxFlightCount) | ||
|
|
||
| if !checker.increment() { | ||
| t.Errorf("First increment should succeed with max inflight count=%d", maxFlightCount) | ||
| } | ||
| assertEqual(t, checker.count, 1, "Inflight count after first increment") | ||
|
|
||
| if !checker.increment() { | ||
| t.Errorf("Second increment should succeed with max inflight count=%d", maxFlightCount) | ||
| } | ||
| assertEqual(t, checker.count, 2, "Inflight count after second increment") | ||
|
|
||
| if checker.increment() { | ||
| t.Errorf("Third increment should fail with max inflight count=%d", maxFlightCount) | ||
| } | ||
| assertEqual(t, checker.count, 2, "Inflight count after third increment") | ||
| } | ||
|
|
||
| func TestDecrement(t *testing.T) { | ||
| maxFlightCount := int64(2) | ||
| checker := NewInFlightMountTracker(maxFlightCount) | ||
| checker.increment() | ||
| checker.increment() | ||
|
|
||
| checker.decrement() | ||
| assertEqual(t, checker.count, 1, "Inflight count after first decrement") | ||
|
|
||
| checker.decrement() | ||
| assertEqual(t, checker.count, 0, "Inflight count after second decrement") | ||
|
|
||
| // Should not decrement further when the count is already zero | ||
| checker.decrement() | ||
| assertEqual(t, checker.count, 0, "Inflight count after decrement when count is already zero") | ||
| } | ||
|
|
||
| func TestConcurrency(t *testing.T) { | ||
| // Run multiple iterations to increase chance of catching race conditions | ||
| for i := 0; i < 100; i++ { | ||
| maxFlightCount := int64(500) | ||
| checker := NewInFlightMountTracker(maxFlightCount) | ||
| var wg sync.WaitGroup | ||
| var mu sync.Mutex | ||
|
|
||
| numGoRoutinesForIncrement := 400 | ||
| for range numGoRoutinesForIncrement { | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| checker.increment() | ||
| }() | ||
| } | ||
|
|
||
| numGoRoutinesForDecrement := 350 | ||
| actualDecrements := 0 | ||
| for range numGoRoutinesForDecrement { | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| if checker.decrement() { | ||
| mu.Lock() | ||
| actualDecrements++ | ||
| mu.Unlock() | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| wg.Wait() | ||
| assertEqual(t, checker.count, int64(numGoRoutinesForIncrement-actualDecrements), "inflight count") | ||
| } | ||
| } |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could simplify this by just creating one flag for each? If no is maxInflightCalls or volumeLimit is provided we just use current behavior (not limited at all)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep the
OptInflag primarily for future-proofing. If we later implement default calculations formaxInflightCallsorvolumeLimit, users could setOptIn=truewithout specifying values for parameters likemaxInflightMountCalls.While we could remove the
OptInfeature gate later if we decide not to provide default calculations, adding it back would be more disruptive. From a user perspective, withoutOptIn=true, a configuration withmaxInflightMountCalls=10would be ignored, breaking backward compatibility.