-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
GH-9291: Enhanced unlock() method of JdbcLock to verify successful unlocking #9292
Conversation
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 also requires respective changes in the lock-registry.adoc
, redis.adoc
and whats-new.adoc
.
The change is aimed for new 6.4
version.
Thanks
...integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/LockRepository.java
Show resolved
Hide resolved
...tegration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java
Outdated
Show resolved
Hide resolved
@@ -24,8 +24,6 @@ | |||
|
|||
<bean id="lockClient" class="org.springframework.integration.jdbc.lock.DefaultLockRepository"> | |||
<constructor-arg name="dataSource" ref="dataSource"/> | |||
<property name="insertQuery" | |||
value="INSERT INTO INT_LOCK (REGION, LOCK_KEY, CLIENT_ID, CREATED_DATE) VALUES (?, ?, ?, ?)"/> |
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.
There was a reason to have this in the test: #3866.
Even if it is the same value we have by default, it still covers a respective setter call.
So, please, consider to revert your change.
It is still not relevant to the purpose of this PR.
...tion-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java
Outdated
Show resolved
Hide resolved
...tion-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java
Outdated
Show resolved
Hide resolved
...ration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java
Show resolved
Hide resolved
...tegration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java
Outdated
Show resolved
Hide resolved
...integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/LockRepository.java
Show resolved
Hide resolved
…y successful unlocking Fixes: spring-projects#9291 * Modify `unlock()` method of `JdbcLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown. * Modify `unlock()` method of `RedisLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown. * Maintain test cases
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.
So, all good. Just a couple nit-pics for the doc.
Thanks
src/reference/antora/modules/ROOT/pages/jdbc/lock-registry.adoc
Outdated
Show resolved
Hide resolved
Thank you for your feedback and the good reference! I've just pushed another commit. |
thank you for contribution; looking forward for more! I know, we have with you a |
Here is a Migration Guide page we have talked about before: https://github.com/spring-projects/spring-integration/wiki/Spring-Integration-6.3-to-6.4-Migration-Guide |
Fixes: #9291
unlock()
method ofJdbcLock
: if the lock ownership can not be removed due to data expiration, aConcurrentModificationException
should be thrown.unlock()
method ofRedisLock
: if the lock ownership can not be removed due to data expiration, aConcurrentModificationException
should be thrown.