Commit bc289e91 authored by smartcontracts's avatar smartcontracts Committed by GitHub

fix(rhc): bug when target leads reference (#2456)

Fixes a bug that would cause the healthcheck service not to properly
check blocks when the target is advancing faster than the reference
client. RHC will now check the block correpsonding to the lesser of the
two block heights. Also adds a new metric to track the height difference
in the two providers which could be used to alert if the target leads
the reference by too much or vice versa.
Co-authored-by: default avatarmergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
parent dee51bc8
---
'@eth-optimism/replica-healthcheck': patch
---
Fixes a bug that would cause the service to stop properly checking blocks when the target client consistently leads the reference client
...@@ -18,6 +18,7 @@ type HealthcheckMetrics = { ...@@ -18,6 +18,7 @@ type HealthcheckMetrics = {
isCurrentlyDiverged: Gauge isCurrentlyDiverged: Gauge
referenceHeight: Gauge referenceHeight: Gauge
targetHeight: Gauge targetHeight: Gauge
heightDifference: Gauge
targetConnectionFailures: Counter targetConnectionFailures: Counter
referenceConnectionFailures: Counter referenceConnectionFailures: Counter
} }
...@@ -66,6 +67,10 @@ export class HealthcheckService extends BaseServiceV2< ...@@ -66,6 +67,10 @@ export class HealthcheckService extends BaseServiceV2<
type: Gauge, type: Gauge,
desc: 'Block height of the target client', desc: 'Block height of the target client',
}, },
heightDifference: {
type: Gauge,
desc: 'Difference in block heights between the two clients',
},
targetConnectionFailures: { targetConnectionFailures: {
type: Counter, type: Counter,
desc: 'Number of connection failures to the target client', desc: 'Number of connection failures to the target client',
...@@ -109,23 +114,36 @@ export class HealthcheckService extends BaseServiceV2< ...@@ -109,23 +114,36 @@ export class HealthcheckService extends BaseServiceV2<
} }
} }
// Later logic will depend on the height difference.
const heightDiff = Math.abs(referenceLatest.number - targetLatest.number)
const minBlock = Math.min(targetLatest.number, referenceLatest.number)
// Update these metrics first so they'll refresh no matter what. // Update these metrics first so they'll refresh no matter what.
this.metrics.targetHeight.set(targetLatest.number) this.metrics.targetHeight.set(targetLatest.number)
this.metrics.referenceHeight.set(referenceLatest.number) this.metrics.referenceHeight.set(referenceLatest.number)
this.metrics.heightDifference.set(heightDiff)
this.logger.info(`latest block heights`, { this.logger.info(`latest block heights`, {
targetHeight: targetLatest.number, targetHeight: targetLatest.number,
referenceHeight: referenceLatest.number, referenceHeight: referenceLatest.number,
heightDifference: referenceLatest.number - targetLatest.number, heightDifference: heightDiff,
minBlockNumber: minBlock,
}) })
const referenceCorresponding = const reference = await this.options.referenceRpcProvider.getBlock(minBlock)
await this.options.referenceRpcProvider.getBlock(targetLatest.number) if (!reference) {
// This is ok, but we should log it and restart the loop.
this.logger.info(`reference block was not found`, {
blockNumber: reference.number,
})
return
}
if (!referenceCorresponding) { const target = await this.options.targetRpcProvider.getBlock(minBlock)
if (!target) {
// This is ok, but we should log it and restart the loop. // This is ok, but we should log it and restart the loop.
this.logger.info(`reference client does not have block yet`, { this.logger.info(`target block was not found`, {
blockNumber: targetLatest.number, blockNumber: target.number,
}) })
return return
} }
...@@ -134,9 +152,11 @@ export class HealthcheckService extends BaseServiceV2< ...@@ -134,9 +152,11 @@ export class HealthcheckService extends BaseServiceV2<
// catch discrepancies in blocks that may not impact the state. For example, if clients have // catch discrepancies in blocks that may not impact the state. For example, if clients have
// blocks with two different timestamps, the state root will only diverge if the timestamp is // blocks with two different timestamps, the state root will only diverge if the timestamp is
// actually used during the transaction(s) within the block. // actually used during the transaction(s) within the block.
if (referenceCorresponding.hash !== targetLatest.hash) { if (reference.hash !== target.hash) {
this.logger.error(`reference client has different hash for block`, { this.logger.error(`reference client has different hash for block`, {
blockNumber: targetLatest.number, blockNumber: target.number,
referenceHash: reference.hash,
targetHash: target.hash,
}) })
// The main loop polls for "latest" so aren't checking every block. We need to use a binary // The main loop polls for "latest" so aren't checking every block. We need to use a binary
...@@ -144,7 +164,7 @@ export class HealthcheckService extends BaseServiceV2< ...@@ -144,7 +164,7 @@ export class HealthcheckService extends BaseServiceV2<
this.logger.info(`beginning binary search to find first mismatched block`) this.logger.info(`beginning binary search to find first mismatched block`)
let start = 0 let start = 0
let end = targetLatest.number let end = target.number
while (start !== end) { while (start !== end) {
const mid = Math.floor((start + end) / 2) const mid = Math.floor((start + end) / 2)
this.logger.info(`checking block`, { blockNumber: mid }) this.logger.info(`checking block`, { blockNumber: mid })
...@@ -171,11 +191,11 @@ export class HealthcheckService extends BaseServiceV2< ...@@ -171,11 +191,11 @@ export class HealthcheckService extends BaseServiceV2<
} }
this.logger.info(`blocks are matching`, { this.logger.info(`blocks are matching`, {
blockNumber: targetLatest.number, blockNumber: target.number,
}) })
// Update latest matching state root height and reset the diverged metric in case it was set. // Update latest matching state root height and reset the diverged metric in case it was set.
this.metrics.lastMatchingStateRootHeight.set(targetLatest.number) this.metrics.lastMatchingStateRootHeight.set(target.number)
this.metrics.isCurrentlyDiverged.set(0) this.metrics.isCurrentlyDiverged.set(0)
} }
} }
......
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