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

Stop subgraphs passing max endBlock #5583

Merged

Conversation

shuaibbapputty
Copy link
Contributor

@shuaibbapputty shuaibbapputty commented Aug 3, 2024

Closes #5535.

Previously, subgraphs continued scanning even when all the data sources had passed the end block.

This update finds the maximum end block across the data sources and stops the subgraph from scanning once the max_end_block is reached.

Aug 03 02:51:00.634 DEBG Starting or restarting subgraph, sgd: 8, test: end_block, subgraph_id: QmPL5J13p2652oit5umZZPQiNAvfV4E2KXkLNayguL9cJd
Aug 03 02:51:00.634 DEBG Starting block stream, sgd: 8, test: end_block, subgraph_id: QmPL5J13p2652oit5umZZPQiNAvfV4E2KXkLNayguL9cJd
Aug 03 02:51:00.635 INFO Stopping subgraph as we reached maximum endBlock, current_block: 9, max_end_block: 8, sgd: 8, test: end_block, subgraph_id: QmPL5J13p2652oit5umZZPQiNAvfV4E2KXkLNayguL9cJd

@shuaibbapputty
Copy link
Contributor Author

@fordN @leoyvens would appreciate a review on this.😊

@incrypto32
Copy link
Member

Nice! I think it'd be a bit more cleaner if we just set the exisiting stop_block value instead of creating a new max_end_block And also stop the subgraph on start up by checking stop block early on, just like you did for max_end_block

@alex-pakalniskis
Copy link

@shuaibbapputty bump 😄

@@ -419,6 +426,7 @@ impl<S: SubgraphStore> SubgraphInstanceManager<S> {
start_blocks,
end_blocks,
stop_block,
max_end_block,
Copy link
Member

Choose a reason for hiding this comment

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

We already have a stop_block, why not set it instead of a separate max_end_block?

@@ -331,6 +331,13 @@ impl<S: SubgraphStore> SubgraphInstanceManager<S> {
})
.collect();

let max_end_block: Option<BlockNumber> = if manifest.data_sources.len() == end_blocks.len()
Copy link
Member

Choose a reason for hiding this comment

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

A subgraph can also have offchain data sources, so we cant just compare it like this since offchain datasources dont have an end block

@shuaibbapputty
Copy link
Contributor Author

Sorry for the delay @alex-pakalniskis

As per my discussion with @incrypto32 on discord, I will address the review changes and update the PR asap. Thankyou!

core/src/subgraph/instance_manager.rs Outdated Show resolved Hide resolved
Co-authored-by: Krishnanand V P <[email protected]>
@incrypto32 incrypto32 merged commit c1cee73 into graphprotocol:master Nov 14, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Subgraph with endBlock on all datasources keeps synching with unfiltered firehose request
3 participants