-
Notifications
You must be signed in to change notification settings - Fork 3
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: Ensure Indexer Config Updates Are Read in Runner #384
Conversation
if (isHistorical) { | ||
METRICS.LAST_PROCESSED_BLOCK_HEIGHT.labels({ indexer: indexerName, type: workerContext.streamType }).set(block.blockHeight); | ||
} | ||
METRICS.LAST_PROCESSED_BLOCK_HEIGHT.labels({ indexer: indexerName, type: workerContext.streamType }).set(block.blockHeight); |
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.
Is this metric still needed now that we use UNPROCESSED_STREAM_MESSAGES
to calculate BPS?
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 actually kept it in for a different thing I planned to tackle. We have alarms for social feed lag that still point to V1. Those are constantly firing. But, that did remind me that we probably want to have a last processed block metric for indexers so that we can use them for alarming for production indexers. I haven't researched it yet, but I figure if its already in grafana, getting an alarm set up shouldn't be too hard.
runner/src/stream-handler/worker.ts
Outdated
const functions = { | ||
let indexerConfig = await workerContext.redisClient.getStreamStorage(streamKey); | ||
let indexerName = `${indexerConfig.account_id}/${indexerConfig.function_name}`; | ||
let functions = { |
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.
This doesn't need to be defined in this scope, or necessarily be mutable. Should we move the declaration to the assignment below?
Runner sets Indexer Config when the thread is started. As a result, it does not react to updates to that config, such as updates to code. This is a problem as that means unless runner is restarted, published code changes won't be used. I've changed it so that the config is read each iteration of the loop. That way, config updates will be consumed. In the short term, this can be tuned such as reading every X loops and on every failure, if need be. In the long term, improved communication between coordinator and runner can facilitate coordinator communicating to runner to read the config as opposed to doing so all the time.
In addition, the metrics for block wait duration and overall execution duration were wrong. I've moved the start time to the correct spot.