Skip to content

Commit cfc8184

Browse files
artembilangaryrussell
authored andcommitted
spring-projectsGH-173: Leaders: Warn event errors, not re-throw
Fixes spring-projectsGH-173 (spring-projects#173) Currently when an error is thrown from the event publishing the role granting is broken and we just go to the role revoking. * Since it's just an event publishing it shouldn't effect the original leader election. * `try...catch` event publishing in the `LeaderInitiator` and `logger.warn` an `Exception`
1 parent 848ae2d commit cfc8184

File tree

2 files changed

+66
-7
lines changed

2 files changed

+66
-7
lines changed

spring-integration-hazelcast/src/main/java/org/springframework/integration/hazelcast/leader/LeaderInitiator.java

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
import java.util.concurrent.ThreadFactory;
2424
import java.util.concurrent.TimeUnit;
2525

26+
import org.apache.commons.logging.Log;
27+
import org.apache.commons.logging.LogFactory;
28+
2629
import org.springframework.beans.factory.DisposableBean;
2730
import org.springframework.context.ApplicationEventPublisher;
2831
import org.springframework.context.ApplicationEventPublisherAware;
@@ -49,6 +52,8 @@
4952
*/
5053
public class LeaderInitiator implements SmartLifecycle, DisposableBean, ApplicationEventPublisherAware {
5154

55+
private static final Log logger = LogFactory.getLog(LeaderInitiator.class);
56+
5257
private static int threadNameCount = 0;
5358

5459
private static final Context NULL_CONTEXT = new NullContext();
@@ -236,8 +241,13 @@ public Void call() throws Exception {
236241
try {
237242
this.locked = LeaderInitiator.this.lock.tryLock(Long.MAX_VALUE, TimeUnit.MILLISECONDS);
238243
if (this.locked) {
239-
LeaderInitiator.this.leaderEventPublisher.publishOnGranted(LeaderInitiator.this,
240-
this.context, this.role);
244+
try {
245+
LeaderInitiator.this.leaderEventPublisher.publishOnGranted(LeaderInitiator.this,
246+
this.context, this.role);
247+
}
248+
catch (Exception e) {
249+
logger.warn("Error publishing OnGranted event.", e);
250+
}
241251
LeaderInitiator.this.candidate.onGranted(this.context);
242252
Thread.sleep(Long.MAX_VALUE);
243253
}
@@ -249,9 +259,14 @@ public Void call() throws Exception {
249259
// The lock was broken and we are no longer leader
250260
LeaderInitiator.this.candidate.onRevoked(this.context);
251261
if (LeaderInitiator.this.leaderEventPublisher != null) {
252-
LeaderInitiator.this.leaderEventPublisher.publishOnRevoked(
253-
LeaderInitiator.this, this.context,
254-
LeaderInitiator.this.candidate.getRole());
262+
try {
263+
LeaderInitiator.this.leaderEventPublisher.publishOnRevoked(
264+
LeaderInitiator.this, this.context,
265+
LeaderInitiator.this.candidate.getRole());
266+
}
267+
catch (Exception ex) {
268+
logger.warn("Error publishing OnRevoked event.", ex);
269+
}
255270
}
256271
Thread.currentThread().interrupt();
257272
return null;
@@ -266,8 +281,13 @@ public Void call() throws Exception {
266281
// We are stopping, therefore not leading any more
267282
LeaderInitiator.this.candidate.onRevoked(this.context);
268283
if (LeaderInitiator.this.leaderEventPublisher != null) {
269-
LeaderInitiator.this.leaderEventPublisher.publishOnRevoked(
270-
LeaderInitiator.this, this.context, this.role);
284+
try {
285+
LeaderInitiator.this.leaderEventPublisher.publishOnRevoked(
286+
LeaderInitiator.this, this.context, this.role);
287+
}
288+
catch (Exception e) {
289+
logger.warn("Error publishing OnRevoked event.", e);
290+
}
271291
}
272292
}
273293
}

spring-integration-hazelcast/src/test/java/org/springframework/integration/hazelcast/leader/LeaderInitiatorTests.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import static org.hamcrest.CoreMatchers.is;
2121
import static org.junit.Assert.assertNotNull;
2222
import static org.junit.Assert.assertThat;
23+
import static org.mockito.BDDMockito.willAnswer;
24+
import static org.mockito.Matchers.any;
25+
import static org.mockito.Mockito.spy;
2326

2427
import java.util.ArrayList;
2528
import java.util.List;
@@ -36,6 +39,7 @@
3639
import org.springframework.integration.leader.Context;
3740
import org.springframework.integration.leader.DefaultCandidate;
3841
import org.springframework.integration.leader.event.AbstractLeaderEvent;
42+
import org.springframework.integration.leader.event.DefaultLeaderEventPublisher;
3943
import org.springframework.integration.leader.event.LeaderEventPublisher;
4044
import org.springframework.test.annotation.DirtiesContext;
4145
import org.springframework.test.context.ContextConfiguration;
@@ -168,8 +172,43 @@ public void publishOnRevoked(Object source, Context context, String role) {
168172
assertThat(revoked11.await(10, TimeUnit.SECONDS), is(true));
169173

170174
initiator1.destroy();
175+
176+
177+
178+
CountDownLatch onGranted = new CountDownLatch(1);
179+
180+
DefaultCandidate candidate = spy(new DefaultCandidate());
181+
willAnswer(invocation -> {
182+
try {
183+
return invocation.callRealMethod();
184+
}
185+
finally {
186+
onGranted.countDown();
187+
}
188+
})
189+
.given(candidate).onGranted(any(Context.class));
190+
191+
LeaderInitiator initiator = new LeaderInitiator(this.hazelcastInstance, candidate);
192+
193+
initiator.setLeaderEventPublisher(new DefaultLeaderEventPublisher() {
194+
195+
@Override
196+
public void publishOnGranted(Object source, Context context, String role) {
197+
throw new RuntimeException("intentional");
198+
}
199+
200+
});
201+
202+
initiator.start();
203+
204+
assertThat(onGranted.await(5, TimeUnit.SECONDS), is(true));
205+
206+
assertThat(initiator.getContext().isLeader(), is(true));
207+
208+
initiator.destroy();
171209
}
172210

211+
173212
@Configuration
174213
public static class TestConfig {
175214

0 commit comments

Comments
 (0)