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

Improve LockService functionality and flexibility #1541

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Nov 16, 2020

Implementation of #1453

 - Refactored code to fit with new Plugin system
 - Cleaned up duplicate/uneeded code
 - Introduced explicit NoLockService and liquibase.changeLogLockEnabled setting
 - Refactoring to better isolate changeloglock table functionality in the StandardLockService class
 - Misc lock service code cleanup
public void waitForLock() throws LockException {

boolean locked = false;
long timeToGiveUp = new Date().getTime() + (getChangeLogLockWaitTime() * 1000 * 60);
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be worthwhile switching to Duration types instead of numeric types for wait and recheck times.

# Conflicts:
#	liquibase-core/src/main/java/liquibase/lockservice/LockServiceFactory.java
#	liquibase-integration-tests/src/test/java/liquibase/statementexecute/CreateDatabaseChangeLogLockTableExecuteTest.java
@nvoxland
Copy link
Contributor Author

nvoxland commented Jan 8, 2021

Realized an issue with the heartbeat strategy: Liquibase currently makes just one connection but the heartbeat thread needs a separate connection for transaction reasons. We can't have the heartbeat committing the normal liquibase transaction, and we can't have it waiting for a commit from the normal connection.

We'll have to build in support for opening separate connections before we can make progress on this.

@codecov
Copy link

codecov bot commented Jan 15, 2021

Codecov Report

Merging #1541 (0eb8df0) into master (43ed0e5) will increase coverage by 1.59%.
The diff coverage is 41.43%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1541      +/-   ##
============================================
+ Coverage     48.01%   49.60%   +1.59%     
+ Complexity     7903     7867      -36     
============================================
  Files           793      757      -36     
  Lines         38608    37042    -1566     
  Branches       6904     6703     -201     
============================================
- Hits          18536    18374     -162     
+ Misses        17645    16236    -1409     
- Partials       2427     2432       +5     
Impacted Files Coverage Δ Complexity Δ
...uibase-core/src/main/java/liquibase/Liquibase.java 39.88% <ø> (-1.79%) 57.00 <0.00> (-5.00)
...a/liquibase/change/core/CreateProcedureChange.java 29.16% <0.00%> (+0.24%) 24.00 <0.00> (ø)
...va/liquibase/change/core/LoadUpdateDataChange.java 27.27% <ø> (+2.27%) 9.00 <0.00> (+1.00)
...liquibase/command/core/DiffToChangeLogCommand.java 7.40% <0.00%> (+0.26%) 2.00 <0.00> (ø)
...n/java/liquibase/command/core/SnapshotCommand.java 3.77% <ø> (+0.06%) 2.00 <0.00> (ø)
...in/java/liquibase/database/core/MySQLDatabase.java 32.03% <0.00%> (+2.87%) 28.00 <0.00> (ø)
...java/liquibase/database/core/PostgresDatabase.java 44.68% <0.00%> (+0.70%) 37.00 <0.00> (+1.00)
...quibase/diff/output/changelog/DiffToChangeLog.java 34.12% <0.00%> (-0.69%) 31.00 <0.00> (ø)
...e/src/main/java/liquibase/hub/core/HttpClient.java 15.09% <0.00%> (-0.30%) 5.00 <0.00> (ø)
...main/java/liquibase/hub/core/OnlineHubService.java 27.30% <0.00%> (-0.31%) 18.00 <0.00> (ø)
... and 189 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66f82cd...2aeef29. Read the comment docs.

@kataggart kataggart added this to To Do in Conditioning++ via automation May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants