Skip to content

Commit

Permalink
pr feedback and cleanup
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Farber-Newman <tim.farber-newman@swirldslabs.com>
  • Loading branch information
timfn-hg committed Jan 9, 2025
1 parent 652d835 commit d26cffa
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.swirlds.platform.system.state.notifications.FatalIssListener;
import com.swirlds.platform.system.state.notifications.IssNotification;
import edu.umd.cs.findbugs.annotations.NonNull;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.logging.log4j.LogManager;
Expand All @@ -34,7 +35,7 @@ public FatalIssListenerImpl() {
}

@Override
public void notify(final IssNotification data) {
public void notify(@NonNull final IssNotification data) {
log.warn("ISS detected (type={}, round={})", data.getIssType(), data.getRound());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022-2025 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -65,7 +65,7 @@ public DefaultIssHandler(
@Override
public void issObserved(@NonNull final IssNotification issNotification) {
switch (issNotification.getIssType()) {
case SELF_ISS -> selfIssObserved(issNotification);
case SELF_ISS -> selfIssObserved(issNotification.getRound());
case OTHER_ISS -> otherIssObserved();
case CATASTROPHIC_ISS -> catastrophicIssObserver(issNotification.getRound());
}
Expand Down Expand Up @@ -109,16 +109,16 @@ private void updateIssRoundInScratchpad(final long issRound) {
/**
* This method is called when there is a self ISS.
*
* @param notification the self-ISS notification
* @param round the round of the ISS
*/
private void selfIssObserved(@NonNull final IssNotification notification) {
private void selfIssObserved(@NonNull final Long round) {

if (halted) {
// don't take any action once halted
return;
}

updateIssRoundInScratchpad(notification.getRound());
updateIssRoundInScratchpad(round);

if (stateConfig.haltOnAnyIss()) {
haltRequestedConsumer.accept("self ISS observed");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import com.swirlds.common.notification.Listener;

/**
* Listener for fatal ISS events (i.e. of type SELF or CATASTROPHIC). This listener is unordered and asynchronous.
* Listener for fatal ISS events (i.e. of type SELF or CATASTROPHIC). This listener is ordered and asynchronous.
* If you require ordered and synchronous dispatch that includes all ISS events, then use {@link IssListener}.
*/
@DispatchModel(mode = DispatchMode.ASYNC, order = DispatchOrder.UNORDERED)
@DispatchModel(mode = DispatchMode.ASYNC, order = DispatchOrder.ORDERED)
public interface FatalIssListener extends Listener<IssNotification> {}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022-2025 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,7 +41,6 @@

@DisplayName("IssHandler Tests")
class IssHandlerTests {

@Test
@DisplayName("Other ISS Always Freeze")
void otherIssAlwaysFreeze() {
Expand All @@ -60,7 +59,6 @@ void otherIssAlwaysFreeze() {
final FatalErrorConsumer fatalErrorConsumer = (msg, t, code) -> shutdownCount.getAndIncrement();

final Scratchpad<IssScratchpad> simpleScratchpad = new SimpleScratchpad<>();

final IssHandler handler =
new DefaultIssHandler(platformContext, haltRequestedConsumer, fatalErrorConsumer, simpleScratchpad);

Expand Down Expand Up @@ -130,15 +128,14 @@ void selfIssAutomatedRecovery() {
final IssHandler handler =
new DefaultIssHandler(platformContext, haltRequestedConsumer, fatalErrorConsumer, simpleScratchpad);

final IssNotification notification = new IssNotification(1234L, IssType.SELF_ISS);
handler.issObserved(notification);
handler.issObserved(new IssNotification(1234L, IssType.SELF_ISS));

assertEquals(0, freezeCount.get(), "unexpected freeze count");
assertEquals(1, shutdownCount.get(), "unexpected shutdown count");

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}

@Test
Expand All @@ -163,15 +160,14 @@ void selfIssNoAction() {
final IssHandler handler =
new DefaultIssHandler(platformContext, haltRequestedConsumer, fatalErrorConsumer, simpleScratchpad);

final IssNotification notification = new IssNotification(1234L, IssType.SELF_ISS);
handler.issObserved(notification);
handler.issObserved(new IssNotification(1234L, IssType.SELF_ISS));

assertEquals(0, freezeCount.get(), "unexpected freeze count");
assertEquals(0, shutdownCount.get(), "unexpected shutdown count");

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}

@Test
Expand All @@ -196,21 +192,20 @@ void selfIssAlwaysFreeze() {
final IssHandler handler =
new DefaultIssHandler(platformContext, haltRequestedConsumer, fatalErrorConsumer, simpleScratchpad);

final IssNotification notification = new IssNotification(1234L, IssType.SELF_ISS);
handler.issObserved(notification);
handler.issObserved(new IssNotification(1234L, IssType.SELF_ISS));

assertEquals(1, freezeCount.get(), "unexpected freeze count");
assertEquals(0, shutdownCount.get(), "unexpected shutdown count");

// Once frozen, this should become a no-op
handler.issObserved(new IssNotification(1235L, IssType.SELF_ISS));
handler.issObserved(new IssNotification(1234L, IssType.SELF_ISS));

assertEquals(1, freezeCount.get(), "unexpected freeze count");
assertEquals(0, shutdownCount.get(), "unexpected shutdown count");

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}

@Test
Expand Down Expand Up @@ -242,7 +237,7 @@ void catastrophicIssNoAction() {

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}

@Test
Expand Down Expand Up @@ -280,7 +275,7 @@ void catastrophicIssAlwaysFreeze() {

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}

@Test
Expand Down Expand Up @@ -318,6 +313,6 @@ void catastrophicIssFreezeOnCatastrophic() {

final SerializableLong issRound = simpleScratchpad.get(IssScratchpad.LAST_ISS_ROUND);
assertNotNull(issRound);
assertEquals(1234L, issRound.getValue());
assertEquals(issRound.getValue(), 1234L);
}
}

0 comments on commit d26cffa

Please sign in to comment.