chain,graph,store: added code to properly resume from a subgraph block ptr and no firehose cursor by maoueh · Pull Request #3174 · graphprotocol/graph-node

maoueh · 2022-01-19T21:18:50Z

Firehose is now using the subgraph ptr if no cursor is found. To ensure we do not create inconsistency in the data due to how Firehose resolves current block number, a sanity check has now been added. This check ensure that the first received block's parent from firehose is the same as the subgraph pointer block. If it's not the case, we revert to the last know final block so we are sure to start from a correct chain segment.

What final block to pick is decided by the chain itself. On Ethereum, we use block.number - reorg_threshold (guarded) while on NEAR, we use the last final block height defined by the block itself.

Missing/To Change

I pushed the PR to get an overall feeling of the idea and get early inputs.

Right now I'm using revert_block_operations but this is wrong as it's able to only revert a single block. I need in fact probably to rewind the subgraph but I would like to get advise here on how to do that properly and if there is any risks or stuff I missed with the approach.

Ping @lutter to get your input about the rewind thing above.

Draft

This is based on #3133 for now, so I would like to get the other merged first and rebased + finish this one after.

chain/ethereum/src/chain.rs

maoueh · 2022-01-21T21:06:03Z

This has been updated with the newest discussion we had, and some other improvements:

revert_block_operations can now revert more than 1 block at a time (but still asserts that reverts goes backward)
The chain continuity is now checked inside FirehoseBlockStream directly
Final block from Firehose is actually is now faster and will actually be able to return a response much faster by levering the stop_block_num
Cursor is now automatically deleted when starting an RPC subgraph (so that the case Firehose -> RPC -> Firehose works properly)
NEAR has now a working block_ptr_for_number.

This was tested and worked (still need to validate the POI of a subgraph synced straight with RPC and when switching from RPC to Firehose and the like):

RPC -> Firehose
RPC -> Firehose -> RPC
Firehose -> RPC

Ready for review and merge.

maoueh · 2022-01-25T00:37:37Z

I have fully tested that resuming from RPC subgraph block then falling back to Firehose on a forked block produces the same POI for a few blocks after the forked block in question.

I've fixed also an edge case where I was allowing to revert back behind manifest's defined start block.

This is ready for a final review and merge.

chain/ethereum/src/ethereum_adapter.rs

chain/ethereum/src/chain.rs

core/src/subgraph/instance_manager.rs

leoyvens · 2022-02-11T20:37:42Z

core/src/subgraph/instance_manager.rs

+    let mut backoff = ExponentialBackoff::new(30 * SECOND, *SUBGRAPH_ERROR_RETRY_CEIL_SECS);
+
+    loop {
+        match store.delete_block_cursor() {


DB operations are blocking and need to be asyncified so they can be used in an async context, you need to use with_conn within delete_block_cursor to asyncify it.

This is a copy of other existing methods in the trait like block_ptr, block_cursor and many others. Is the new paradigm to always use async from now on?

I would like to perform that in another PR mainly because this is blocking the integration test workflow and it would be nice to unlock it.

leoyvens · 2022-02-11T20:42:15Z

core/src/subgraph/instance_manager.rs


+async fn delete_subgraph_firehose_cursor(logger: &Logger, store: &dyn WritableStore) {
+    debug!(logger, "Deleting any existing Firehose cursor");
+    let mut backoff = ExponentialBackoff::new(30 * SECOND, *SUBGRAPH_ERROR_RETRY_CEIL_SECS);


This retry looks a lot like

graph-node/store/postgres/src/writable.rs

Line 108 in ebc6f65

async fn retry_async<T, F, Fut>(&self, op: &str, f: F) -> Result<T, StoreError>

, you could probably extract a reusable fn.

I agree, in the FirehoseBlockIngestor there is a bunch of those also so it would be a good thing to have a re-usable method. Again, would like to postpone that to another PR.

graph/src/blockchain/firehose_block_stream.rs

leoyvens · 2022-02-11T20:55:45Z

store/postgres/src/deployment.rs

+        .map_err(|e| e.into())
+}
+
 pub fn get_subgraph_firehose_cursor(


It looks like this also needs to be asyncified.

This is not part of this PR, but I would include that in an upcoming PR that would asyncinfy the various methods found in the WritableStore trait.

store/postgres/src/deployment_store.rs

…k ptr and no firehose cursor Firehose is now using the subgraph ptr if no cursor is found. To ensure we do not create inconsistency in the data due to how Firehose resolves current block number, a sanity check has now been added. This check ensure that the first received block's parent from firehose is the same as the subgraph pointer block. If it's not the case, we revert to the last know final block so we are sure to start from a correct chain segment. What final block to pick is decided by the chain itself. On Ethereum, we use `block.number - reorg_threshold` (guarded) while on NEAR, we use the last final block height defined by the block itself.

…t block

sduchesneau · 2022-02-28T20:26:43Z

@otaviopace I just rebased, then double-checked this PR and the behavior is as-expected. Would you kindly go ahead and merge this into master ?

maoueh requested a review from lutter January 19, 2022 21:18

maoueh force-pushed the maoueh/feature/firehose-resume-from-rpc-synced-subgraph branch from fcf443c to 49f5815 Compare January 19, 2022 21:20

leoyvens reviewed Jan 19, 2022

View reviewed changes

chain/ethereum/src/chain.rs Outdated Show resolved Hide resolved

Base automatically changed from maoueh/fix/firehose-save-cursor-on-revert to master January 20, 2022 19:09

maoueh force-pushed the maoueh/feature/firehose-resume-from-rpc-synced-subgraph branch 2 times, most recently from ee6aaa2 to 73ba5f6 Compare January 21, 2022 21:00

maoueh commented Feb 1, 2022

View reviewed changes

chain/ethereum/src/ethereum_adapter.rs Outdated Show resolved Hide resolved

maoueh mentioned this pull request Feb 1, 2022

First working implementation of Solana #3210

Closed

leoyvens reviewed Feb 11, 2022

View reviewed changes

maoueh force-pushed the maoueh/feature/firehose-resume-from-rpc-synced-subgraph branch from b4540a4 to 6d1b8ba Compare February 24, 2022 15:44

leoyvens approved these changes Feb 24, 2022

View reviewed changes

maoueh mentioned this pull request Feb 28, 2022

store: Reduce number of queries to update subgraph_deployment #3290

Merged

maoueh added 4 commits February 28, 2022 15:20

Fixed edge case where we were reverting past the actual manifest star…

5ea1f82

…t block

Updated Ethereum firehose example

835683c

PR review

5d8c4f1

sduchesneau force-pushed the maoueh/feature/firehose-resume-from-rpc-synced-subgraph branch from 6d1b8ba to 5d8c4f1 Compare February 28, 2022 20:23

leoyvens merged commit ae1d808 into master Mar 3, 2022

leoyvens deleted the maoueh/feature/firehose-resume-from-rpc-synced-subgraph branch March 3, 2022 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chain,graph,store: added code to properly resume from a subgraph block ptr and no firehose cursor#3174

chain,graph,store: added code to properly resume from a subgraph block ptr and no firehose cursor#3174
leoyvens merged 4 commits intomasterfrom
maoueh/feature/firehose-resume-from-rpc-synced-subgraph

maoueh commented Jan 19, 2022

Uh oh!

Uh oh!

maoueh commented Jan 21, 2022

Uh oh!

maoueh commented Jan 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leoyvens Feb 11, 2022

Uh oh!

maoueh Feb 24, 2022

Uh oh!

leoyvens Feb 11, 2022

Uh oh!

maoueh Feb 24, 2022

Uh oh!

Uh oh!

Uh oh!

leoyvens Feb 11, 2022

Uh oh!

maoueh Feb 24, 2022

Uh oh!

Uh oh!

sduchesneau commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

maoueh commented Jan 19, 2022

Missing/To Change

Draft

Uh oh!

Uh oh!

maoueh commented Jan 21, 2022

Uh oh!

maoueh commented Jan 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leoyvens Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

maoueh Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

leoyvens Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

maoueh Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

leoyvens Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

maoueh Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sduchesneau commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants