Skip to content
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

feat: Round Robin Weights #15558

Merged
merged 85 commits into from
Aug 15, 2024
Merged

feat: Round Robin Weights #15558

merged 85 commits into from
Aug 15, 2024

Conversation

CarinaWolli
Copy link
Member

@CarinaWolli CarinaWolli commented Jun 25, 2024

What does this PR do?

Adds weights to round robin alogirthm. When weights are enabled each host has 100% as the default weight. That means that ideally, all hosts end up with the same amount of bookings. If weights are enabled, priorities are only taken into account when more than one host is selected by the getUsersBasedOnWeights function.

To make sure that adding new host, when other hosts already have bookings doesn't result in the new host being assigned to all bookings we use weightAdjustments.

WeightAdjustment (nr of 'fake' bookings) = ((nr of all bookings of current RR hosts + sum of all weight adjustments of current RR hosts) / sum of all current RR hosts weights) * new host's weight

Weights enabled:
Screenshot 2024-06-25 at 4 30 13 PM

Weights disabled:
Screenshot 2024-06-25 at 4 30 29 PM

Set weight:
Screenshot 2024-06-25 at 4 31 00 PM

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected)
  • I have added a Docs Add Round Robin weights docs#101 --> still need to finish that
  • I have added or modified automated tests that prove my fix is effective or that my feature works (PRs might be rejected if logical changes are not properly tested)

How should this be tested?

  • Create RR event type and enable weights
  • test different weights and priority settings
  • test adding new hosts when bookings already exists and see if weightAdjustment was correctly saved

@keithwillcode keithwillcode added consumer core area: core, team members only labels Jun 25, 2024
Copy link

vercel bot commented Jun 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2024 1:32pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Aug 15, 2024 1:32pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Aug 15, 2024 1:32pm

Copy link
Contributor

github-actions bot commented Aug 15, 2024

E2E results are ready!

Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

I would've liked to see more test cases, but we can add them after; looks good from my end 🚀

@@ -23,10 +39,11 @@ async function leastRecentlyBookedUser<T extends Pick<User, "id" | "email">>({
createdAt: true,
},
where: {
eventTypeId,
eventTypeId: eventType.id,
status: BookingStatus.ACCEPTED,
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds sensible! Yeah happy with that.

@emrysal emrysal merged commit cd311f0 into main Aug 15, 2024
37 of 38 checks passed
@emrysal emrysal deleted the feat/round-robin-weights branch August 15, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consumer core area: core, team members only ✨ feature New feature or request High priority Created by Linear-GitHub Sync high-risk Requires approval by Foundation team Medium priority Created by Linear-GitHub Sync ❗️ migrations contains migration files ready-for-e2e teams area: teams, round robin, collective, managed event-types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3869] Round Robin Weights
4 participants