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.

Sign up for GitHub

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

Jump to bottom

Stop subgraphs passing max endBlock #5583

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Stop subgraphs passing max endBlock #5583

merged 3 commits into from
Nov 14, 2024

Conversation

shuaibbapputty
Copy link
Contributor

@shuaibbapputty shuaibbapputty commented Aug 3, 2024 •
edited
Loading

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.😊

@fordN fordN requested a review from incrypto32 August 8, 2024 15:53
@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 😄

incrypto32
incrypto32 requested changes Nov 12, 2024
View reviewed changes
core/src/subgraph/instance_manager.rs
@@ -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?

core/src/subgraph/instance_manager.rs Outdated
@@ -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!

incrypto32
incrypto32 approved these changes Nov 14, 2024
View reviewed changes
core/src/subgraph/instance_manager.rs Outdated Show resolved Hide resolved
@shuaibbapputty @incrypto32
Update comment
adfa0b0 
Co-authored-by: Krishnanand V P <[email protected]>
@incrypto32 incrypto32 merged commit c1cee73 into graphprotocol:master Nov 14, 2024
6 checks passed
@shuaibbapputty shuaibbapputty deleted the shuaib/handle-max-endblock branch November 17, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@incrypto32 incrypto32 incrypto32 approved these changes

Assignees

@shuaibbapputty shuaibbapputty

Labels
None yet
Projects
None yet
Milestone
No milestone
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
@shuaibbapputty @incrypto32 @alex-pakalniskis