diff --git a/graph/src/components/store/traits.rs b/graph/src/components/store/traits.rs index cec711b47a3..223bfe55b77 100644 --- a/graph/src/components/store/traits.rs +++ b/graph/src/components/store/traits.rs @@ -401,8 +401,7 @@ pub trait QueryStore: Send + Sync { fn wait_stats(&self) -> Result; - /// If `block` is `None`, assumes the latest block. - async fn has_non_fatal_errors(&self, block: Option) -> Result; + async fn has_deterministic_errors(&self, block: BlockNumber) -> Result; /// Find the current state for the subgraph deployment `id` and /// return details about it needed for executing queries diff --git a/graphql/src/store/resolver.rs b/graphql/src/store/resolver.rs index 747315bc84f..61970c34184 100644 --- a/graphql/src/store/resolver.rs +++ b/graphql/src/store/resolver.rs @@ -79,7 +79,7 @@ impl StoreResolver { let block_ptr = Self::locate_block(store_clone.as_ref(), bc, state).await?; let has_non_fatal_errors = store - .has_non_fatal_errors(Some(block_ptr.block_number())) + .has_deterministic_errors(block_ptr.block_number()) .await?; let resolver = StoreResolver { diff --git a/store/postgres/src/deployment.rs b/store/postgres/src/deployment.rs index 9c5b9b4bb22..68435c536a2 100644 --- a/store/postgres/src/deployment.rs +++ b/store/postgres/src/deployment.rs @@ -652,39 +652,19 @@ pub fn fail( } /// If `block` is `None`, assumes the latest block. -pub(crate) fn has_non_fatal_errors( +pub(crate) fn has_deterministic_errors( conn: &PgConnection, id: &DeploymentHash, - block: Option, + block: BlockNumber, ) -> Result { - use subgraph_deployment as d; use subgraph_error as e; - - match block { - Some(block) => select(diesel::dsl::exists( - e::table - .filter(e::subgraph_id.eq(id.as_str())) - .filter(e::deterministic) - .filter(sql("block_range @> ").bind::(block)), - )) - .get_result(conn), - None => select(diesel::dsl::exists( - e::table - .filter(e::subgraph_id.eq(id.as_str())) - .filter(e::deterministic) - .filter( - sql("block_range @> ") - .bind( - d::table - .filter(d::deployment.eq(id.as_str())) - .select(d::latest_ethereum_block_number) - .single_value(), - ) - .sql("::int"), - ), - )) - .get_result(conn), - } + select(diesel::dsl::exists( + e::table + .filter(e::subgraph_id.eq(id.as_str())) + .filter(e::deterministic) + .filter(sql("block_range @> ").bind::(block)), + )) + .get_result(conn) .map_err(|e| e.into()) } @@ -732,7 +712,7 @@ pub(crate) fn error_count(conn: &PgConnection, id: &DeploymentHash) -> Result Result<(), StoreError> { use subgraph_deployment as d; - let has_errors = has_non_fatal_errors(conn, id, Some(block))?; + let has_errors = has_deterministic_errors(conn, id, block)?; let (new, old) = match has_errors { true => (SubgraphHealth::Unhealthy, SubgraphHealth::Healthy), diff --git a/store/postgres/src/deployment_store.rs b/store/postgres/src/deployment_store.rs index 5710c0f0d9b..1fb86666afa 100644 --- a/store/postgres/src/deployment_store.rs +++ b/store/postgres/src/deployment_store.rs @@ -1337,7 +1337,7 @@ impl DeploymentStore { use deployment::SubgraphHealth::*; // Decide status based on if there are any errors for the previous/parent block let prev_health = - if deployment::has_non_fatal_errors(conn, deployment_id, Some(parent_ptr.number))? { + if deployment::has_deterministic_errors(conn, deployment_id, parent_ptr.number)? { Unhealthy } else { Healthy diff --git a/store/postgres/src/query_store.rs b/store/postgres/src/query_store.rs index f03c82cd551..d6ec59e5fb5 100644 --- a/store/postgres/src/query_store.rs +++ b/store/postgres/src/query_store.rs @@ -85,11 +85,11 @@ impl QueryStoreTrait for QueryStore { self.store.wait_stats(self.replica_id) } - async fn has_non_fatal_errors(&self, block: Option) -> Result { + async fn has_deterministic_errors(&self, block: BlockNumber) -> Result { let id = self.site.deployment.clone(); self.store .with_conn(move |conn, _| { - crate::deployment::has_non_fatal_errors(conn, &id, block).map_err(|e| e.into()) + crate::deployment::has_deterministic_errors(conn, &id, block).map_err(|e| e.into()) }) .await } diff --git a/store/postgres/tests/subgraph.rs b/store/postgres/tests/subgraph.rs index 781f419f919..68b0bf4e03e 100644 --- a/store/postgres/tests/subgraph.rs +++ b/store/postgres/tests/subgraph.rs @@ -1,10 +1,11 @@ use graph::{ components::{ server::index_node::VersionInfo, - store::{DeploymentLocator, StatusStore}, + store::{DeploymentId, DeploymentLocator, StatusStore}, }, data::subgraph::schema::SubgraphHealth, data::subgraph::schema::{DeploymentCreate, SubgraphError}, + prelude::BlockPtr, prelude::EntityChange, prelude::EntityChangeOperation, prelude::QueryStoreManager, @@ -51,6 +52,17 @@ fn get_version_info(store: &Store, subgraph_name: &str) -> VersionInfo { store.version_info(¤t).unwrap() } +async fn latest_block(store: &Store, deployment_id: DeploymentId) -> BlockPtr { + store + .subgraph_store() + .writable(LOGGER.clone(), deployment_id) + .await + .expect("can get writable") + .block_ptr() + .await + .unwrap() +} + #[test] fn reassign_subgraph() { async fn setup() -> DeploymentLocator { @@ -544,6 +556,16 @@ fn fatal_vs_non_fatal() { .await .unwrap(); + // Just to make latest_ethereum_block_number be 0 + transact_and_wait( + &store.subgraph_store(), + &deployment, + BLOCKS[0].clone(), + vec![], + ) + .await + .unwrap(); + let error = || SubgraphError { subgraph_id: deployment.hash.clone(), message: "test".to_string(), @@ -561,13 +583,19 @@ fn fatal_vs_non_fatal() { .await .unwrap(); - assert!(!query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(!query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); transact_errors(&store, &deployment, BLOCKS[1].clone(), vec![error()]) .await .unwrap(); - assert!(query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); }) } @@ -600,7 +628,10 @@ fn fail_unfail_deterministic_error() { .unwrap(); // We don't have any errors and the subgraph is healthy. - assert!(!query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(!query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); let vi = get_version_info(&store, NAME); assert_eq!(&*NAME, vi.deployment_id.as_str()); assert_eq!(false, vi.failed); @@ -617,7 +648,10 @@ fn fail_unfail_deterministic_error() { .unwrap(); // Still no fatal errors. - assert!(!query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(!query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); let vi = get_version_info(&store, NAME); assert_eq!(&*NAME, vi.deployment_id.as_str()); assert_eq!(false, vi.failed); @@ -641,7 +675,10 @@ fn fail_unfail_deterministic_error() { writable.fail_subgraph(error).await.unwrap(); // Now we have a fatal error because the subgraph failed. - assert!(query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); let vi = get_version_info(&store, NAME); assert_eq!(&*NAME, vi.deployment_id.as_str()); assert_eq!(true, vi.failed); @@ -655,7 +692,10 @@ fn fail_unfail_deterministic_error() { // We don't have fatal errors anymore and the block got reverted. assert_eq!(outcome, UnfailOutcome::Unfailed); - assert!(!query_store.has_non_fatal_errors(None).await.unwrap()); + assert!(!query_store + .has_deterministic_errors(latest_block(&store, deployment.id).await.number) + .await + .unwrap()); let vi = get_version_info(&store, NAME); assert_eq!(&*NAME, vi.deployment_id.as_str()); assert_eq!(false, vi.failed);