Skip to content

(2.12) Add Prioritized to the Consumer Priority Modes #7113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 8, 2025

Conversation

Jarema
Copy link
Member

@Jarema Jarema commented Jul 29, 2025

This adds Prioritized mode to the PriorityGroups, which allows for a low-latency switch between clients depending on priority set.

Refer ADR-42 for more details.

Signed-off-by: Tomasz Pietrek [email protected]

@Jarema Jarema force-pushed the consumer-prioritized branch 9 times, most recently from 8a8298a to 30229a0 Compare July 30, 2025 10:59
@Jarema Jarema marked this pull request as ready for review August 7, 2025 09:44
@Jarema Jarema requested a review from a team as a code owner August 7, 2025 09:44
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple small nitpicks

wr.n--

if wr.n > 0 && wq.n > 1 {
if wq.head.next == nil {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if wq.head.next == nil {
if wr.next == nil {

Comment on lines 3569 to 3599
// Find insertion point - insert before first element with higher priority
var prev *waitingRequest
current := wq.head

for current != nil {
currentPriority := math.MaxInt32
if current.priorityGroup != nil {
currentPriority = current.priorityGroup.Priority
}
// Insert before the first element with higher priority (stable sort)
if currentPriority > priority {
break
}
prev = current
current = current.next
}

// Insert at found position
if prev == nil {
// Insert at head (lowest priority so far)
wr.next = wq.head
wq.head = wr
} else {
// Insert after prev
wr.next = prev.next
prev.next = wr
if wr.next == nil {
// We're the new tail
wq.tail = wr
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This same logic is also used in popAndRequeue. Could perhaps extract this logic and reuse it in both insertSorted and popAndRequeue.

@neilalexander neilalexander changed the title Add Prioritized to the Consumer Priority Modes (2.12) Add Prioritized to the Consumer Priority Modes Aug 7, 2025
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

Overall LGTM, can you please squash down & rebase on top of latest main?

This adds Prioritized mode to the PriorityGroups, which allows
for a low-latency switch between clients depending on priority set.

Refer ADR-42 for more details.

Signed-off-by: Tomasz Pietrek <[email protected]>
@Jarema Jarema force-pushed the consumer-prioritized branch from 98a4c29 to dbeca22 Compare August 8, 2025 13:25
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

LGTM

@neilalexander neilalexander merged commit 3d6c1e6 into main Aug 8, 2025
48 checks passed
@neilalexander neilalexander deleted the consumer-prioritized branch August 8, 2025 13:37
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.

4 participants