Skip to content

Commit 7185020

Browse files
committed
docs: Update comments to align with FlowController
This commit updates documentation and code comments across various framework components to align with the concepts and architecture introduced by the `FlowController`. Key changes include: - FCFS Policy: Clarified the distinction between "logical" and "physical" enqueue time and the behavioral trade-offs when pairing with different queue capabilities. - ListQueue: Expanded the documentation to explain its role as a high-performance, approximate FCFS queue in the context of the `FlowController`'s retry mechanics. - Request Types: Refined the comments for `QueueItemAccessor` to be more precise about the meaning of `EnqueueTime`.
1 parent 945ad9c commit 7185020

File tree

5 files changed

+77
-17
lines changed

5 files changed

+77
-17
lines changed

pkg/epp/flowcontrol/framework/plugins/policies/intraflow/dispatch/fcfs/fcfs.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,32 @@ import (
2626
)
2727

2828
// FCFSPolicyName is the name of the FCFS policy implementation.
29+
//
30+
// This policy implements a First-Come, First-Served (FCFS) strategy by selecting the item with the earliest logical
31+
// enqueue time.
32+
//
33+
// # Behavior and Queue Pairing
34+
//
35+
// The behavioral guarantees of this policy are critically dependent on the capabilities of the `framework.SafeQueue` it
36+
// is paired with. The system distinguishes between:
37+
// - "Logical Enqueue Time": The timestamp when a request first arrives at the `controller.FlowController`.
38+
// - "Physical Enqueue Time": The timestamp when a request is added to a specific shard's queue, which happens later.
39+
//
40+
// This policy's behavior changes accordingly:
41+
// - Paired with a `CapabilityPriorityConfigurable` queue, it provides strict FCFS ordering based on logical enqueue
42+
// time, aligning with this policy's vended `framework.ItemComparator`.
43+
// This configuration ensures that requests are processed in the order they arrived at the controller, providing the
44+
// most intuitive behavior.
45+
// - Paired with a `CapabilityFIFO` queue, it provides approximate FCFS ordering based on physical arrival order at
46+
// the `framework.SafeQueue`.
47+
// This configuration offers higher performance at the cost of strict logical-time ordering, as the
48+
// `controller.FlowController`'s "bounce-and-retry" mechanic for Draining shards means a bounced request may be
49+
// processed after a request that logically darrived later.
50+
//
51+
// Given that true end-to-end ordering is non-deterministic in a distributd system, this policy defaults to pairing with
52+
// a CapabilityFIFO` queue (like "ListQueue") to prioritize performance and high throughput. For users who require the
53+
// strictest possible logical-time ordering that this layer can provide, explicitly pairing this policy with a
54+
// `CapabilityPriorityConfigurable` queue is recommended.
2955
const FCFSPolicyName = "FCFS"
3056

3157
func init() {
@@ -35,7 +61,9 @@ func init() {
3561
})
3662
}
3763

38-
// fcfs (First-Come, First-Served) implements the `framework.IntraFlowDispatchPolicy` interface.
64+
// fcfs is the internal implementation of the FCFS policy.
65+
// See the documentation for the exported `FCFSPolicyName` constant for detailed user-facing information about its
66+
// behavior.
3967
type fcfs struct {
4068
comparator framework.ItemComparator
4169
}
@@ -70,9 +98,10 @@ func (p *fcfs) Comparator() framework.ItemComparator {
7098
return p.comparator
7199
}
72100

73-
// RequiredQueueCapabilities specifies that this policy needs a queue that supports FIFO operations.
101+
// RequiredQueueCapabilities returns an empty slice, indicating that this policy can operate with any queue.
102+
// See the `FCFSPolicyName` constant's documentation for details on the behavioral trade-offs.
74103
func (p *fcfs) RequiredQueueCapabilities() []framework.QueueCapability {
75-
return []framework.QueueCapability{framework.CapabilityFIFO}
104+
return []framework.QueueCapability{}
76105
}
77106

78107
// --- enqueueTimeComparator ---

pkg/epp/flowcontrol/framework/plugins/policies/intraflow/dispatch/fcfs/fcfs_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ func TestFCFS_RequiredQueueCapabilities(t *testing.T) {
4141
t.Parallel()
4242
policy := newFCFS()
4343
caps := policy.RequiredQueueCapabilities()
44-
require.Len(t, caps, 1, "RequiredQueueCapabilities should return one capability")
45-
assert.Equal(t, framework.CapabilityFIFO, caps[0], "Required capability should be FIFO")
44+
require.Empty(t, caps, "No required capabilities should be returned")
4645
}
4746

4847
func TestFCFS_SelectItem(t *testing.T) {

pkg/epp/flowcontrol/framework/plugins/queue/listqueue/listqueue.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
// Package listqueue provides a simple, concurrent-safe queue implementation using a standard library
18-
// `container/list.List` as the underlying data structure for FIFO (First-In, First-Out) behavior.
17+
// Package listqueue provides a high-performance, concurrent-safe FIFO (First-In, First-Out) implementation of
18+
// implementation of the `framework.SafeQueue` based on the standard library's `container/list`.
1919
package listqueue
2020

2121
import (
@@ -28,7 +28,28 @@ import (
2828
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/flowcontrol/types"
2929
)
3030

31-
// ListQueueName is the name of the list queue implementation.
31+
// ListQueueName is the name of the list-based queue implementation.
32+
//
33+
// This queue provides a high-performance, low-overhead implementation based on a standard `container/list`.
34+
// It advertises the `CapabilityFIFO`.
35+
//
36+
// # Behavioral Guarantees
37+
//
38+
// The core guarantee of this queue is strict physical First-In, First-Out (FIFO) ordering. It processes items in the
39+
// exact order they are added to the queue on a specific shard.
40+
//
41+
// # Performance and Trade-offs
42+
//
43+
// Because the physical insertion order may not match a request's logical arrival time (due to the
44+
// `controller.FlowController`'s internal "bounce-and-retry" mechanic), this queue provides an*approximate FCFS behavior
45+
// from a system-wide perspective.
46+
//
47+
// Given that true end-to-end ordering is non-deterministic in a distributed system, this high-performance queue is the
48+
// recommended default for most FCFS-like policies. It prioritizes throughput and efficiency, which aligns with the
49+
// primary goal of the Flow Control system.
50+
//
51+
// For workloads that require the strictest possible logical-time ordering this layer can provide, explicitly using a
52+
// queue that supports `CapabilityPriorityConfigurable` is the appropriate choice.
3253
const ListQueueName = "ListQueue"
3354

3455
func init() {
@@ -39,8 +60,8 @@ func init() {
3960
})
4061
}
4162

42-
// listQueue implements the `framework.SafeQueue` interface using a standard `container/list.List` for FIFO behavior.
43-
// This implementation is concurrent-safe.
63+
// listQueue is the internal implementation of the ListQueue.
64+
// See the documentation for the exported `ListQueueName` constant for detailed user-facing information.
4465
type listQueue struct {
4566
requests *list.List
4667
byteSize atomic.Uint64

pkg/epp/flowcontrol/registry/config_test.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,16 +211,26 @@ func TestConfig_NewConfig(t *testing.T) {
211211
name: "ShouldError_WhenQueueFactoryFails",
212212
input: Config{
213213
PriorityBands: []PriorityBandConfig{{
214-
Priority: 1,
215-
PriorityName: "High",
216-
Queue: queue.RegisteredQueueName("failing-queue"),
214+
Priority: 1,
215+
PriorityName: "High",
216+
Queue: queue.RegisteredQueueName("failing-queue"),
217+
IntraFlowDispatchPolicy: intra.RegisteredPolicyName("policy-with-req"),
217218
}},
218219
},
219220
expectErr: true,
220-
opts: []configOption{withQueueFactory(
221-
func(_ queue.RegisteredQueueName, _ framework.ItemComparator) (framework.SafeQueue, error) {
221+
opts: []configOption{
222+
withIntraFlowDispatchPolicyFactory( // Forces queue instance creation for validating capabilities.
223+
func(name intra.RegisteredPolicyName) (framework.IntraFlowDispatchPolicy, error) {
224+
return &mocks.MockIntraFlowDispatchPolicy{
225+
NameV: string(name),
226+
RequiredQueueCapabilitiesV: []framework.QueueCapability{"required-capability"},
227+
}, nil
228+
},
229+
),
230+
withQueueFactory(func(_ queue.RegisteredQueueName, _ framework.ItemComparator) (framework.SafeQueue, error) {
222231
return nil, errors.New("queue creation failed")
223-
})},
232+
}),
233+
},
224234
},
225235
{
226236
name: "ShouldError_WhenPolicyFactoryFails",

pkg/epp/flowcontrol/types/request.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ type QueueItemAccessor interface {
9292
OriginalRequest() FlowControlRequest
9393

9494
// EnqueueTime is the timestamp when the item was logically accepted by the `controller.FlowController` for queuing
95-
// (i.e., when `controller.FlowController.EnqueueAndWait()` was called).
95+
// (i.e., when `controller.FlowController.EnqueueAndWait()` was called). It does not reflect the time the request
96+
// landed in a `framework.SafeQueue` instance.
9697
EnqueueTime() time.Time
9798

9899
// EffectiveTTL is the actual Time-To-Live assigned to this item by the `controller.FlowController`, taking into

0 commit comments

Comments
 (0)