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

fix: concurrency in Applitools being too high #1997

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

KevinGhadyani-Okta
Copy link
Contributor

https://oktainc.atlassian.net/browse/OKTA-651614

Summary

Applitools had too high of a concurrency for our plan. This has been taken down to only 10 threads to reduce the workload.

Testing & Screenshots

  • I have confirmed this change with my designer and the Odyssey Design Team.

N/A

Sorry, something went wrong.

@@ -26,5 +26,5 @@ module.exports = {
matchLevel: "Strict",
parentBranchName,
showStorybookOutput: true,
testConcurrency: 20,
testConcurrency: 10,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the current issue? what is the optimal number ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The current issue is that we only have 20 concurrencies with Applitools in total and we have gone above it by 100% on multiple occasions recently. To avoid overcharging or forced upgrades at the renewal, we should monitor our usage and keep it below that 20 concurrency limit. It's very unlikely that we have more than 2 Odyssey PRs running VRT at the same time, so 10 could be an optimal number in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know !! ohh ok so the total thread is per account ?! hopefully only max 2 prs runs vrt at same time.
Updated the jira ticket with this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying Ali!

Copy link
Contributor

@rajaguruduraisamy-okta rajaguruduraisamy-okta left a comment

Choose a reason for hiding this comment

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

lgtm

@KevinGhadyani-Okta KevinGhadyani-Okta merged commit 34a3299 into main Oct 10, 2023
@KevinGhadyani-Okta KevinGhadyani-Okta deleted the kg/OKTA-651614 branch October 10, 2023 13:49
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.

None yet

4 participants