Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: block watcher metrics and last processed block #147

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mfw78
Copy link
Contributor

@mfw78 mfw78 commented Feb 14, 2024

Description

Fixes an issue associated with block processing unexpected errors.

Changes

  • Move metrics to finally
  • Move registry saving to finally

How to test

  1. Observe in staging

@mfw78 mfw78 added the bug Something isn't working label Feb 14, 2024
@mfw78 mfw78 requested a review from anxolin February 14, 2024 19:02
@mfw78 mfw78 self-assigned this Feb 14, 2024
// Block height metric
this.registry.lastProcessedBlock = blockToRegistryBlock(block);
await this.registry.write();
metrics.blockHeight.labels(chainId.toString()).set(blockNumber);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if the block was not process, why should we write it is.
Wouldn't this cover up next issue we have?

@@ -325,8 +325,11 @@ export class ChainContext {
log.debug(`Watchdog timeout: ${watchdogTimeout} seconds`);
let lastBlockReceived = lastProcessedBlock;
provider.on("block", async (blockNumber: number) => {
// Intentionally don't guard the getBlock call with a try/catch block
// because we want to crash the process if this fails. This will result
// in the kubernetes pod restarting and trying to recover.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would rephrase and not reference kubernetes here, as this is code that can be run in various ways

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants