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

APS-1787 add seed job to create applications with pending request for placements #2781

Merged

Conversation

davidatkinsuk
Copy link
Contributor

@davidatkinsuk davidatkinsuk commented Jan 9, 2025

Add a seed job to create test applications that either pending submission, or pending matching

This job uses co-routines when creating applications. limitedParallelism is set to 10 to ensure we don’t exceed the size of the JDBC connection pool. Otherwise we start seeing timeouts waiting on connections in the pool. Adding additional parallelism didn’t produce any notable gains, although that my not be the case in other better-sized environments.

@davidatkinsuk davidatkinsuk force-pushed the feature/aps-1787-add-seed-job-to-create-test-applications branch 17 times, most recently from 1146506 to 52d635c Compare January 10, 2025 16:56
@davidatkinsuk davidatkinsuk marked this pull request as ready for review January 10, 2025 17:03

log.info("Creating $count applications for CRN $crn in state $state using creator username $deliusUsername")

runBlocking(Dispatchers.IO.limitedParallelism(10)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Other option to consider is to use CoroutineScope if we dont need to block the thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was playing around with this and i think CoroutineScope only works if being called from a suspend function, which eventually will need a runBlocking somewhere further up the call chain. But this is all quite new to me so there may be a way to do it without that. I did verify that parallelism is occurring with this configuration. My initial attempt didn't specify a dispatcher and that led to the default dispatched being used which just ran one co-routine at a time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree though, we could consider running this without blocking. In this case I think blocking is fine as we're mostly always exhausting the parallelism on the dispatcher so not much else could happen regardless (e.g. process another CSV row)

Move logic into service in preparation for reuse
This will be used to support load testing
This commit updates the CAS1 Create Test Applications Seed Job to allow applications to be seeded as assessed.

This job also uses co-routines when creating applications. limitedParallelism is set to 10 to ensure we don’t exceed the size of the JDBC connection pool. Otherwise we start seeing timeouts waiting on connections in the pool. Adding additional parallelism didn’t produce any notable gains, although that my not be the case in other better-sized environments.
@davidatkinsuk davidatkinsuk force-pushed the feature/aps-1787-add-seed-job-to-create-test-applications branch from 52d635c to d2205ac Compare January 13, 2025 11:17
@davidatkinsuk davidatkinsuk merged commit 49f3405 into main Jan 13, 2025
7 checks passed
@davidatkinsuk davidatkinsuk deleted the feature/aps-1787-add-seed-job-to-create-test-applications branch January 13, 2025 11:27
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.

2 participants