[Healthcontroller] Ensure only rooms with cur scheduler version are running #612
Conversation
This file contains 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
The scheduler now accepts a downSurge optional field to control how many rooms at once Maestro will downscale. This enables us to be more agressive in spawning new roms, with maxSurge, while being more conservative when scaling them down
There might be cases in which we have rooms running with multiple scheduler versions: during scheduler switch version. We want to ensure that only rooms for the current version are running and delete the others following the down surge.
gussf
reviewed
Apr 24, 2024
| desiredNumberOfRooms, err := ex.getDesiredNumberOfRooms(ctx, logger, scheduler) | ||
| if err != nil { | ||
| logger.Error("error getting the desired number of rooms", zap.Error(err)) | ||
| return err | ||
| } | ||
| reportDesiredNumberOfRooms(scheduler.Game, scheduler.Name, desiredNumberOfRooms) | ||
|
|
||
| err = ex.ensureDesiredAmountOfInstances(ctx, op, def, scheduler, logger, len(availableRooms), desiredNumberOfRooms) | ||
| err = ex.ensureDesiredAmountOfInstances(ctx, op, def, scheduler, logger, availableRoomsAmount, desiredNumberOfRooms) | ||
| if err != nil { |
There was a problem hiding this comment.
should we rollback the operation to delete the old-version rooms, in case this error happens?
| if err != nil { | ||
| continue | ||
| } | ||
| if room.Version != scheduler.Spec.Version { |
There was a problem hiding this comment.
Can we use < operator instead? It seems to me this could kill new version rooms if it runs at the exact time of the rollout
There was a problem hiding this comment.
actually it doesn't seem possible for this to happen hahaha, I'll wait to see your reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
On
healthcontrollerworker, add a function to check if there are any available rooms running with a previous scheduler version. If so, then we remove those rooms (via the remove operation).The amount of rooms deleted is capped by the
downSurgeof the scheduler