-
Notifications
You must be signed in to change notification settings - Fork 987
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
Make batching conditional on caught-up status #5252
Conversation
This takes a different approach to PR #5198. Importantly, it will properly disable batching if the subgraph is caught up with the chain, regardless of the 'synced' flag status. It separates the related concepts of synced and caught-up, and moves handling that into transact_block_operations`.
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.
LGTM. I am a little concerned whether we will turn batching on too often, and users will think their subgraph is stuck, though I think it won't be observable by users, but we need to keep an eye out for that.
@@ -338,16 +338,15 @@ pub trait WritableStore: ReadStore + DeploymentCursorTracker { | |||
deterministic_errors: Vec<SubgraphError>, | |||
offchain_to_remove: Vec<StoredDynamicDataSource>, | |||
is_non_fatal_errors_active: bool, | |||
is_caught_up_with_chain_head: bool, |
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.
Maybe worth commenting what behavior this triggers
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.
Documented
So that we don't flip-flop batching too often, and so that we don't have to think too hard about off-by-one situations and small latency lags affecting this, the subgraph is now considered synced if it's 10 blocks behind chain head, rather than just 1 block. |
This takes a different approach to PR #5198. Importantly, it will properly disable batching if the subgraph is caught up with the chain, regardless of the 'synced' flag status, fixing a regression.
It separates the related concepts of synced and caught-up, and moves handling that into transact_block_operations`. This nicely allows us to resume batching once caught up.
Resolves #5251.