Commit 549a9018 authored by Georg Mittendorfer's avatar Georg Mittendorfer

Replace pow check with health check that checks for pow as well

Old pow check was to send an invalid empty attachToTangle command and if
pow was enabled IRI returned a http 400. This was replaced by a feature
in the GetNodeInfo response. Additionally a bug with 1.7.1 brakes the
old pow check. This change is incompatible to old nodes that don't have
the GetNodeInfo features. Pow nodes still use the old check as many/all
do not support GetNodeInfo.
parent 2b3d4dc3
Pipeline #66974100 passed with stage
in 3 minutes and 47 seconds
......@@ -31,10 +31,11 @@ import org.slf4j.LoggerFactory;
import org.springframework.http.ResponseEntity;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Set;
import java.util.stream.Collectors;
import static com.mio.piri.commands.response.NodeInfo.REMOTE_POW;
public class GetNodeInfoResponsePostProcessor {
private final Logger logger = LoggerFactory.getLogger(getClass());
......@@ -135,26 +136,12 @@ public class GetNodeInfoResponsePostProcessor {
}
}
private void correctPowFeature(NodeInfo nodeInfo) {
if (powAvailable) {
if (!containsPowFeature(nodeInfo)) {
addPowFeature(nodeInfo);
}
} else if (containsPowFeature(nodeInfo)) {
nodeInfo.getFeatures().remove("RemotePOW");
}
}
private boolean containsPowFeature(NodeInfo nodeInfo) {
return nodeInfo.getFeatures() != null && nodeInfo.getFeatures().contains("RemotePOW");
}
private void addPowFeature(NodeInfo nodeInfo) {
if (nodeInfo.getFeatures() == null) {
nodeInfo.setFeatures(new ArrayList<>());
if (!powAvailable) {
nodeInfo.removeFeature(REMOTE_POW);
} else {
nodeInfo.addFeature(REMOTE_POW);
}
nodeInfo.getFeatures().add("RemotePOW");
}
}
......@@ -22,12 +22,14 @@ package com.mio.piri.commands.response;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import java.util.List;
import java.util.LinkedHashSet;
import java.util.Set;
import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_EMPTY;
import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
......@@ -40,6 +42,7 @@ import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
public class NodeInfo {
public static final int INVALID_MILESTONE_INDEX = -1;
public static final String REMOTE_POW = "RemotePOW";
@JsonInclude(value=NON_EMPTY)
private String appName;
......@@ -78,7 +81,8 @@ public class NodeInfo {
private int transactionsToRequest;
@JsonInclude(value=NON_NULL)
private List<String> features;
@JsonDeserialize(as=LinkedHashSet.class) // preserves order
private Set<String> features;
private String coordinatorAddress;
......@@ -90,6 +94,31 @@ public class NodeInfo {
return hasValidLatestMilestoneIndex() && latestSolidSubtangleMilestoneIndex + 1 >= latestMilestoneIndex;
}
public boolean supportsPow() {
return features != null && features.contains(REMOTE_POW);
}
/**
* Removes a features or does nothing if the feature is not present.
* @param feature The feature to be removed.
*/
public void removeFeature(String feature) {
if (features != null) {
features.remove(feature);
}
}
/**
* Adds a feature.
* @param feature The feature to be added. Implementation makes sure that no duplicates are added.
*/
public void addFeature(String feature) {
if (features == null) {
features = new LinkedHashSet<>();
}
features.add(feature);
}
@JsonIgnore
private boolean hasValidLatestMilestoneIndex() {
return latestMilestoneIndex > 0;
......
......@@ -21,10 +21,13 @@ package com.mio.piri.commands.response;
import lombok.Builder;
import java.util.LinkedHashSet;
import java.util.List;
public class ResponseDtoBuilder {
@Builder(builderMethodName = "nodeInfoBuilder")
public static NodeInfo buildNodeInfo(String appName, String appVersion, int availableProcessors, long jreFreeMemory, long jreTotalMemory, long jreMaxMemory, int neighbors, int tips, int transactionsToRequest, Integer latestMilestoneIndex, Integer latestSolidSubtangleMilestoneIndex) {
public static NodeInfo buildNodeInfo(String appName, String appVersion, int availableProcessors, long jreFreeMemory, long jreTotalMemory, long jreMaxMemory, int neighbors, int tips, int transactionsToRequest, Integer latestMilestoneIndex, Integer latestSolidSubtangleMilestoneIndex, List<String> features) {
NodeInfo nodeInfo = new NodeInfo();
nodeInfo.setAppName(appName);
nodeInfo.setAppVersion(appVersion);
......@@ -35,6 +38,7 @@ public class ResponseDtoBuilder {
nodeInfo.setNeighbors(neighbors);
nodeInfo.setTips(tips);
nodeInfo.setTransactionsToRequest(transactionsToRequest);
nodeInfo.setFeatures(features == null ? null : new LinkedHashSet<>(features));
if (latestMilestoneIndex != null) {
nodeInfo.setLatestMilestoneIndex(latestMilestoneIndex);
}
......
......@@ -37,7 +37,6 @@ import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import reactor.core.publisher.Mono;
......@@ -163,22 +162,6 @@ public class IriNode implements Node {
.onErrorResume(t -> Mono.just(this.updateWithHealthCheckError(t)));
}
public void doPowCheck() {
// CHECK circuit breaker should not get triggered here, should it?
nodeClient.exchange(this, new AttachToTangle()).subscribe(
responseEntity -> {
// if pow is enabled node sends 400 with 'invalid parameters' message
logger.debug("Received POW check response status [{}] for node [{}].",
responseEntity.getStatusCodeValue(), getName());
updatePowAvailability(responseEntity.getStatusCode() == HttpStatus.BAD_REQUEST);
},
t -> {
logger.info("Error on POW check for node [{}]: {}", getName(), t.toString());
updatePowAvailability(false);
}
);
}
private boolean isCallPermitted() {
boolean callPermitted = circuitBreaker.isCallPermitted();
if (!callPermitted && logger.isDebugEnabled()) {
......@@ -229,6 +212,8 @@ public class IriNode implements Node {
this.info.setLatestMilestoneIndex(currentNodeInfo.getLatestMilestoneIndex());
this.info.setLatestSolidSubtangleMilestoneIndex(currentNodeInfo.getLatestSolidSubtangleMilestoneIndex());
updatePowAvailability(currentNodeInfo.supportsPow());
if (!isVersionSupported(appVersion) || isVersionBlacklisted(appVersion)) {
// disable or leave disabled if version is not supported
if (isAvailable()) {
......@@ -253,7 +238,7 @@ public class IriNode implements Node {
&& VERSION_BLACKLIST.matcher(StringUtils.defaultString(appVersion, "null")).matches();
}
public void updatePowAvailability(boolean available) {
void updatePowAvailability(boolean available) {
logger.debug("POW check for node [{}]. POW: [{}]", name, available);
if (powAvailable != available) {
powAvailable = available;
......
......@@ -83,10 +83,6 @@ public class NodeRegistry {
t -> logger.error(t.toString(), t)
);
Duration powCheckInterval = DurationStyle.detectAndParse(env.getProperty("piri.pow.check.interval", "30m"));
Flux.interval(Duration.ofMinutes(1), powCheckInterval).subscribe(checkNumber -> runPowCheck(nodes, checkNumber),
t -> logger.error(t.toString(), t));
}
public IriNode registerIriNode(String name, String url, String key, boolean pow) {
......@@ -175,10 +171,10 @@ public class NodeRegistry {
return StringUtils.isNotBlank(url) && urlValidator.isValidUrl(url);
}
void runPowCheck(Map<String, IriNode> nodes, long checkNumber) {
logger.info("Running pow check [{}].", checkNumber);
nodes.values().stream().filter(node -> node.isPowEnabled() && node.isCallPossible()).forEach(IriNode::doPowCheck);
}
// void runPowCheck(Map<String, IriNode> nodes, long checkNumber) {
// logger.info("Running pow check [{}].", checkNumber);
// nodes.values().stream().filter(node -> node.isPowEnabled() && node.isCallPossible()).forEach(IriNode::doPowCheck);
// }
void runHealthCheck(Map<String, IriNode> nodes, long checkNumber) {
logger.info("Running health check [{}].", checkNumber);
......
......@@ -32,7 +32,9 @@ import reactor.core.publisher.Mono;
import java.time.Duration;
// TODO add test and fix hierarchy
import static com.mio.piri.commands.response.NodeInfo.REMOTE_POW;
// TODO fix hierarchy Pow node is NOT an IRI node
public class PowNode extends IriNode implements Node {
public static Duration POW_TIMEOUT = Duration.ofSeconds(5);
......@@ -42,6 +44,7 @@ public class PowNode extends IriNode implements Node {
super(name, url, key,true, nodeClient, metrics, circuitBreaker);
info.setAppName("POW Node");
info.setAppVersion(null);
info.addFeature(REMOTE_POW);
info.setNeighbors(0);
}
......@@ -77,7 +80,7 @@ public class PowNode extends IriNode implements Node {
private NodeInfo updateWithHealthCheckError(String message) {
logger.info("Node [{}] not healthy: {}", name, message);
if (isAvailable()) {
setUnavailable("not available");
setUnavailable("health check failed");
}
return info;
}
......@@ -92,7 +95,7 @@ public class PowNode extends IriNode implements Node {
@Override
public boolean isSupported(IriCommand command) {
// we support pow only
return powAvailable && command instanceof AttachToTangle;
return command instanceof AttachToTangle;
}
}
......@@ -17,8 +17,6 @@ server.address=0.0.0.0
# health check interval duration
piri.health.check.interval=1m
# checks if pow for node is still available
piri.pow.check.interval=1h
# bind client sessions to certain nodes
# this makes sure that wallets a most consistent state by being served by the same node if feasible
......
......@@ -22,15 +22,19 @@ package com.mio.piri.commands.response;
import com.mio.piri.util.JsonUtil;
import org.junit.Test;
import java.util.Arrays;
import java.util.Collections;
import static com.mio.piri.commands.response.ResponseDtoBuilder.nodeInfoBuilder;
import static org.assertj.core.api.Assertions.assertThat;
public class IriNodeInfoTest {
public class NodeInfoTest {
private final NodeInfo sameMileStone = nodeInfoBuilder().latestMilestoneIndex(42).latestSolidSubtangleMilestoneIndex(42).build();
private final NodeInfo oneOff = nodeInfoBuilder().latestMilestoneIndex(42).latestSolidSubtangleMilestoneIndex(41).build();
private final NodeInfo notSynced = nodeInfoBuilder().latestMilestoneIndex(42).latestSolidSubtangleMilestoneIndex(40).build();
@SuppressWarnings("SpellCheckingInspection")
private static final String nodeInfoExample = "{" +
"\"appName\":\"IRI Testnet\"," +
"\"appVersion\":\"1.5.6-RELEASE\"," +
......@@ -83,6 +87,7 @@ public class IriNodeInfoTest {
.contains("\"latestMilestoneIndex\":42");
}
@SuppressWarnings("SpellCheckingInspection")
@Test
public void whenFromJsonThenAllPropertiesAreSet() {
NodeInfo nodeInfo = JsonUtil.fromJson(nodeInfoExample, NodeInfo.class);
......@@ -115,4 +120,38 @@ public class IriNodeInfoTest {
assertThat(nodeInfoExample).isEqualTo(back);
}
@Test
public void givenNoFeaturesWhenSupportsPowThenFalse() {
NodeInfo nodeInfo = nodeInfoBuilder().build();
assertThat(nodeInfo.supportsPow()).isFalse();
}
@Test
public void givenNoPowFeatureWhenSupportsPowThenFalse() {
NodeInfo nodeInfo = nodeInfoBuilder().features(Arrays.asList("foo", "bar")).build();
assertThat(nodeInfo.supportsPow()).isFalse();
}
@Test
public void givenPowFeatureWhenSupportsPowThenTrue() {
NodeInfo nodeInfo = nodeInfoBuilder().features(Arrays.asList("foo", "RemotePOW", "bar")).build();
assertThat(nodeInfo.supportsPow()).isTrue();
}
@Test
public void whenAddFeatureThenCreateSet() {
NodeInfo nodeInfo = nodeInfoBuilder().build();
assertThat(nodeInfo.getFeatures()).isNull();
nodeInfo.addFeature("foo");
assertThat(nodeInfo.getFeatures()).contains("foo");
}
@Test
public void whenRemoveFeaturesThenRemoveFromSet() {
NodeInfo nodeInfo = nodeInfoBuilder().features(Collections.singletonList("foo")).build();
nodeInfo.removeFeature("foo");
nodeInfo.removeFeature("bar");
assertThat(nodeInfo.getFeatures()).isEmpty(); // could this be a problem in the future that features is not null now?
}
}
\ No newline at end of file
......@@ -36,8 +36,10 @@ import org.springframework.http.ResponseEntity;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.util.Collections;
import java.util.regex.Pattern;
import static com.mio.piri.commands.response.NodeInfo.REMOTE_POW;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.any;
......@@ -228,8 +230,17 @@ public class IriNodeTest {
assertThat(node.isSupported(mock(AttachToTangle.class))).isTrue();
}
/**
* Tests case where node is configured to support pow but health check shows that pow is not available.
*/
@Test
public void givenPowDisabledWhenIsCommandSupportedReturnFalse() {
public void givenPowUnavailableWhenIsCommandSupportedThenReturnFalse() {
node.updatePowAvailability(false);
assertThat(node.isSupported(mock(AttachToTangle.class))).isFalse();
}
@Test
public void givenPowDisabledWhenIsCommandSupportedThenReturnFalse() {
IriNode node = new IriNode("foo", "url", "fooKey", false, nodeClient, metrics, circuitBreaker);
assertThat(node.isSupported(mock(AttachToTangle.class))).isFalse();
}
......@@ -252,6 +263,26 @@ public class IriNodeTest {
assertThat(node.getLatestNodeInfo().getLatestMilestoneIndex()).isEqualTo(3);
}
@Test
public void givenPowSupportedWhenUpdateWithHealthCheckEnablePow() {
NodeInfo nodeInfo = ResponseDtoBuilder.nodeInfoBuilder().features(Collections.singletonList(REMOTE_POW)).build();
node.updatePowAvailability(false);
node.updateWithHealthCheckResult(nodeInfo);
assertThat(node.isPowAvailable()).isTrue();
}
@Test
public void givenNoPowSupportedWhenUpdateWithHealthCheckDisablePow() {
NodeInfo nodeInfo = ResponseDtoBuilder.nodeInfoBuilder().build();
assertThat(node.isPowAvailable()).isTrue();
node.updateWithHealthCheckResult(nodeInfo);
assertThat(node.isPowAvailable()).isFalse();
}
@Test
public void givenGetNodeInfoWhenCallNodeThenReuseForHealthUpdate() {
IriCommand command = new GetNodeInfo();
......@@ -284,40 +315,4 @@ public class IriNodeTest {
.verifyComplete();
}
@Test
public void givenPowAvailableWhenPowCheckThenEnablePow() {
setWebClientResponse(ResponseEntity.status(HttpStatus.BAD_REQUEST).build());
node.updatePowAvailability(false);
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isTrue();
}
@Test
public void givenNoPowWhenPowCheckThenDisablePow() {
setWebClientResponse(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
}
@Test
public void given5xxWhenPowCheckThenDisablePow() {
setWebClientResponse(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
}
@Test
public void givenRateLimitExceededWhenPowCheckThenDisablePow() {
setWebClientResponse(ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
}
@Test
public void givenServerErrorWhenPowCheckThenDisablePow() {
setWebClientResponse(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
}
}
\ No newline at end of file
......@@ -44,7 +44,6 @@ import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
......@@ -81,7 +80,6 @@ public class NodeRegistryTest {
public void setUpMocks() {
when(circuitBreakerFactory.circuitBreaker(anyString())).thenReturn(CircuitBreaker.ofDefaults("foo"));
when(urlValidator.isValidUrl(anyString())).thenReturn(true);
when(env.getProperty(anyString(), anyString())).thenReturn("42s");
when(env.getProperty(eq("piri.health.check.interval"), anyString())).thenReturn("10s");
registry = new NodeRegistry(urlValidator, null, meterFactory, circuitBreakerFactory, nodesRepository, env);
}
......@@ -132,11 +130,6 @@ public class NodeRegistryTest {
registry.runHealthCheck(new HashMap<>(), 0);
}
@Test
public void givenNoNodesWhenPowCheckThenDoNotThrow() {
registry.runPowCheck(new HashMap<>(), 0);
}
@Test
public void whenHealthCheckThenQueryForEachNode() {
HashMap<String, IriNode> nodeMap = nodeMapWithTwoNodes();
......@@ -145,21 +138,6 @@ public class NodeRegistryTest {
nodeMap.values().forEach(node -> verify(node).queryNodeHealth());
}
@Test
public void whenPowCheckThenQueryEachNodeThatHasPowEnabled() {
HashMap<String, IriNode> nodeMap = nodeMapWithTwoNodes();
IriNode node = nodeWithPow();
nodeMap.put("foo", node);
registry.runPowCheck(nodeMap, 0);
nodeMap.forEach((key, value) -> {
if (!key.equals("foo")) {
verify(value, never()).doPowCheck();
} else {
verify(value).doPowCheck();
}
});
}
@Test
public void whenHealthCheckThenUpdateLatestKnownMilestone() {
HashMap<String, IriNode> nodeMap = nodeMapWithTwoNodes();
......
......@@ -61,6 +61,11 @@ public class PowNodeTest {
when(nodeClient.exchange(any(Node.class), any(AttachToTangle.class))).thenReturn(Mono.just(clientResponse));
}
@Test
public void shouldSupportAttachToTangle() {
assertThat(node.isSupported(new AttachToTangle())).isTrue();
}
@Test
public void givenClosedCircuitBreakerWhenHealthCheckThenDoNotThrow() {
setWebClientResponse(okResponseEntity); // mock node client to return response mono.
......@@ -76,32 +81,48 @@ public class PowNodeTest {
}
@Test
public void givenPowAvailableWhenPowCheckThenEnablePow() {
public void givenPowAvailableWhenPowCheckThenSetAvailable() {
setWebClientResponse(ResponseEntity.status(HttpStatus.BAD_REQUEST).build());
node.updatePowAvailability(false);
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isTrue();
node.setUnavailable("test preparation");
StepVerifier.create(node.queryNodeHealth())
.assertNext(info -> assertThat(info.supportsPow()).isTrue())
.verifyComplete();
assertThat(node.isAvailable()).isTrue();
}
@Test
public void given401WhenPowCheckThenDisablePow() {
public void given401WhenPowCheckThenSetDisabled() {
setWebClientResponse(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
StepVerifier.create(node.queryNodeHealth())
.assertNext(info -> assertThat(info.supportsPow()).isTrue())
.verifyComplete();
assertThat(node.isAvailable()).isFalse();
}
@Test
public void given429WhenPowCheckThenDisablePow() {
public void given429WhenPowCheckThenSetDisabled() {
setWebClientResponse(ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
StepVerifier.create(node.queryNodeHealth())
.assertNext(info -> assertThat(info.supportsPow()).isTrue())
.verifyComplete();
assertThat(node.isAvailable()).isFalse();
}
@Test
public void given5xxWhenPowCheckThenDisablePow() {
public void given5xxWhenPowCheckThenSetDisabled() {
setWebClientResponse(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build());
node.doPowCheck();
assertThat(node.isSupported(new AttachToTangle())).isFalse();
StepVerifier.create(node.queryNodeHealth())
.assertNext(info -> assertThat(info.supportsPow()).isTrue())
.verifyComplete();
assertThat(node.isAvailable()).isFalse();
}
}
\ No newline at end of file
......@@ -29,7 +29,7 @@ import org.slf4j.LoggerFactory;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.junit4.SpringRunner;
import java.util.ArrayList;
import java.util.HashSet;
import static org.assertj.core.api.Assertions.assertThat;
......@@ -54,7 +54,7 @@ public class IriApiHandlerIT extends AbstractIriApiHandlerTest {
@Test
public void givenGetNodeInfoWhenExchangeThenReturnRemotePowEnabled() {
NodeInfo nodeInfo = new NodeInfo();
nodeInfo.setFeatures(new ArrayList<>());
nodeInfo.setFeatures(new HashSet<>());
nodeInfo.getFeatures().add("RemotePOW");
prepareResponse(response -> response
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment