-
Notifications
You must be signed in to change notification settings - Fork 16
Ensure valid ENSRainbow connection before indexing starts #1850
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
base: main
Are you sure you want to change the base?
Changes from all commits
3f5dc4f
f9e4e11
40bab87
e84c0c7
febed1c
911035a
22c3c22
d0dd98c
b9a0829
ef53bf8
d51b2ea
e60b06f
f929618
63fbec5
fb1d93e
c3967b9
f346b2a
4e2454b
caef441
3d7bfb3
8b96b5d
4a80427
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@ensnode/ensdb-sdk": minor | ||
| --- | ||
|
|
||
| Extended the ENSNode Metadata with ENSRainbow Public Config. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ import config from "@/config"; | |||||
| import { secondsToMilliseconds } from "date-fns"; | ||||||
| import pRetry from "p-retry"; | ||||||
|
|
||||||
| import type { Duration } from "@ensnode/ensnode-sdk"; | ||||||
| import { EnsRainbowApiClient } from "@ensnode/ensrainbow-sdk"; | ||||||
|
|
||||||
| import { logger } from "@/lib/logger"; | ||||||
|
|
@@ -47,28 +48,34 @@ let waitForEnsRainbowToBeReadyPromise: Promise<void> | undefined; | |||||
| * This error will trigger termination of the ENSIndexer process. | ||||||
| */ | ||||||
| export function waitForEnsRainbowToBeReady(): Promise<void> { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Please note: There's many places in this file where the word "healthy" is written. I assume every single instance where this word is used should change to "ready". We should always write in a way that is maximally accurate. Here we are not checking the |
||||||
| if (waitForEnsRainbowToBeReadyPromise) { | ||||||
| return waitForEnsRainbowToBeReadyPromise; | ||||||
| } | ||||||
| if (waitForEnsRainbowToBeReadyPromise) return waitForEnsRainbowToBeReadyPromise; | ||||||
|
|
||||||
| logger.info({ | ||||||
| msg: `Waiting for ENSRainbow instance to be ready`, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ensRainbowInstance: ensRainbowUrl.href, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| }); | ||||||
|
|
||||||
| const retryInterval: Duration = 5; | ||||||
| const retryIntervalMs = secondsToMilliseconds(retryInterval); | ||||||
| const retriesPerMinute = 60 / retryInterval; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
import from |
||||||
|
|
||||||
| waitForEnsRainbowToBeReadyPromise = pRetry(async () => ensRainbowClient.health(), { | ||||||
| retries: 60, // This allows for a total of over 1 hour of retries with 1 minute between attempts. | ||||||
| minTimeout: secondsToMilliseconds(60), | ||||||
| maxTimeout: secondsToMilliseconds(60), | ||||||
| retries: retriesPerMinute * 60, // This allows for a total of over 1 hour of retries with `retryInterval` between attempts. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| minTimeout: retryIntervalMs, | ||||||
| maxTimeout: retryIntervalMs, | ||||||
| onFailedAttempt: ({ error, attemptNumber, retriesLeft }) => { | ||||||
| logger.warn({ | ||||||
| msg: `ENSRainbow health check failed`, | ||||||
| attempt: attemptNumber, | ||||||
| retriesLeft, | ||||||
| error: retriesLeft === 0 ? error : undefined, | ||||||
| ensRainbowInstance: ensRainbowUrl.href, | ||||||
| advice: `This might be due to ENSRainbow having a cold start, which can take 30+ minutes.`, | ||||||
| }); | ||||||
| // Log once every minute to avoid excessive logging during ENSRainbow cold start, | ||||||
| // while still providing visibility into the retry process. | ||||||
| if (attemptNumber % 12 === 0) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| logger.warn({ | ||||||
| msg: `ENSRainbow health check failed`, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| attempt: attemptNumber, | ||||||
| retriesLeft, | ||||||
| error: retriesLeft === 0 ? error : undefined, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do this? Why not always display the error, even if Then, in the case of the other special final error log statement after all retries have failed, we might not log any Please ignore if a good reason. |
||||||
| ensRainbowInstance: ensRainbowUrl.href, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| advice: `This might be due to ENSRainbow having a cold start, which can take 30+ minutes.`, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| }); | ||||||
| } | ||||||
| }, | ||||||
| }) | ||||||
| .then(() => { | ||||||
|
|
@@ -78,18 +85,13 @@ export function waitForEnsRainbowToBeReady(): Promise<void> { | |||||
| }); | ||||||
| }) | ||||||
| .catch((error) => { | ||||||
| const errorMessage = error instanceof Error ? error.message : "Unknown error"; | ||||||
|
|
||||||
| logger.error({ | ||||||
| msg: `ENSRainbow health check failed after multiple attempts`, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| error, | ||||||
| ensRainbowInstance: ensRainbowUrl.href, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| }); | ||||||
|
|
||||||
| // Throw the error to terminate the ENSIndexer process due to the failed health check of a critical dependency | ||||||
| throw new Error(errorMessage, { | ||||||
| cause: error instanceof Error ? error : undefined, | ||||||
| }); | ||||||
| throw error; | ||||||
| }); | ||||||
|
|
||||||
| return waitForEnsRainbowToBeReadyPromise; | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not confident it's right to completely remove this logic.
My worry is: how do we tell the difference between the following situations:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omnichain status is "unstarted" if an only if no indexing has started. This is related to the fact that the omnichain status can only be "unstarted" if and only if all chains are queued. And a chain is queued if and only if its
config.startBlockis equal to itscheckpointBlock(checkpointBlockis sourced from Ponder Indexing Status, which is sourced from_ponder_checkpointtable in the ENSIndexer Schema).If indexing has started and ENSIndexer has written any indexed data into the ENSIndexer Schema, the
checkpointBlockfor some indexed chain has also been stored in the_ponder_checkpointtable in the ENSIndexer Schema in ENSDb. Therefore, if ENSIndexer instance restarts, some of the indexed chains will not be "queued" anyomore ascheckpointBlockwill be ahead ofconfig.startBlockfor that chain. That leads us to the fact that the omnichain status cannot be "unstarted" anymore, and goes straight to "backfill".