-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Added ThreadLocalScopeManager #3240
Conversation
The failing pro-tests and test-harness tests are unrelated to this change |
Made SqlGeneratorFactory getGenerators() synchronized as it sometimes has concurrency issues
Made SqlGeneratorFactory getGenerators() synchronized as it sometimes has concurrency issues
9e7dfcf
to
a44bafe
Compare
liquibase-core/src/main/java/liquibase/ThreadLocalScopeManager.java
Outdated
Show resolved
Hide resolved
Hi @nvoxland, this looks good! Now there is hope :-) Tested on https://github.com/bvremmeinfor/liquibase-threading-demo/tree/liquibase_pr_3240_test branch using snapshot build of #3240 (with public ThreadLocalScopeManager constructor). Could simplify code in test project and use official implementations. Nice! Repeated stress tests performed - 100% success. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds a new class, ThreadLocalScopeManager, to allow Liquibase integrations to leverage multiple threads. The default behavior for CLI and any integrations not opting to use the new ThreadLocalScopeManager class remains the same, i.e., uses a singleton created by SingletonScopeManager . The new test class FileSystemResourceAccessorTest is an excellent overview of how the new feature works.
APPROVED
- Note: We had some build issues that prevented us from running all functional tests in a branch. The plan is to merge this to liquibase/liquibase master & re-run the tests. @nvoxland and I will be watching to see if there are adverse effects due to the merge. If so, we will fix or revert (depending on severity).
Package across-test.properties, which will be used by TestDataSourceConfigurer, unless you have a $HOME/dev-configs/across-test.properties file. Upgrade liquibase and use ThreadLocalScopeManager to handle a concurrency issue that occurs on the Amazon runners: liquibase/liquibase#3240 Use GitLab CI/CD services for the databases instead of test containers: this is an order of magnitude faster, especially for Oracle, because it does not start and stop the entire database server for each new spring test context.
Impact
Description
By default, Liquibase uses SingletonScopeManager which is designed to be a single scope regardless of threads going on in the application. That works fine for single-threaded applications and use cases in a multi-threaded application, but for other multi-threaded applications it does not work.
This adds a
liquibase.ThreadLocalScopeManager
class which custom integrations can use to replace the default SingletonScopeManager with a call toScope.setScopeManager(new ThreadLocalScopeManager())
.Things to be aware of
Things to worry about
Additional Context
Fixes #2018
Fixes #2248