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.

Sign up for GitHub

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

Jump to bottom

Set app_replicas to 2 for production #1848

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Set app_replicas to 2 for production #1848

merged 1 commit into from
Dec 6, 2023

Conversation

johnake
Copy link
Contributor

@johnake johnake commented Dec 6, 2023

No description provided.

@johnake johnake requested a review from a team as a code owner December 6, 2023 14:09
@johnake
Copy link
Contributor Author

johnake commented Dec 6, 2023

Increase Apply for QTS replicas

thomasleese
thomasleese reviewed Dec 6, 2023
View reviewed changes
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

Looks good, could we also add a description to the commit explaining why we're making this change?

terraform/application/application.tf Outdated Show resolved Hide resolved
@johnake johnake force-pushed the 811-increase-apply-for-qts-replicas_ branch from afbf672 to a1f1f04 Compare December 6, 2023 15:24
thomasleese
thomasleese previously approved these changes Dec 6, 2023
View reviewed changes
@thomasleese thomasleese dismissed their stale review December 6, 2023 15:37

Didn't mean to approve.

thomasleese
thomasleese reviewed Dec 6, 2023
View reviewed changes
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

Code looks great, I would like to see a description in the commit message of why we're making this change.

I also wonder if there's anything we need to be careful of when deploying this as obviously we can't test it in any non-production environments? Should this change also be applied to pre-production?

@johnake
Set app_replicas to 2 for production
7044ae9 
During the last AKS cluster upgrade, the app had a small outage. We suspect
it's because it's deployed with only 1 replica, hence we increasing the number
of repicas to 2.
@johnake johnake force-pushed the 811-increase-apply-for-qts-replicas_ branch from a1f1f04 to 7044ae9 Compare December 6, 2023 16:08
@saliceti
Copy link
Member

saliceti commented Dec 6, 2023

Code looks great, I would like to see a description in the commit message of why we're making this change.

I also wonder if there's anything we need to be careful of when deploying this as obviously we can't test it in any non-production environments? Should this change also be applied to pre-production?

On the infrastructure side, it's a very common configuration so there is no risk here. The only issue would be if the app is stateful and keeps something in the container as requests will now be spread across 2 instances. But I think we would have seen issue before, as containers are restarted from time to time which would remove the state.

@johnake johnake merged commit 48f3088 into main Dec 6, 2023
12 checks passed
@johnake johnake deleted the 811-increase-apply-for-qts-replicas_ branch December 6, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saliceti saliceti saliceti left review comments

@temitope777 temitope777 temitope777 approved these changes

@thomasleese thomasleese thomasleese approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@johnake @saliceti @thomasleese @temitope777