Skip to content

Commit

Permalink
Less strict engine QoS timer (hyperledger#4411)
Browse files Browse the repository at this point in the history
* reset engine QoS timer with every call to the engine API, ExchangeTransitionConfiguration mismatch will only submit a debug log not a warning anymore

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
  • Loading branch information
daniellehrner authored Sep 21, 2022
1 parent 5e15625 commit bc9ff86
Show file tree
Hide file tree
Showing 14 changed files with 276 additions and 149 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### Additions and Improvements
- Allow free gas networks in the London fee market [#4061](https://github.com/hyperledger/besu/issues/4061)
- Upgrade besu-native to 0.6.0 and use Blake2bf native implementation if available by default [#4264](https://github.com/hyperledger/besu/pull/4264)
- Resets engine QoS timer with every call to the engine API instead of only when ExchangeTransitionConfiguration is called [#4411](https://github.com/hyperledger/besu/issues/4411)
- ExchangeTransitionConfiguration mismatch will only submit a debug log not a warning anymore [#4411](https://github.com/hyperledger/besu/issues/4411)

### Bug Fixes
- Retry block creation if there is a transient error and we still have time, to mitigate empty block issue [#4407](https://github.com/hyperledger/besu/pull/4407)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.hyperledger.besu.consensus.merge.MergeContext;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineCallListener;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
Expand Down Expand Up @@ -45,12 +46,17 @@ public enum EngineStatus {
protected final Optional<MergeContext> mergeContextOptional;
protected final Supplier<MergeContext> mergeContext;
protected final ProtocolContext protocolContext;
protected final EngineCallListener engineCallListener;

protected ExecutionEngineJsonRpcMethod(final Vertx vertx, final ProtocolContext protocolContext) {
protected ExecutionEngineJsonRpcMethod(
final Vertx vertx,
final ProtocolContext protocolContext,
final EngineCallListener engineCallListener) {
this.syncVertx = vertx;
this.protocolContext = protocolContext;
this.mergeContextOptional = protocolContext.safeConsensusContext(MergeContext.class);
this.mergeContext = mergeContextOptional::orElseThrow;
this.engineCallListener = engineCallListener;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Hyperledger Besu Contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

public interface EngineCallListener {
void executionEngineCalled();
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.QosTimer;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineExchangeTransitionConfigurationParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
Expand All @@ -46,21 +45,11 @@ public class EngineExchangeTransitionConfiguration extends ExecutionEngineJsonRp
Difficulty.fromHexString(
"0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc00");

static final long QOS_TIMEOUT_MILLIS = 120000L;

private final QosTimer qosTimer;

public EngineExchangeTransitionConfiguration(
final Vertx vertx, final ProtocolContext protocolContext) {
super(vertx, protocolContext);
qosTimer =
new QosTimer(
vertx,
QOS_TIMEOUT_MILLIS,
lastCall ->
LOG.warn(
"not called in {} seconds, consensus client may not be connected",
QOS_TIMEOUT_MILLIS / 1000L));
final Vertx vertx,
final ProtocolContext protocolContext,
final EngineCallListener engineCallListener) {
super(vertx, protocolContext, engineCallListener);
}

@Override
Expand All @@ -70,8 +59,7 @@ public String getName() {

@Override
public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) {
// update our QoS "last call time"
getQosTimer().resetTimer();
engineCallListener.executionEngineCalled();

final EngineExchangeTransitionConfigurationParameter remoteTransitionConfiguration =
requestContext.getRequiredParameter(
Expand All @@ -97,7 +85,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
if (!localTransitionConfiguration
.getTerminalTotalDifficulty()
.equals(remoteTransitionConfiguration.getTerminalTotalDifficulty())) {
LOG.warn(
LOG.debug(
"Configured terminal total difficulty {} does not match value of consensus client {}",
localTransitionConfiguration.getTerminalTotalDifficulty(),
remoteTransitionConfiguration.getTerminalTotalDifficulty());
Expand All @@ -106,7 +94,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
if (!localTransitionConfiguration
.getTerminalBlockHash()
.equals(remoteTransitionConfiguration.getTerminalBlockHash())) {
LOG.warn(
LOG.debug(
"Configured terminal block hash {} does not match value of consensus client {}",
localTransitionConfiguration.getTerminalBlockHash(),
remoteTransitionConfiguration.getTerminalBlockHash());
Expand All @@ -128,9 +116,4 @@ private JsonRpcResponse respondWith(
final EngineExchangeTransitionConfigurationResult transitionConfiguration) {
return new JsonRpcSuccessResponse(requestId, transitionConfiguration);
}

// QosTimer accessor for testing considerations
QosTimer getQosTimer() {
return qosTimer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ public class EngineForkchoiceUpdated extends ExecutionEngineJsonRpcMethod {
public EngineForkchoiceUpdated(
final Vertx vertx,
final ProtocolContext protocolContext,
final MergeMiningCoordinator mergeCoordinator) {
super(vertx, protocolContext);
final MergeMiningCoordinator mergeCoordinator,
final EngineCallListener engineCallListener) {
super(vertx, protocolContext, engineCallListener);
this.mergeCoordinator = mergeCoordinator;
}

Expand All @@ -62,6 +63,8 @@ public String getName() {

@Override
public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) {
engineCallListener.executionEngineCalled();

final Object requestId = requestContext.getRequest().getId();

final EngineForkchoiceUpdatedParameter forkChoice =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ public class EngineGetPayload extends ExecutionEngineJsonRpcMethod {
public EngineGetPayload(
final Vertx vertx,
final ProtocolContext protocolContext,
final BlockResultFactory blockResultFactory) {
super(vertx, protocolContext);
final BlockResultFactory blockResultFactory,
final EngineCallListener engineCallListener) {
super(vertx, protocolContext, engineCallListener);
this.blockResultFactory = blockResultFactory;
}

Expand All @@ -55,6 +56,8 @@ public String getName() {

@Override
public JsonRpcResponse syncResponse(final JsonRpcRequestContext request) {
engineCallListener.executionEngineCalled();

final PayloadIdentifier payloadId = request.getRequiredParameter(0, PayloadIdentifier.class);

final Optional<Block> block = mergeContext.get().retrieveBlockById(payloadId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ public EngineNewPayload(
final Vertx vertx,
final ProtocolContext protocolContext,
final MergeMiningCoordinator mergeCoordinator,
final EthPeers ethPeers) {
super(vertx, protocolContext);
final EthPeers ethPeers,
final EngineCallListener engineCallListener) {
super(vertx, protocolContext, engineCallListener);
this.mergeCoordinator = mergeCoordinator;
this.ethPeers = ethPeers;
}
Expand All @@ -86,6 +87,8 @@ public String getName() {

@Override
public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) {
engineCallListener.executionEngineCalled();

final EnginePayloadParameter blockParam =
requestContext.getRequiredParameter(0, EnginePayloadParameter.class);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Hyperledger Besu Contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import org.hyperledger.besu.ethereum.api.jsonrpc.internal.QosTimer;

import com.google.common.annotations.VisibleForTesting;
import io.vertx.core.Vertx;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class EngineQosTimer implements EngineCallListener {
static final long QOS_TIMEOUT_MILLIS = 120000L;
private static final Logger LOG = LoggerFactory.getLogger(EngineQosTimer.class);

private final QosTimer qosTimer;

public EngineQosTimer(final Vertx vertx) {
qosTimer = new QosTimer(vertx, QOS_TIMEOUT_MILLIS, lastCall -> logTimeoutWarning());
qosTimer.resetTimer();
}

@Override
public void executionEngineCalled() {
getQosTimer().resetTimer();
}

public void logTimeoutWarning() {
LOG.warn(
"Execution engine not called in {} seconds, consensus client may not be connected",
QOS_TIMEOUT_MILLIS / 1000L);
}

@VisibleForTesting
public QosTimer getQosTimer() {
return qosTimer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineForkchoiceUpdated;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineGetPayload;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineNewPayload;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineQosTimer;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
Expand Down Expand Up @@ -62,17 +63,26 @@ protected String getApiGroup() {

@Override
protected Map<String, JsonRpcMethod> create() {
final EngineQosTimer engineQosTimer = new EngineQosTimer(consensusEngineServer);

if (mergeCoordinator.isPresent()) {
return mapOf(
new EngineGetPayload(consensusEngineServer, protocolContext, blockResultFactory),
new EngineGetPayload(
consensusEngineServer, protocolContext, blockResultFactory, engineQosTimer),
new EngineNewPayload(
consensusEngineServer, protocolContext, mergeCoordinator.get(), ethPeers),
consensusEngineServer,
protocolContext,
mergeCoordinator.get(),
ethPeers,
engineQosTimer),
new EngineForkchoiceUpdated(
consensusEngineServer, protocolContext, mergeCoordinator.get()),
new EngineExchangeTransitionConfiguration(consensusEngineServer, protocolContext));
consensusEngineServer, protocolContext, mergeCoordinator.get(), engineQosTimer),
new EngineExchangeTransitionConfiguration(
consensusEngineServer, protocolContext, engineQosTimer));
} else {
return mapOf(
new EngineExchangeTransitionConfiguration(consensusEngineServer, protocolContext));
new EngineExchangeTransitionConfiguration(
consensusEngineServer, protocolContext, engineQosTimer));
}
}
}
Loading

0 comments on commit bc9ff86

Please sign in to comment.