Skip to content

Commit 06529ab

Browse files
mhyeon-leeschauder
mhyeon-lee
authored and
schauder
committed
DATAJDBC-488 - Avoid deadlocks by first accessing the root entity.
The previous process of deleting referencing entities, updating the root and then inserting referencing entities hat the potential of causing deadlocks. When one process didn't obtain a lock on delete because there wasn't anything to delete root and referencing entities got locks in opposite order, which is a classical cause for deadlocks. Original pull request: #191.
1 parent 95428b7 commit 06529ab

File tree

5 files changed

+161
-13
lines changed

5 files changed

+161
-13
lines changed

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryConcurrencyIntegrationTests.java

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
package org.springframework.data.jdbc.repository;
2+
3+
import junit.framework.AssertionFailedError;
4+
import lombok.AllArgsConstructor;
5+
import lombok.Getter;
6+
import lombok.With;
7+
import org.junit.ClassRule;
8+
import org.junit.Rule;
9+
import org.junit.Test;
10+
import org.springframework.beans.factory.annotation.Autowired;
11+
import org.springframework.context.annotation.Bean;
12+
import org.springframework.context.annotation.Configuration;
13+
import org.springframework.context.annotation.Import;
14+
import org.springframework.data.annotation.Id;
15+
import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory;
16+
import org.springframework.data.jdbc.testing.TestConfiguration;
17+
import org.springframework.data.repository.CrudRepository;
18+
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
19+
import org.springframework.test.context.ActiveProfiles;
20+
import org.springframework.test.context.ContextConfiguration;
21+
import org.springframework.test.context.junit4.rules.SpringClassRule;
22+
import org.springframework.test.context.junit4.rules.SpringMethodRule;
23+
import org.springframework.transaction.PlatformTransactionManager;
24+
import org.springframework.transaction.support.TransactionTemplate;
25+
26+
import java.util.ArrayList;
27+
import java.util.Arrays;
28+
import java.util.List;
29+
import java.util.concurrent.CopyOnWriteArrayList;
30+
import java.util.concurrent.CountDownLatch;
31+
32+
import static org.assertj.core.api.Assertions.assertThat;
33+
34+
/**
35+
* @author Myeonghyeon Lee
36+
*/
37+
@ContextConfiguration
38+
@ActiveProfiles("mysql")
39+
public class JdbcRepositoryConcurrencyIntegrationTests {
40+
@Configuration
41+
@Import(TestConfiguration.class)
42+
static class Config {
43+
44+
@Autowired JdbcRepositoryFactory factory;
45+
46+
@Bean
47+
Class<?> testClass() {
48+
return JdbcRepositoryConcurrencyIntegrationTests.class;
49+
}
50+
51+
@Bean
52+
DummyEntityRepository dummyEntityRepository() {
53+
return factory.getRepository(DummyEntityRepository.class);
54+
}
55+
}
56+
57+
@ClassRule
58+
public static final SpringClassRule classRule = new SpringClassRule();
59+
@Rule
60+
public SpringMethodRule methodRule = new SpringMethodRule();
61+
62+
@Autowired
63+
NamedParameterJdbcTemplate template;
64+
@Autowired
65+
DummyEntityRepository repository;
66+
@Autowired
67+
PlatformTransactionManager transactionManager;
68+
69+
@Test // DATAJDBC-488
70+
public void updateConcurrencyWithEmptyReferences() throws Exception {
71+
DummyEntity entity = createDummyEntity();
72+
entity = repository.save(entity);
73+
74+
assertThat(entity.getId()).isNotNull();
75+
76+
List<DummyEntity> concurrencyEntities = new ArrayList<>();
77+
Element element1 = new Element(null, 1L);
78+
Element element2 = new Element(null, 2L);
79+
80+
for (int i = 0; i < 100; i++) {
81+
List<Element> newContent = Arrays.asList(
82+
element1.withContent(element1.content + i + 2),
83+
element2.withContent(element2.content + i + 2)
84+
);
85+
86+
concurrencyEntities.add(entity
87+
.withName(entity.getName() + i)
88+
.withContent(newContent));
89+
}
90+
91+
TransactionTemplate transactionTemplate = new TransactionTemplate(this.transactionManager);
92+
93+
List<Exception> exceptions = new CopyOnWriteArrayList<>();
94+
CountDownLatch countDownLatch = new CountDownLatch(concurrencyEntities.size());
95+
concurrencyEntities.stream()
96+
.map(e -> new Thread(() -> {
97+
countDownLatch.countDown();
98+
try {
99+
transactionTemplate.execute(status -> repository.save(e));
100+
} catch (Exception ex) {
101+
exceptions.add(ex);
102+
}
103+
}))
104+
.forEach(Thread::start);
105+
106+
countDownLatch.await();
107+
108+
Thread.sleep(1000);
109+
DummyEntity reloaded = repository.findById(entity.id).orElseThrow(AssertionFailedError::new);
110+
assertThat(reloaded.content).hasSize(2);
111+
assertThat(exceptions).isEmpty();
112+
}
113+
114+
private static DummyEntity createDummyEntity() {
115+
return new DummyEntity(null, "Entity Name", new ArrayList<>());
116+
}
117+
118+
interface DummyEntityRepository extends CrudRepository<DummyEntity, Long> {
119+
}
120+
121+
@Getter
122+
@AllArgsConstructor
123+
static class DummyEntity {
124+
125+
@Id
126+
private Long id;
127+
@With
128+
String name;
129+
@With
130+
final List<Element> content;
131+
132+
}
133+
134+
@AllArgsConstructor
135+
static class Element {
136+
137+
@Id private Long id;
138+
@With final Long content;
139+
}
140+
}

‎spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mysql.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
CREATE TABLE dummy_entity ( id BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100));
2+
CREATE TABLE element (id BIGINT AUTO_INCREMENT PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT);

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
* @author Jens Schauder
3838
* @author Bastian Wilhelm
3939
* @author Mark Paluch
40+
* @author Myeonghyeon Lee
4041
*/
4142
class WritingContext {
4243

@@ -73,14 +74,17 @@ List<DbAction<?>> insert() {
7374

7475
/**
7576
* Leaves out the isNew check as defined in #DATAJDBC-282
77+
* Possible Deadlocks in Execution Order in #DATAJDBC-488
7678
*
7779
* @return List of {@link DbAction}s
7880
* @see <a href="https://jira.spring.io/browse/DATAJDBC-282">DAJDBC-282</a>
81+
* @see <a href="https://jira.spring.io/browse/DATAJDBC-488">DAJDBC-488</a>
7982
*/
8083
List<DbAction<?>> update() {
8184

82-
List<DbAction<?>> actions = new ArrayList<>(deleteReferenced());
85+
List<DbAction<?>> actions = new ArrayList<>();
8386
actions.add(setRootAction(new DbAction.UpdateRoot<>(entity)));
87+
actions.addAll(deleteReferenced());
8488
actions.addAll(insertReferenced());
8589
return actions;
8690
}
@@ -94,8 +98,8 @@ List<DbAction<?>> save() {
9498
actions.addAll(insertReferenced());
9599
} else {
96100

97-
actions.addAll(deleteReferenced());
98101
actions.add(setRootAction(new DbAction.UpdateRoot<>(entity)));
102+
actions.addAll(deleteReferenced());
99103
actions.addAll(insertReferenced());
100104
}
101105

‎spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityUpdateWriterUnitTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
* Unit tests for the {@link RelationalEntityUpdateWriter}
3131
*
3232
* @author Thomas Lang
33+
* @author Myeonghyeon Lee
3334
*/
3435
@RunWith(MockitoJUnitRunner.class)
3536
public class RelationalEntityUpdateWriterUnitTests {
@@ -51,8 +52,8 @@ public void existingEntityGetsConvertedToDeletePlusUpdate() {
5152
.extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType,
5253
DbActionTestSupport::isWithDependsOn) //
5354
.containsExactly( //
54-
tuple(DbAction.Delete.class, Element.class, "other", null, false), //
55-
tuple(DbAction.UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false) //
55+
tuple(DbAction.UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), //
56+
tuple(DbAction.Delete.class, Element.class, "other", null, false) //
5657
);
5758
}
5859

‎spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
* @author Jens Schauder
5050
* @author Bastian Wilhelm
5151
* @author Mark Paluch
52+
* @author Myeonghyeon Lee
5253
*/
5354
@RunWith(MockitoJUnitRunner.class)
5455
public class RelationalEntityWriterUnitTests {
@@ -156,9 +157,9 @@ public void existingEntityGetsConvertedToDeletePlusUpdate() {
156157
DbActionTestSupport::extractPath, //
157158
DbActionTestSupport::actualEntityType, //
158159
DbActionTestSupport::isWithDependsOn) //
159-
.containsExactly( //
160-
tuple(Delete.class, Element.class, "other", null, false), //
161-
tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false) //
160+
.containsExactly(
161+
tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), //
162+
tuple(Delete.class, Element.class, "other", null, false) //
162163
);
163164
}
164165

@@ -180,8 +181,8 @@ public void newReferenceTriggersDeletePlusInsert() {
180181
DbActionTestSupport::actualEntityType, //
181182
DbActionTestSupport::isWithDependsOn) //
182183
.containsExactly( //
183-
tuple(Delete.class, Element.class, "other", null, false), //
184184
tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), //
185+
tuple(Delete.class, Element.class, "other", null, false), //
185186
tuple(Insert.class, Element.class, "other", Element.class, true) //
186187
);
187188
}
@@ -291,9 +292,9 @@ public void cascadingReferencesTriggerCascadingActionsForUpdate() {
291292
DbActionTestSupport::actualEntityType, //
292293
DbActionTestSupport::isWithDependsOn) //
293294
.containsExactly( //
295+
tuple(UpdateRoot.class, CascadingReferenceEntity.class, "", CascadingReferenceEntity.class, false), //
294296
tuple(Delete.class, Element.class, "other.element", null, false),
295297
tuple(Delete.class, CascadingReferenceMiddleElement.class, "other", null, false),
296-
tuple(UpdateRoot.class, CascadingReferenceEntity.class, "", CascadingReferenceEntity.class, false), //
297298
tuple(Insert.class, CascadingReferenceMiddleElement.class, "other", CascadingReferenceMiddleElement.class,
298299
true), //
299300
tuple(Insert.class, CascadingReferenceMiddleElement.class, "other", CascadingReferenceMiddleElement.class,
@@ -447,8 +448,8 @@ public void mapTriggersDeletePlusInsert() {
447448
this::getMapKey, //
448449
DbActionTestSupport::extractPath) //
449450
.containsExactly( //
450-
tuple(Delete.class, Element.class, null, "elements"), //
451451
tuple(UpdateRoot.class, MapContainer.class, null, ""), //
452+
tuple(Delete.class, Element.class, null, "elements"), //
452453
tuple(Insert.class, Element.class, "one", "elements") //
453454
);
454455
}
@@ -469,8 +470,8 @@ public void listTriggersDeletePlusInsert() {
469470
this::getListKey, //
470471
DbActionTestSupport::extractPath) //
471472
.containsExactly( //
472-
tuple(Delete.class, Element.class, null, "elements"), //
473473
tuple(UpdateRoot.class, ListContainer.class, null, ""), //
474+
tuple(Delete.class, Element.class, null, "elements"), //
474475
tuple(Insert.class, Element.class, 0, "elements") //
475476
);
476477
}
@@ -494,9 +495,9 @@ public void multiLevelQualifiedReferencesWithId() {
494495
a -> getQualifier(a, listMapContainerElements), //
495496
DbActionTestSupport::extractPath) //
496497
.containsExactly( //
498+
tuple(UpdateRoot.class, ListMapContainer.class, null, null, ""), //
497499
tuple(Delete.class, Element.class, null, null, "maps.elements"), //
498500
tuple(Delete.class, MapContainer.class, null, null, "maps"), //
499-
tuple(UpdateRoot.class, ListMapContainer.class, null, null, ""), //
500501
tuple(Insert.class, MapContainer.class, 0, null, "maps"), //
501502
tuple(Insert.class, Element.class, null, "one", "maps.elements") //
502503
);
@@ -521,9 +522,9 @@ public void multiLevelQualifiedReferencesWithOutId() {
521522
a -> getQualifier(a, noIdListMapContainerElements), //
522523
DbActionTestSupport::extractPath) //
523524
.containsExactly( //
525+
tuple(UpdateRoot.class, NoIdListMapContainer.class, null, null, ""), //
524526
tuple(Delete.class, NoIdElement.class, null, null, "maps.elements"), //
525527
tuple(Delete.class, NoIdMapContainer.class, null, null, "maps"), //
526-
tuple(UpdateRoot.class, NoIdListMapContainer.class, null, null, ""), //
527528
tuple(Insert.class, NoIdMapContainer.class, 0, null, "maps"), //
528529
tuple(Insert.class, NoIdElement.class, 0, "one", "maps.elements") //
529530
);

0 commit comments

Comments
 (0)