Skip to content

Use round-robin load balancing strategy for usage service#7920

Draft
jdolle wants to merge 4 commits intomainfrom
load-balance-strategy
Draft

Use round-robin load balancing strategy for usage service#7920
jdolle wants to merge 4 commits intomainfrom
load-balance-strategy

Conversation

@jdolle
Copy link
Copy Markdown
Collaborator

@jdolle jdolle commented Mar 27, 2026

Background

Description

Checklist

  • Input validation
  • Output encoding
  • Authentication management
  • Session management
  • Access control
  • Cryptographic practices
  • Error handling and logging
  • Data protection
  • Communication security
  • System configuration
  • Database security
  • File management
  • Memory management
  • Testing

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a configurable loadBalanceStrategy for proxy routes, defaulting to Cookie. The graphql-api and usage services are updated to use the RoundRobin strategy. Feedback suggests using JSDoc to document the default value for the new property.

@jdolle jdolle requested a review from dotansimha March 27, 2026 00:38
@jdolle jdolle self-assigned this Mar 27, 2026
@jdolle jdolle changed the title Use round-robin for graphql and usage apis Use round-robin load balancing strategy for usage service Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

🐋 This PR was built and pushed to the following Docker images:

Targets: build

Platforms: linux/amd64

Image Tag: 320f27c73194f3f9617fc5d9029d04dbdc8146de

path: '/usage',
service: usage.service,
retriable: true,
loadBalanceStrategy: 'RoundRobin',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would assume that the usage SDK is not sending any cookies in the first place since it is not a browser client.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thus, I am doubting whether this change would change anything. It might be worth exploring what would be the fallback load balancing strategy in case of a missing cookie.

Comment on lines 162 to +163
loadBalancerPolicy: {
strategy: 'Cookie',
strategy: route.loadBalanceStrategy ?? 'Cookie',
Copy link
Copy Markdown
Contributor

@n1ru4l n1ru4l Mar 27, 2026

Choose a reason for hiding this comment

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

In any case, all of our services are stateless, so I think we can remove Cookie as the default. (unrelated to the problem we are trying to be solving here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants