Skip to content

Commit

Permalink
Updates from feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
EddieChoCho committed Jul 2, 2024
1 parent 2665cd4 commit 98ff051
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -311,16 +311,15 @@ public void unlock() {
return;
}
else {
throw new ConcurrentModificationException();
// the lock is no longer owned by current process, the exception should be handle and rollback the execution result
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
}
catch (TransientDataAccessException | TransactionTimedOutException | TransactionSystemException e) {
// try again
}
catch (ConcurrentModificationException e) {
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
throw e;
}
catch (Exception e) {
throw new DataAccessResourceFailureException("Failed to release mutex at " + this.path, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class JdbcLockRegistryDifferentClientTests {
private JdbcLockRegistry registry;

@Autowired
private DefaultLockRepository client;
private LockRepository client;

@Autowired
private ConfigurableApplicationContext context;
Expand All @@ -81,7 +81,6 @@ class JdbcLockRegistryDifferentClientTests {
public void clear() {
this.registry.expireUnusedOlderThan(0);
this.client.close();
this.client.afterPropertiesSet();
this.child = new AnnotationConfigApplicationContext();
this.child.registerBean("childLockRepository", DefaultLockRepository.class, this.dataSource);
this.child.setParent(this.context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

<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 (?, ?, ?, ?)"/>
</bean>

<tx:annotation-driven/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,7 @@ void testTwoThreadsDifferentRegistries() throws Exception {
Thread.currentThread().interrupt();
}
finally {
try {
lock2.unlock();
}
catch (ConcurrentModificationException ignored) {
}

lock2.unlock();
latch3.countDown();
}
});
Expand Down Expand Up @@ -514,7 +509,7 @@ void noTableThrowsExceptionOnStart() {
}

@Test
void testUnlock_lockStatusIsExpired_lockHasBeenAcquiredByAnotherProcess_ConcurrentModificationExceptionWillBeThrown() throws Exception {
void testUnlockAfterLockStatusHasBeenExpiredAndLockHasBeenAcquiredByAnotherProcess() throws Exception {
int ttl = 100;
DefaultLockRepository client1 = new DefaultLockRepository(dataSource);
client1.setApplicationContext(this.context);
Expand All @@ -541,7 +536,7 @@ void testUnlock_lockStatusIsExpired_lockHasBeenAcquiredByAnotherProcess_Concurre
}

@Test
void testUnlock_lockStatusIsExpired_lockDataHasBeenDeleted_ConcurrentModificationExceptionWillBeThrown() throws Exception {
void testUnlockAfterLockStatusHasBeenExpiredAndDeleted() throws Exception {
DefaultLockRepository client = new DefaultLockRepository(dataSource);
client.setApplicationContext(this.context);
client.setTimeToLive(100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void testLock(RedisLockType testRedisLockType) {

@ParameterizedTest
@EnumSource(RedisLockType.class)
void testUnlock_lockStatusIsExpired_ConcurrentModificationExceptionWillBeThrown(RedisLockType testRedisLockType) throws InterruptedException {
void testUnlockAfterLockStatusHasBeenExpired(RedisLockType testRedisLockType) throws InterruptedException {
RedisLockRegistry registry = new RedisLockRegistry(redisConnectionFactory, this.registryKey, 100);
registry.setRedisLockType(testRedisLockType);
Lock lock = registry.obtain("foo");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,4 @@ For example, an insert query for PostgreSQL hint can be configured like this:
lockRepository.setInsertQuery(lockRepository.getInsertQuery() + " ON CONFLICT DO NOTHING");
----

Starting with version 6.4, the `LockRepository.delete()` method return the result of removing ownership of a distributed lock. And the `JdbcLockRegistry.JdbcLock.unlock()` method throws `ConcurrentModificationException` if the ownership of the lock is expired.
2 changes: 2 additions & 0 deletions src/reference/antora/modules/ROOT/pages/redis.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -855,3 +855,5 @@ Default.

The pub-sub is preferred mode - less network chatter between client Redis server, and more performant - the lock is acquired immediately when subscription is notified about unlocking in the other process.
However, the Redis does not support pub-sub in the Master/Replica connections (for example in AWS ElastiCache environment), therefore a busy-spin mode is chosen as a default to make the registry working in any environment.

Starting with version 6.4, instead of throwing `IllegalStateException`, the `RedisLockRegistry.RedisLock.unlock()` method throws `ConcurrentModificationException` if the ownership of the lock is expired.
9 changes: 8 additions & 1 deletion src/reference/antora/modules/ROOT/pages/whats-new.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,14 @@ The `LobHandler` (and respective API) has been deprecated for removal in Spring
Respective option on `JdbcMessageStore` (and similar) have been deprecated as well.
The byte array handling for serialized message is fully deferred to JDBC driver.

The `LockRepository.delete()` method return the result of removing ownership of a distributed lock. And the `JdbcLockRegistry.JdbcLock.unlock()` method throws ConcurrentModificationException if the ownership of the lock is expired.

[[x6.4-zeromq-changes]]
=== ZeroMQ Changes

The outbound component `ZeroMqMessageHandler` (and respective API) can now bind a TCP port instead of connecting to a given URL.
The outbound component `ZeroMqMessageHandler` (and respective API) can now bind a TCP port instead of connecting to a given URL.

[[x6.4-redis-changes]]
=== Redis Changes

Instead of throwing `IllegalStateException`, the `RedisLockRegistry.RedisLock.unlock()` method throws `ConcurrentModificationException` if the ownership of the lock is expired.

0 comments on commit 98ff051

Please sign in to comment.