From c0a89fd353dccad5ad82d39bf72d25c32d21b2ec Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Thu, 30 May 2024 07:46:29 +1200 Subject: [PATCH 01/32] Revert "chore: Remove notes" This reverts commit b25685b35dda631cf213c97193f7d61b9a3fbad2. --- TODO.md | 7 +++++++ coordinator/src/indexer_state.rs | 3 +++ coordinator/src/main.rs | 15 +++++++++++++++ runner/protos/data-layer.proto | 14 ++++++++++++++ runner/src/indexer/indexer.ts | 3 +++ 5 files changed, 42 insertions(+) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 000000000..24838d516 --- /dev/null +++ b/TODO.md @@ -0,0 +1,7 @@ +- Implement `DataLayerService` gRPC, to provide control over provisioning, ignoring de-provisioning for now +- Remove implicit provisioning from execution +- Refactor Coordinator executor/block_stream synchronization so that it is a single stateful process? +- Integrate `DataLayerService` in to Coordinator and add synchronization step to manage this +- Add gRPC method to deprovision data layer +- Refactor Coordinator synchronization so that we capture indexer "removals" +- Add de-provisioning to Coordinator synchronization diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index a8d1eddb9..8d3fd9643 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -18,6 +18,8 @@ struct OldIndexerState { block_stream_synced_at: Option, } +// NOTE We'll need to add more fields here - is there a way to gracefully handle non-existant +// fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct IndexerState { pub block_stream_synced_at: Option, @@ -42,6 +44,7 @@ pub struct IndexerStateManagerImpl { redis_client: RedisClient, } +// NOTE we probably need a "list" method, which means storing all state ids in a Redis set #[cfg_attr(test, mockall::automock)] impl IndexerStateManagerImpl { pub fn new(redis_client: RedisClient) -> Self { diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index f19d5e5e8..bdf732071 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -68,11 +68,26 @@ async fn main() -> anyhow::Result<()> { .migrate_state_if_needed(&indexer_registry) .await?; + // NOTE Rather than filtering them here, we can pass `IndexerState` to the sync methods, + // and let them decide what to do. That would be a bit cleaner? + // + // This will also allow us to determine when an Indexer has been deleted, rather than + // implicitly relying on the existance of executors/block_streams. This is going to be + // important for deprovisioning. let indexer_registry = indexer_state_manager .filter_disabled_indexers(&indexer_registry) .await?; tokio::try_join!( + // NOTE this may need to be regactored in to a combined "synchronise" function. + // The addition of DataLayer provisioning makes the process a bit more stateful, i.e. + // we need to do provisioning first, wait till it completes, and can then kick off + // executor/block_stream sync processes + // + // It's probably still helpful to encapsulate the block_stream/executor sync methods, + // as they are quite involved, but call them from an overall synchronise method + // + // We'll need to store the `ProvisioningStatus` in Redis, so we know when to poll synchronise_executors(&indexer_registry, &executors_handler), synchronise_block_streams( &indexer_registry, diff --git a/runner/protos/data-layer.proto b/runner/protos/data-layer.proto index eaef8034a..97445e01f 100644 --- a/runner/protos/data-layer.proto +++ b/runner/protos/data-layer.proto @@ -2,7 +2,19 @@ syntax = "proto3"; package data_layer; +// NOTE this will eventually be expanded to handle more granular operations +// such as truncating the database, or perhaps running migrations service DataLayer { + // NOTE As this process can take a while, we need to handle this asynchronously. + // Therefore, this will trigger the provisioning process and return immediately. + // The client can then poll the CheckProvisioningStatus method to determine when the + // provisioning process has completed. + // + // Maybe we should call this TriggerProvisioning instead of Provision? + // + // Need to figure out how this process will actually be kicked off asynchronously, + // can we just fire a promise or do we need worker threads? + // Provisions the data layer (PostgreSQL + Hasura) rpc Provision (ProvisionRequest) returns (ProvisionResponse); @@ -11,6 +23,8 @@ service DataLayer { } message ProvisionRequest { + // TODO This is only a partial `IndexerConfig`, which may pose an issue as + // all the provisioning methods expect a full `IndexerConfig` string account_id = 1; string function_name = 2; string schema = 3; diff --git a/runner/src/indexer/indexer.ts b/runner/src/indexer/indexer.ts index 344deae86..18f9f3497 100644 --- a/runner/src/indexer/indexer.ts +++ b/runner/src/indexer/indexer.ts @@ -48,6 +48,9 @@ const defaultConfig: Config = { hasuraEndpoint: process.env.HASURA_ENDPOINT, }; +// NOTE We need to remove provisioning from here, +// I'm thinking we remove knowledge of it entirely? and just inject he db parameters +// in from `StreamHandler`? Maybe a good opportunity to rename `StreamHandler` in to something more appropriate export default class Indexer { DEFAULT_HASURA_ROLE: string; IS_FIRST_EXECUTION: boolean = true; From a90e41620406d7dbb495af5487e6655d205328ae Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Thu, 30 May 2024 08:06:28 +1200 Subject: [PATCH 02/32] refactor: Remove unused migration code --- coordinator/src/indexer_config.rs | 4 - coordinator/src/indexer_state.rs | 280 ------------------------------ coordinator/src/main.rs | 11 +- coordinator/src/redis.rs | 25 --- 4 files changed, 7 insertions(+), 313 deletions(-) diff --git a/coordinator/src/indexer_config.rs b/coordinator/src/indexer_config.rs index a827c0f85..bcf043a05 100644 --- a/coordinator/src/indexer_config.rs +++ b/coordinator/src/indexer_config.rs @@ -26,10 +26,6 @@ impl IndexerConfig { format!("{}:last_published_block", self.get_full_name()) } - pub fn get_redis_stream_version_key(&self) -> String { - format!("{}:version", self.get_redis_stream_key()) - } - pub fn get_state_key(&self) -> String { format!("{}:state", self.get_full_name()) } diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 8d3fd9643..cd7b03599 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -13,11 +13,6 @@ pub enum SyncStatus { New, } -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -struct OldIndexerState { - block_stream_synced_at: Option, -} - // NOTE We'll need to add more fields here - is there a way to gracefully handle non-existant // fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -107,66 +102,6 @@ impl IndexerStateManagerImpl { Ok(filtered_registry) } - pub async fn migrate_state_if_needed( - &self, - indexer_registry: &IndexerRegistry, - ) -> anyhow::Result<()> { - if self.redis_client.is_migration_complete().await?.is_none() { - tracing::info!("Migrating indexer state"); - - for indexer_config in indexer_registry.iter() { - if let Some(version) = self.redis_client.get_stream_version(indexer_config).await? { - self.redis_client - .set_indexer_state( - indexer_config, - serde_json::to_string(&OldIndexerState { - block_stream_synced_at: Some(version), - })?, - ) - .await?; - } - } - - tracing::info!("Indexer state migration complete"); - - self.redis_client.set_migration_complete().await?; - } - - if self - .redis_client - .get::<_, bool>("state_migration:enabled_flag") - .await? - .is_none() - { - tracing::info!("Migrating enabled flag"); - - for indexer_config in indexer_registry.iter() { - let existing_state = self.redis_client.get_indexer_state(indexer_config).await?; - - let state = match existing_state { - Some(state) => { - let old_state: OldIndexerState = serde_json::from_str(&state)?; - IndexerState { - block_stream_synced_at: old_state.block_stream_synced_at, - enabled: true, - } - } - None => IndexerState::default(), - }; - - self.set_state(indexer_config, state).await?; - } - - self.redis_client - .set("state_migration:enabled_flag", true) - .await?; - - tracing::info!("Enabled flag migration complete"); - } - - Ok(()) - } - pub async fn get_block_stream_sync_status( &self, indexer_config: &IndexerConfig, @@ -288,221 +223,6 @@ mod tests { assert!(filtered_registry.contains_key(&morgs_config.account_id)); } - #[tokio::test] - async fn migrates_enabled_flag() { - let morgs_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Height(100), - }; - let darunrs_config = IndexerConfig { - account_id: "darunrs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: None, - start_block: StartBlock::Height(100), - }; - - let indexer_registry = IndexerRegistry::from(&[ - ( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), morgs_config.clone())]), - ), - ( - "darunrs.near".parse().unwrap(), - HashMap::from([("test".to_string(), darunrs_config.clone())]), - ), - ]); - - let mut mock_redis_client = RedisClient::default(); - mock_redis_client - .expect_is_migration_complete() - .returning(|| Ok(Some(true))) - .times(2); - mock_redis_client - .expect_get::<&str, bool>() - .with(predicate::eq("state_migration:enabled_flag")) - .returning(|_| Ok(None)) - .once(); - mock_redis_client - .expect_get::<&str, bool>() - .with(predicate::eq("state_migration:enabled_flag")) - .returning(|_| Ok(Some(true))) - .once(); - mock_redis_client - .expect_get_indexer_state() - .with(predicate::eq(morgs_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 200 }).to_string(), - )) - }) - .once(); - mock_redis_client - .expect_get_indexer_state() - .with(predicate::eq(darunrs_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 1 }).to_string(), - )) - }) - .once(); - mock_redis_client - .expect_set_indexer_state() - .with( - predicate::eq(morgs_config), - predicate::eq( - serde_json::json!({ "block_stream_synced_at": 200, "enabled": true }) - .to_string(), - ), - ) - .returning(|_, _| Ok(())) - .once(); - mock_redis_client - .expect_set_indexer_state() - .with( - predicate::eq(darunrs_config), - predicate::eq( - serde_json::json!({ "block_stream_synced_at": 1, "enabled": true }).to_string(), - ), - ) - .returning(|_, _| Ok(())) - .once(); - mock_redis_client - .expect_set::<&str, bool>() - .with( - predicate::eq("state_migration:enabled_flag"), - predicate::eq(true), - ) - .returning(|_, _| Ok(())) - .once(); - - let indexer_manager = IndexerStateManagerImpl::new(mock_redis_client); - - indexer_manager - .migrate_state_if_needed(&indexer_registry) - .await - .unwrap(); - - // ensure it is only called once - indexer_manager - .migrate_state_if_needed(&indexer_registry) - .await - .unwrap(); - } - - #[tokio::test] - async fn migrates_state_to_indexer_manager() { - let morgs_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Height(100), - }; - let darunrs_config = IndexerConfig { - account_id: "darunrs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: None, - start_block: StartBlock::Height(100), - }; - - let indexer_registry = IndexerRegistry(HashMap::from([ - ( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), morgs_config.clone())]), - ), - ( - "darunrs.near".parse().unwrap(), - HashMap::from([("test".to_string(), darunrs_config.clone())]), - ), - ])); - - let mut mock_redis_client = RedisClient::default(); - mock_redis_client - .expect_is_migration_complete() - .returning(|| Ok(None)) - .once(); - mock_redis_client - .expect_is_migration_complete() - .returning(|| Ok(Some(true))) - .once(); - mock_redis_client - .expect_get::<&str, _>() - .with(predicate::eq("state_migration:enabled_flag")) - .returning(|_| Ok(Some(true))); - mock_redis_client - .expect_set_migration_complete() - .returning(|| Ok(())) - .once(); - mock_redis_client - .expect_get_stream_version() - .with(predicate::eq(morgs_config.clone())) - .returning(|_| Ok(Some(200))) - .once(); - mock_redis_client - .expect_get_stream_version() - .with(predicate::eq(darunrs_config.clone())) - .returning(|_| Ok(Some(1))) - .once(); - mock_redis_client - .expect_set_indexer_state() - .with( - predicate::eq(morgs_config), - predicate::eq(serde_json::json!({ "block_stream_synced_at": 200 }).to_string()), - ) - .returning(|_, _| Ok(())) - .once(); - mock_redis_client - .expect_set_indexer_state() - .with( - predicate::eq(darunrs_config), - predicate::eq(serde_json::json!({ "block_stream_synced_at": 1 }).to_string()), - ) - .returning(|_, _| Ok(())) - .once(); - - let indexer_manager = IndexerStateManagerImpl::new(mock_redis_client); - - indexer_manager - .migrate_state_if_needed(&indexer_registry) - .await - .unwrap(); - - // ensure it is only called once - indexer_manager - .migrate_state_if_needed(&indexer_registry) - .await - .unwrap(); - } - #[tokio::test] pub async fn outdated_block_stream() { let indexer_config = IndexerConfig { diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index bdf732071..5f738c81a 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -63,10 +63,13 @@ async fn main() -> anyhow::Result<()> { loop { let indexer_registry = registry.fetch().await?; - - indexer_state_manager - .migrate_state_if_needed(&indexer_registry) - .await?; + // fetch state + // fetch executors + // fetch block streams + // + // iterate over the longest of state/registry + // pass Option of each to sync method + // sync method decides what to do // NOTE Rather than filtering them here, we can pass `IndexerState` to the sync methods, // and let them decide what to do. That would be a bit cleaner? diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index e750776f5..71d61cf03 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -73,14 +73,6 @@ impl RedisClientImpl { Ok(()) } - pub async fn get_stream_version( - &self, - indexer_config: &IndexerConfig, - ) -> anyhow::Result> { - self.get::<_, u64>(indexer_config.get_redis_stream_version_key()) - .await - } - pub async fn get_last_published_block( &self, indexer_config: &IndexerConfig, @@ -107,14 +99,6 @@ impl RedisClientImpl { ) -> anyhow::Result<()> { self.set(indexer_config.get_state_key(), state).await } - - pub async fn set_migration_complete(&self) -> anyhow::Result<()> { - self.set("indexer_manager_migration_complete", true).await - } - - pub async fn is_migration_complete(&self) -> anyhow::Result> { - self.get("indexer_manager_migration_complete").await - } } #[cfg(test)] @@ -130,11 +114,6 @@ mockall::mock! { state: String, ) -> anyhow::Result<()>; - pub async fn get_stream_version( - &self, - indexer_config: &IndexerConfig, - ) -> anyhow::Result>; - pub async fn get_last_published_block( &self, indexer_config: &IndexerConfig, @@ -142,10 +121,6 @@ mockall::mock! { pub async fn clear_block_stream(&self, indexer_config: &IndexerConfig) -> anyhow::Result<()>; - pub async fn set_migration_complete(&self) -> anyhow::Result<()>; - - pub async fn is_migration_complete(&self) -> anyhow::Result>; - pub async fn get(&self, key: T) -> anyhow::Result> where T: ToRedisArgs + Debug + Send + Sync + 'static, From 75c53dbce4f7b1a08ac646777a2fc2bb79f93b62 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Thu, 30 May 2024 15:19:29 +1200 Subject: [PATCH 03/32] feat: Create merged synchroniser --- coordinator/src/main.rs | 1 + coordinator/src/synchroniser.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 coordinator/src/synchroniser.rs diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 5f738c81a..80e640d63 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -18,6 +18,7 @@ mod indexer_state; mod redis; mod registry; mod server; +mod synchroniser; mod utils; const CONTROL_LOOP_THROTTLE_SECONDS: Duration = Duration::from_secs(1); diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs new file mode 100644 index 000000000..88fbf6260 --- /dev/null +++ b/coordinator/src/synchroniser.rs @@ -0,0 +1,30 @@ +use crate::{ + block_streams::BlockStreamsHandler, executors::ExecutorsHandler, + indexer_state::IndexerStateManager, redis::RedisClient, registry::Registry, +}; + +pub struct Synchroniser<'a> { + block_streams_handler: &'a BlockStreamsHandler, + executors_handler: &'a ExecutorsHandler, + registry: &'a Registry, + state_manager: &'a IndexerStateManager, + redis_client: &'a RedisClient, +} + +impl<'a> Synchroniser<'a> { + pub fn new( + block_streams_handler: &'a BlockStreamsHandler, + executors_handler: &'a ExecutorsHandler, + registry: &'a Registry, + state_manager: &'a IndexerStateManager, + redis_client: &'a RedisClient, + ) -> Self { + Self { + block_streams_handler, + executors_handler, + registry, + state_manager, + redis_client, + } + } +} From 44fb56d5d2413c78eb57d1775e07d91e1d4bd397 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Thu, 30 May 2024 16:27:58 +1200 Subject: [PATCH 04/32] feat: Start new indexer --- coordinator/src/indexer_config.rs | 19 +++++ coordinator/src/indexer_state.rs | 11 +++ coordinator/src/registry.rs | 9 ++ coordinator/src/synchroniser.rs | 135 +++++++++++++++++++++++++++++- 4 files changed, 172 insertions(+), 2 deletions(-) diff --git a/coordinator/src/indexer_config.rs b/coordinator/src/indexer_config.rs index bcf043a05..78220c76e 100644 --- a/coordinator/src/indexer_config.rs +++ b/coordinator/src/indexer_config.rs @@ -13,6 +13,25 @@ pub struct IndexerConfig { pub created_at_block_height: u64, } +#[cfg(test)] +impl Default for IndexerConfig { + fn default() -> Self { + Self { + account_id: "morgs.near".parse().unwrap(), + function_name: "test".to_string(), + code: "code".to_string(), + schema: "schema".to_string(), + rule: Rule::ActionAny { + affected_account_id: "queryapi.dataplatform.near".to_string(), + status: registry_types::Status::Any, + }, + created_at_block_height: 1, + updated_at_block_height: Some(2), + start_block: StartBlock::Height(100), + } + } +} + impl IndexerConfig { pub fn get_full_name(&self) -> String { format!("{}/{}", self.account_id, self.function_name) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index cd7b03599..af08bfc49 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -1,6 +1,8 @@ #![cfg_attr(test, allow(dead_code))] +use near_primitives::types::AccountId; use std::cmp::Ordering; +use std::str::FromStr; use crate::indexer_config::IndexerConfig; use crate::redis::RedisClient; @@ -17,6 +19,9 @@ pub enum SyncStatus { // fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct IndexerState { + // TODO need to migrate existing state + pub account_id: AccountId, + pub function_name: String, pub block_stream_synced_at: Option, pub enabled: bool, } @@ -24,6 +29,8 @@ pub struct IndexerState { impl Default for IndexerState { fn default() -> Self { Self { + account_id: AccountId::from_str("morgs.near").unwrap(), + function_name: String::new(), block_stream_synced_at: None, enabled: true, } @@ -140,6 +147,10 @@ impl IndexerStateManagerImpl { Ok(()) } + + pub async fn list(&self) -> anyhow::Result> { + todo!() + } } #[cfg(test)] diff --git a/coordinator/src/registry.rs b/coordinator/src/registry.rs index 8b09d8b10..a5b197589 100644 --- a/coordinator/src/registry.rs +++ b/coordinator/src/registry.rs @@ -14,6 +14,7 @@ use registry_types::AllIndexers; use crate::indexer_config::IndexerConfig; use crate::utils::exponential_retry; +#[derive(Clone)] pub struct IndexerRegistry(pub HashMap>); impl IndexerRegistry { @@ -32,6 +33,14 @@ impl IndexerRegistry { function_iter: None, } } + + pub fn get(&self, account_id: &AccountId, function_name: &str) -> Option<&IndexerConfig> { + self.0.get(account_id)?.get(function_name) + } + + pub fn remove(&mut self, account_id: &AccountId, function_name: &str) -> Option { + self.0.get_mut(account_id)?.remove(function_name) + } } pub struct IndexerRegistryIter<'a> { diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 88fbf6260..866c4e3e4 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -1,6 +1,12 @@ +use registry_types::StartBlock; + use crate::{ - block_streams::BlockStreamsHandler, executors::ExecutorsHandler, - indexer_state::IndexerStateManager, redis::RedisClient, registry::Registry, + block_streams::BlockStreamsHandler, + executors::ExecutorsHandler, + indexer_config::IndexerConfig, + indexer_state::IndexerStateManager, + redis::RedisClient, + registry::{IndexerRegistry, Registry}, }; pub struct Synchroniser<'a> { @@ -12,6 +18,7 @@ pub struct Synchroniser<'a> { } impl<'a> Synchroniser<'a> { + // TODO use builder? pub fn new( block_streams_handler: &'a BlockStreamsHandler, executors_handler: &'a ExecutorsHandler, @@ -27,4 +34,128 @@ impl<'a> Synchroniser<'a> { redis_client, } } + + pub async fn sync_block_stream() {} + pub async fn sync_executor() {} + + pub async fn handle_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { + self.executors_handler.start(config).await?; + + let start_block = match config.start_block { + StartBlock::Height(height) => height, + StartBlock::Latest => config.get_registry_version(), + StartBlock::Continue => { + tracing::warn!( + "Attempted to start new Block Stream with CONTINUE, using LATEST instead" + ); + config.get_registry_version() + } + }; + + self.block_streams_handler + .start(start_block, config) + .await?; + + Ok(()) + } + + pub async fn sync(&self) -> anyhow::Result<()> { + let states = self.state_manager.list().await?; + let mut registry = self.registry.fetch().await?; + // TODO get instead of list? + let executors = self.executors_handler.list().await?; + let block_streams = self.block_streams_handler.list().await?; + + for state in states { + let config = registry.get(&state.account_id, &state.function_name); + let executor = executors.iter().find(|e| { + e.account_id == state.account_id && e.function_name == state.function_name + }); + let block_stream = block_streams.iter().find(|b| { + b.account_id == state.account_id && b.function_name == state.function_name + }); + + if config.is_some() { + registry.remove(&state.account_id, &state.function_name); + // handle_existing() + } else { + // handle_deleted() + } + } + + for config in registry.iter() { + // shouldn't be any executor/block_stream + self.handle_new_indexer(config).await?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + use mockall::predicate::*; + use std::collections::HashMap; + + #[tokio::test] + async fn starts_new_indexer() { + let config1 = IndexerConfig::default(); + let config2 = IndexerConfig { + function_name: "test2".to_string(), + start_block: StartBlock::Latest, + ..Default::default() + }; + + let indexer_registry = IndexerRegistry::from(&[( + config1.account_id.clone(), + HashMap::from([ + (config1.function_name.clone(), config1.clone()), + (config2.function_name.clone(), config2.clone()), + ]), + )]); + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler.expect_list().returning(|| Ok(vec![])); + block_streams_handler + .expect_start() + .with(eq(100), eq(config1.clone())) + .returning(|_, _| Ok(())); + block_streams_handler + .expect_start() + .with(eq(config2.get_registry_version()), eq(config2.clone())) + .returning(|_, _| Ok(())); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler.expect_list().returning(|| Ok(vec![])); + executors_handler + .expect_start() + .with(eq(config1)) + .returning(|_| Ok(())); + executors_handler + .expect_start() + .with(eq(config2)) + .returning(|_| Ok(())); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + state_manager.expect_list().returning(|| Ok(vec![])); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync().await.unwrap(); + } } From 6703248515123c7a52d72d8bbea94e042216411d Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Fri, 31 May 2024 08:48:28 +1200 Subject: [PATCH 05/32] feat: Handle synced/outdated indexers --- coordinator/src/executors/handler.rs | 1 + .../src/server/indexer_manager_service.rs | 2 + coordinator/src/synchroniser.rs | 391 +++++++++++++++--- 3 files changed, 330 insertions(+), 64 deletions(-) diff --git a/coordinator/src/executors/handler.rs b/coordinator/src/executors/handler.rs index 68be45f29..fcbbb6a6b 100644 --- a/coordinator/src/executors/handler.rs +++ b/coordinator/src/executors/handler.rs @@ -32,6 +32,7 @@ impl ExecutorsHandlerImpl { } pub async fn list(&self) -> anyhow::Result> { + // TODO remove retry logic and just let it propogate up to the main loop exponential_retry(|| async { let response = self .client diff --git a/coordinator/src/server/indexer_manager_service.rs b/coordinator/src/server/indexer_manager_service.rs index 36a91ca31..412124a15 100644 --- a/coordinator/src/server/indexer_manager_service.rs +++ b/coordinator/src/server/indexer_manager_service.rs @@ -40,6 +40,8 @@ impl indexer_manager::indexer_manager_server::IndexerManager for IndexerManagerS .parse() .map_err(|_| Status::invalid_argument("Invalid account ID"))?; + // NOTE we probably need a way to write the state before publishing, so that we can trigger + // features before it starts let indexer_config = self .registry .fetch_indexer(account_id, request.function_name) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 866c4e3e4..c1b5bbee9 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -1,10 +1,13 @@ +// TODO re-export these from handler +use block_streamer::StreamInfo; use registry_types::StartBlock; +use runner::ExecutorInfo; use crate::{ block_streams::BlockStreamsHandler, executors::ExecutorsHandler, indexer_config::IndexerConfig, - indexer_state::IndexerStateManager, + indexer_state::{IndexerState, IndexerStateManager}, redis::RedisClient, registry::{IndexerRegistry, Registry}, }; @@ -35,10 +38,7 @@ impl<'a> Synchroniser<'a> { } } - pub async fn sync_block_stream() {} - pub async fn sync_executor() {} - - pub async fn handle_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { + async fn handle_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { self.executors_handler.start(config).await?; let start_block = match config.start_block { @@ -59,6 +59,108 @@ impl<'a> Synchroniser<'a> { Ok(()) } + async fn sync_executor( + &self, + config: &IndexerConfig, + executor: Option<&ExecutorInfo>, + ) -> anyhow::Result<()> { + if let Some(executor) = executor { + if executor.version == config.get_registry_version() { + return Ok(()); + } + + tracing::info!("Stopping outdated executor"); + + self.executors_handler + .stop(executor.executor_id.clone()) + .await?; + } + + tracing::info!("Starting new executor"); + + self.executors_handler.start(config).await?; + + Ok(()) + } + + async fn sync_block_stream( + &self, + config: &IndexerConfig, + state: &IndexerState, + block_stream: Option<&StreamInfo>, + ) -> anyhow::Result<()> { + if let Some(block_stream) = block_stream { + if block_stream.version == config.get_registry_version() { + return Ok(()); + } + + tracing::info!( + previous_version = block_stream.version, + "Stopping outdated block stream" + ); + + self.block_streams_handler + .stop(block_stream.stream_id.clone()) + .await?; + } + + //if let Some(previous_sync_height) = state.block_stream_synced_at { + // if previous_sync_height == config.get_registry_version() { + // return Ok(()); + // } + //} + + // TODO do we still need to check if it was previously synced? I think so because if block + // streamer restarts we dont want to accidently clear the stream + //if matches!( + // config.start_block, + // StartBlock::Latest | StartBlock::Height(..) + //) { + // self.redis_client.clear_block_stream(config).await?; + //} + + let height = match config.start_block { + StartBlock::Latest => Ok(config.get_registry_version()), + StartBlock::Height(height) => Ok(height), + StartBlock::Continue => self + .redis_client + .get_last_published_block(config) + .await? + .map(|height| height + 1) + .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")), + }?; + + //let sync_status = self + // .state_manager + // .get_block_stream_sync_status(config) + // .await?; + + //clear_block_stream_if_needed(&sync_status, indexer_config, redis_client).await?; + // + //let start_block_height = + // determine_start_block_height(&sync_status, indexer_config, redis_client).await?; + + self.block_streams_handler.start(height, config).await?; + + //self.state_manager.set_block_stream_synced(config).await?; + + Ok(()) + } + + async fn handle_existing_indexer( + &self, + config: &IndexerConfig, + // TODO handle disabled indexers + state: &IndexerState, + executor: Option<&ExecutorInfo>, + block_stream: Option<&StreamInfo>, + ) -> anyhow::Result<()> { + self.sync_executor(config, executor).await?; + self.sync_block_stream(config, state, block_stream).await?; + + Ok(()) + } + pub async fn sync(&self) -> anyhow::Result<()> { let states = self.state_manager.list().await?; let mut registry = self.registry.fetch().await?; @@ -67,7 +169,9 @@ impl<'a> Synchroniser<'a> { let block_streams = self.block_streams_handler.list().await?; for state in states { - let config = registry.get(&state.account_id, &state.function_name); + let config = registry + .get(&state.account_id, &state.function_name) + .cloned(); let executor = executors.iter().find(|e| { e.account_id == state.account_id && e.function_name == state.function_name }); @@ -75,8 +179,11 @@ impl<'a> Synchroniser<'a> { b.account_id == state.account_id && b.function_name == state.function_name }); - if config.is_some() { + if let Some(config) = config { registry.remove(&state.account_id, &state.function_name); + + self.handle_existing_indexer(&config, &state, executor, block_stream) + .await?; // handle_existing() } else { // handle_deleted() @@ -99,63 +206,219 @@ mod test { use mockall::predicate::*; use std::collections::HashMap; - #[tokio::test] - async fn starts_new_indexer() { - let config1 = IndexerConfig::default(); - let config2 = IndexerConfig { - function_name: "test2".to_string(), - start_block: StartBlock::Latest, - ..Default::default() - }; + mod new_indexer { + use super::*; + + #[tokio::test] + async fn start() { + let config1 = IndexerConfig::default(); + let config2 = IndexerConfig { + function_name: "test2".to_string(), + start_block: StartBlock::Latest, + ..Default::default() + }; + + let indexer_registry = IndexerRegistry::from(&[( + config1.account_id.clone(), + HashMap::from([ + (config1.function_name.clone(), config1.clone()), + (config2.function_name.clone(), config2.clone()), + ]), + )]); - let indexer_registry = IndexerRegistry::from(&[( - config1.account_id.clone(), - HashMap::from([ - (config1.function_name.clone(), config1.clone()), - (config2.function_name.clone(), config2.clone()), - ]), - )]); - - let mut block_streams_handler = BlockStreamsHandler::default(); - block_streams_handler.expect_list().returning(|| Ok(vec![])); - block_streams_handler - .expect_start() - .with(eq(100), eq(config1.clone())) - .returning(|_, _| Ok(())); - block_streams_handler - .expect_start() - .with(eq(config2.get_registry_version()), eq(config2.clone())) - .returning(|_, _| Ok(())); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler.expect_list().returning(|| Ok(vec![])); - executors_handler - .expect_start() - .with(eq(config1)) - .returning(|_| Ok(())); - executors_handler - .expect_start() - .with(eq(config2)) - .returning(|_| Ok(())); - - let mut registry = Registry::default(); - registry - .expect_fetch() - .returning(move || Ok(indexer_registry.clone())); - - let mut state_manager = IndexerStateManager::default(); - state_manager.expect_list().returning(|| Ok(vec![])); - - let redis_client = RedisClient::default(); - - let synchroniser = Synchroniser::new( - &block_streams_handler, - &executors_handler, - ®istry, - &state_manager, - &redis_client, - ); - - synchroniser.sync().await.unwrap(); + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler.expect_list().returning(|| Ok(vec![])); + block_streams_handler + .expect_start() + .with(eq(100), eq(config1.clone())) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with(eq(config2.get_registry_version()), eq(config2.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler.expect_list().returning(|| Ok(vec![])); + executors_handler + .expect_start() + .with(eq(config1)) + .returning(|_| Ok(())) + .once(); + executors_handler + .expect_start() + .with(eq(config2)) + .returning(|_| Ok(())) + .once(); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + state_manager.expect_list().returning(|| Ok(vec![])); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync().await.unwrap(); + } + } + + mod existing_indexer { + use super::*; + + #[tokio::test] + async fn ignores_synced() { + let config = IndexerConfig::default(); + + let indexer_registry = IndexerRegistry::from(&[( + config.account_id.clone(), + HashMap::from([(config.function_name.clone(), config.clone())]), + )]); + + let mut block_streams_handler = BlockStreamsHandler::default(); + let config_clone = config.clone(); + block_streams_handler.expect_list().returning(move || { + Ok(vec![StreamInfo { + stream_id: config_clone.get_redis_stream_key(), + account_id: config_clone.account_id.to_string(), + function_name: config_clone.function_name.clone(), + version: config_clone.get_registry_version(), + }]) + }); + block_streams_handler.expect_stop().never(); + block_streams_handler.expect_start().never(); + + let mut executors_handler = ExecutorsHandler::default(); + let config_clone = config.clone(); + executors_handler.expect_list().returning(move || { + Ok(vec![ExecutorInfo { + executor_id: "executor_id".to_string(), + account_id: config_clone.account_id.to_string(), + function_name: config_clone.function_name.clone(), + version: config_clone.get_registry_version(), + status: "running".to_string(), + }]) + }); + executors_handler.expect_stop().never(); + executors_handler.expect_start().never(); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + state_manager.expect_list().returning(move || { + Ok(vec![IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version()), + enabled: true, + }]) + }); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync().await.unwrap(); + } + + #[tokio::test] + async fn restarts_outdated() { + let config = IndexerConfig::default(); + + let indexer_registry = IndexerRegistry::from(&[( + config.account_id.clone(), + HashMap::from([(config.function_name.clone(), config.clone())]), + )]); + + let mut block_streams_handler = BlockStreamsHandler::default(); + let config_clone = config.clone(); + block_streams_handler.expect_list().returning(move || { + Ok(vec![StreamInfo { + stream_id: "stream_id".to_string(), + account_id: config_clone.account_id.to_string(), + function_name: config_clone.function_name.clone(), + version: config_clone.get_registry_version() + 1, + }]) + }); + block_streams_handler + .expect_stop() + .with(eq("stream_id".to_string())) + .returning(|_| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with(eq(100), eq(config.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + let config_clone = config.clone(); + executors_handler.expect_list().returning(move || { + Ok(vec![ExecutorInfo { + executor_id: "executor_id".to_string(), + account_id: config_clone.account_id.to_string(), + function_name: config_clone.function_name.clone(), + version: config_clone.get_registry_version() + 1, + status: "running".to_string(), + }]) + }); + executors_handler + .expect_stop() + .with(eq("executor_id".to_string())) + .returning(|_| Ok(())) + .once(); + executors_handler + .expect_start() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + state_manager.expect_list().returning(move || { + Ok(vec![IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version()), + enabled: true, + }]) + }); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync().await.unwrap(); + } } } From dec56d70989b969fd3b2817aaf3f3211453b54ba Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Fri, 31 May 2024 10:58:11 +1200 Subject: [PATCH 06/32] feat: Sync existing block streams --- coordinator/src/indexer_state.rs | 5 + coordinator/src/synchroniser.rs | 180 ++++++++++++++++++++++++------- 2 files changed, 145 insertions(+), 40 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index af08bfc49..72056446e 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -22,6 +22,11 @@ pub struct IndexerState { // TODO need to migrate existing state pub account_id: AccountId, pub function_name: String, + // TODO this no longer needs to be optional, because the existance of the state object implies + // that the stream has been started + // + // but we might need to write the state object before we register, so therefore may need this + // to be none pub block_stream_synced_at: Option, pub enabled: bool, } diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index c1b5bbee9..0ea36ad07 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -56,6 +56,8 @@ impl<'a> Synchroniser<'a> { .start(start_block, config) .await?; + // flush state + Ok(()) } @@ -83,6 +85,33 @@ impl<'a> Synchroniser<'a> { Ok(()) } + async fn get_continuation_block_height(&self, config: &IndexerConfig) -> anyhow::Result { + self.redis_client + .get_last_published_block(config) + .await? + .map(|height| height + 1) + .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")) + } + + async fn restart_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { + if matches!( + config.start_block, + StartBlock::Latest | StartBlock::Height(..) + ) { + self.redis_client.clear_block_stream(config).await?; + } + + let height = match config.start_block { + StartBlock::Latest => config.get_registry_version(), + StartBlock::Height(height) => height, + StartBlock::Continue => self.get_continuation_block_height(config).await?, + }; + + self.block_streams_handler.start(height, config).await?; + + Ok(()) + } + async fn sync_block_stream( &self, config: &IndexerConfig, @@ -102,47 +131,24 @@ impl<'a> Synchroniser<'a> { self.block_streams_handler .stop(block_stream.stream_id.clone()) .await?; + + self.restart_block_stream(config).await?; + + return Ok(()); } - //if let Some(previous_sync_height) = state.block_stream_synced_at { - // if previous_sync_height == config.get_registry_version() { - // return Ok(()); - // } - //} - - // TODO do we still need to check if it was previously synced? I think so because if block - // streamer restarts we dont want to accidently clear the stream - //if matches!( - // config.start_block, - // StartBlock::Latest | StartBlock::Height(..) - //) { - // self.redis_client.clear_block_stream(config).await?; - //} + // state was written before indexer was registered + if state.block_stream_synced_at.is_none() + || state.block_stream_synced_at.unwrap() != config.get_registry_version() + { + self.restart_block_stream(config).await?; - let height = match config.start_block { - StartBlock::Latest => Ok(config.get_registry_version()), - StartBlock::Height(height) => Ok(height), - StartBlock::Continue => self - .redis_client - .get_last_published_block(config) - .await? - .map(|height| height + 1) - .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")), - }?; - - //let sync_status = self - // .state_manager - // .get_block_stream_sync_status(config) - // .await?; - - //clear_block_stream_if_needed(&sync_status, indexer_config, redis_client).await?; - // - //let start_block_height = - // determine_start_block_height(&sync_status, indexer_config, redis_client).await?; + return Ok(()); + } - self.block_streams_handler.start(height, config).await?; + let height = self.get_continuation_block_height(config).await?; - //self.state_manager.set_block_stream_synced(config).await?; + self.block_streams_handler.start(height, config).await?; Ok(()) } @@ -155,9 +161,12 @@ impl<'a> Synchroniser<'a> { executor: Option<&ExecutorInfo>, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { + // TODO in parallel self.sync_executor(config, executor).await?; self.sync_block_stream(config, state, block_stream).await?; + // TODO flush state + Ok(()) } @@ -206,7 +215,7 @@ mod test { use mockall::predicate::*; use std::collections::HashMap; - mod new_indexer { + mod new { use super::*; #[tokio::test] @@ -274,7 +283,7 @@ mod test { } } - mod existing_indexer { + mod existing { use super::*; #[tokio::test] @@ -398,6 +407,13 @@ mod test { .expect_fetch() .returning(move || Ok(indexer_registry.clone())); + let mut redis_client = RedisClient::default(); + redis_client + .expect_clear_block_stream() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); + let mut state_manager = IndexerStateManager::default(); state_manager.expect_list().returning(move || { Ok(vec![IndexerState { @@ -408,8 +424,6 @@ mod test { }]) }); - let redis_client = RedisClient::default(); - let synchroniser = Synchroniser::new( &block_streams_handler, &executors_handler, @@ -420,5 +434,91 @@ mod test { synchroniser.sync().await.unwrap(); } + + #[tokio::test] + async fn restarts_stopped_and_outdated_block_stream() { + let config = IndexerConfig::default(); + let state = IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version() - 1), + enabled: true, + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with(eq(100), eq(config.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut redis_client = RedisClient::default(); + redis_client + .expect_clear_block_stream() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); + + let state_manager = IndexerStateManager::default(); + let executors_handler = ExecutorsHandler::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_block_stream(&config, &state, None) + .await + .unwrap(); + } + + #[tokio::test] + async fn resumes_stopped_and_synced_block_stream() { + let config = IndexerConfig::default(); + let state = IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version()), + enabled: true, + }; + + let last_published_block = 1; + + let mut redis_client = RedisClient::default(); + redis_client.expect_clear_block_stream().never(); + redis_client + .expect_get_last_published_block() + .with(eq(config.clone())) + .returning(move |_| Ok(Some(last_published_block))); + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with(eq(last_published_block + 1), eq(config.clone())) + .returning(|_, _| Ok(())) + .once(); + + let state_manager = IndexerStateManager::default(); + let executors_handler = ExecutorsHandler::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_block_stream(&config, &state, None) + .await + .unwrap(); + } } } From f6f14f982d5f210d387f3f35609584a61cbd08a2 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Fri, 31 May 2024 11:21:57 +1200 Subject: [PATCH 07/32] test: Test reconfiguration of block streams --- coordinator/src/synchroniser.rs | 111 ++++++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 14 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 0ea36ad07..927cf8b23 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -38,7 +38,7 @@ impl<'a> Synchroniser<'a> { } } - async fn handle_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { + async fn sync_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { self.executors_handler.start(config).await?; let start_block = match config.start_block { @@ -61,7 +61,7 @@ impl<'a> Synchroniser<'a> { Ok(()) } - async fn sync_executor( + async fn sync_existing_executor( &self, config: &IndexerConfig, executor: Option<&ExecutorInfo>, @@ -93,7 +93,7 @@ impl<'a> Synchroniser<'a> { .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")) } - async fn restart_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { + async fn reconfigure_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { if matches!( config.start_block, StartBlock::Latest | StartBlock::Height(..) @@ -112,7 +112,7 @@ impl<'a> Synchroniser<'a> { Ok(()) } - async fn sync_block_stream( + async fn sync_existing_block_stream( &self, config: &IndexerConfig, state: &IndexerState, @@ -132,16 +132,16 @@ impl<'a> Synchroniser<'a> { .stop(block_stream.stream_id.clone()) .await?; - self.restart_block_stream(config).await?; + self.reconfigure_block_stream(config).await?; return Ok(()); } - // state was written before indexer was registered + // TODO handle state which was written before indexer was registered if state.block_stream_synced_at.is_none() || state.block_stream_synced_at.unwrap() != config.get_registry_version() { - self.restart_block_stream(config).await?; + self.reconfigure_block_stream(config).await?; return Ok(()); } @@ -153,7 +153,7 @@ impl<'a> Synchroniser<'a> { Ok(()) } - async fn handle_existing_indexer( + async fn sync_existing_indexer( &self, config: &IndexerConfig, // TODO handle disabled indexers @@ -162,8 +162,9 @@ impl<'a> Synchroniser<'a> { block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { // TODO in parallel - self.sync_executor(config, executor).await?; - self.sync_block_stream(config, state, block_stream).await?; + self.sync_existing_executor(config, executor).await?; + self.sync_existing_block_stream(config, state, block_stream) + .await?; // TODO flush state @@ -191,7 +192,7 @@ impl<'a> Synchroniser<'a> { if let Some(config) = config { registry.remove(&state.account_id, &state.function_name); - self.handle_existing_indexer(&config, &state, executor, block_stream) + self.sync_existing_indexer(&config, &state, executor, block_stream) .await?; // handle_existing() } else { @@ -201,7 +202,7 @@ impl<'a> Synchroniser<'a> { for config in registry.iter() { // shouldn't be any executor/block_stream - self.handle_new_indexer(config).await?; + self.sync_new_indexer(config).await?; } Ok(()) @@ -472,7 +473,7 @@ mod test { ); synchroniser - .sync_block_stream(&config, &state, None) + .sync_existing_block_stream(&config, &state, None) .await .unwrap(); } @@ -516,7 +517,89 @@ mod test { ); synchroniser - .sync_block_stream(&config, &state, None) + .sync_existing_block_stream(&config, &state, None) + .await + .unwrap(); + } + + #[tokio::test] + async fn reconfigures_block_stream() { + let config_with_latest = IndexerConfig { + start_block: StartBlock::Latest, + ..IndexerConfig::default() + }; + let height = 5; + let config_with_height = IndexerConfig { + start_block: StartBlock::Height(height), + ..IndexerConfig::default() + }; + let last_published_block = 1; + let config_with_continue = IndexerConfig { + start_block: StartBlock::Continue, + ..IndexerConfig::default() + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with( + eq(last_published_block + 1), + eq(config_with_continue.clone()), + ) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with( + eq(config_with_latest.get_registry_version()), + eq(config_with_latest.clone()), + ) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with(eq(height), eq(config_with_height.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut redis_client = RedisClient::default(); + redis_client + .expect_clear_block_stream() + .with(eq(config_with_latest.clone())) + .returning(|_| Ok(())) + .once(); + redis_client + .expect_clear_block_stream() + .with(eq(config_with_height.clone())) + .returning(|_| Ok(())) + .once(); + redis_client + .expect_get_last_published_block() + .with(eq(config_with_continue.clone())) + .returning(move |_| Ok(Some(last_published_block))); + + let state_manager = IndexerStateManager::default(); + let executors_handler = ExecutorsHandler::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .reconfigure_block_stream(&config_with_latest) + .await + .unwrap(); + synchroniser + .reconfigure_block_stream(&config_with_height) + .await + .unwrap(); + synchroniser + .reconfigure_block_stream(&config_with_continue) .await .unwrap(); } From da19ba90f5c830c888531ee40a270db09e5b1d76 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Fri, 31 May 2024 15:12:00 +1200 Subject: [PATCH 08/32] feat: Update state after synchronisation --- coordinator/src/block_streams/handler.rs | 15 ++--- coordinator/src/indexer_state.rs | 20 +++++- coordinator/src/redis.rs | 5 +- coordinator/src/synchroniser.rs | 80 +++++++++++++++++++----- 4 files changed, 92 insertions(+), 28 deletions(-) diff --git a/coordinator/src/block_streams/handler.rs b/coordinator/src/block_streams/handler.rs index f17a974ce..a2315c171 100644 --- a/coordinator/src/block_streams/handler.rs +++ b/coordinator/src/block_streams/handler.rs @@ -62,9 +62,7 @@ impl BlockStreamsHandlerImpl { .clone() .stop_stream(Request::new(request.clone())) .await - .map_err(|e| { - tracing::error!(stream_id, "Failed to stop stream\n{e:?}"); - }); + .context(format!("Failed to stop stream: {stream_id}"))?; tracing::debug!(stream_id, "Stop stream response: {:#?}", response); @@ -125,13 +123,10 @@ impl BlockStreamsHandlerImpl { .clone() .start_stream(Request::new(request.clone())) .await - .map_err(|error| { - tracing::error!( - account_id = indexer_config.account_id.as_str(), - function_name = indexer_config.function_name, - "Failed to start stream\n{error:?}" - ); - }); + .context(format!( + "Failed to start stream: {}", + indexer_config.get_full_name() + ))?; tracing::debug!( account_id = indexer_config.account_id.as_str(), diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 72056446e..dfbce9e60 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -58,6 +58,15 @@ impl IndexerStateManagerImpl { Self { redis_client } } + fn get_default_state(&self, indexer_config: &IndexerConfig) -> IndexerState { + IndexerState { + account_id: indexer_config.account_id.clone(), + function_name: indexer_config.function_name.clone(), + block_stream_synced_at: None, + enabled: true, + } + } + pub async fn get_state(&self, indexer_config: &IndexerConfig) -> anyhow::Result { let raw_state = self.redis_client.get_indexer_state(indexer_config).await?; @@ -65,7 +74,7 @@ impl IndexerStateManagerImpl { return Ok(serde_json::from_str(&raw_state)?); } - Ok(IndexerState::default()) + Ok(self.get_default_state(indexer_config)) } async fn set_state( @@ -80,6 +89,15 @@ impl IndexerStateManagerImpl { .await } + pub async fn set_synced(&self, indexer_config: &IndexerConfig) -> anyhow::Result<()> { + let mut indexer_state = self.get_state(indexer_config).await?; + + indexer_state.block_stream_synced_at = Some(indexer_config.get_registry_version()); + + self.set_state(indexer_config, indexer_state).await?; + Ok(()) + } + pub async fn set_enabled( &self, indexer_config: &IndexerConfig, diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index 71d61cf03..8fe49c4f8 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -82,7 +82,10 @@ impl RedisClientImpl { } pub async fn clear_block_stream(&self, indexer_config: &IndexerConfig) -> anyhow::Result<()> { - self.del(indexer_config.get_redis_stream_key()).await + let stream_key = indexer_config.get_redis_stream_key(); + self.del(stream_key.clone()) + .await + .context(format!("Failed to clear Redis Stream: {}", stream_key)) } pub async fn get_indexer_state( diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 927cf8b23..2008fd51a 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -56,7 +56,7 @@ impl<'a> Synchroniser<'a> { .start(start_block, config) .await?; - // flush state + self.state_manager.set_synced(config).await?; Ok(()) } @@ -78,7 +78,7 @@ impl<'a> Synchroniser<'a> { .await?; } - tracing::info!("Starting new executor"); + tracing::info!("Starting executor"); self.executors_handler.start(config).await?; @@ -107,6 +107,18 @@ impl<'a> Synchroniser<'a> { StartBlock::Continue => self.get_continuation_block_height(config).await?, }; + tracing::info!(height, "Starting block stream"); + + self.block_streams_handler.start(height, config).await?; + + Ok(()) + } + + async fn resume_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { + let height = self.get_continuation_block_height(config).await?; + + tracing::info!(height, "Resuming block stream"); + self.block_streams_handler.start(height, config).await?; Ok(()) @@ -137,18 +149,19 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - // TODO handle state which was written before indexer was registered - if state.block_stream_synced_at.is_none() - || state.block_stream_synced_at.unwrap() != config.get_registry_version() - { + if state.block_stream_synced_at.is_none() { + // This would indicate that the state was inisialized before the Indexer was + // registered, which is currently not possible, but may be in future + anyhow::bail!("Existing Indexer has no `block_stream_synced_at` field") + } + + if state.block_stream_synced_at.unwrap() != config.get_registry_version() { self.reconfigure_block_stream(config).await?; return Ok(()); } - let height = self.get_continuation_block_height(config).await?; - - self.block_streams_handler.start(height, config).await?; + self.resume_block_stream(config).await?; Ok(()) } @@ -161,12 +174,17 @@ impl<'a> Synchroniser<'a> { executor: Option<&ExecutorInfo>, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { - // TODO in parallel - self.sync_existing_executor(config, executor).await?; - self.sync_existing_block_stream(config, state, block_stream) - .await?; + let result = tokio::try_join!( + self.sync_existing_executor(config, executor), + self.sync_existing_block_stream(config, state, block_stream) + ); - // TODO flush state + if let Err(error) = result { + tracing::error!(?error, "Failed to sync Indexer"); + return Ok(()); + } + + self.state_manager.set_synced(config).await?; Ok(()) } @@ -253,12 +271,12 @@ mod test { executors_handler.expect_list().returning(|| Ok(vec![])); executors_handler .expect_start() - .with(eq(config1)) + .with(eq(config1.clone())) .returning(|_| Ok(())) .once(); executors_handler .expect_start() - .with(eq(config2)) + .with(eq(config2.clone())) .returning(|_| Ok(())) .once(); @@ -269,6 +287,16 @@ mod test { let mut state_manager = IndexerStateManager::default(); state_manager.expect_list().returning(|| Ok(vec![])); + state_manager + .expect_set_synced() + .with(eq(config1)) + .returning(|_| Ok(())) + .once(); + state_manager + .expect_set_synced() + .with(eq(config2)) + .returning(|_| Ok(())) + .once(); let redis_client = RedisClient::default(); @@ -329,6 +357,11 @@ mod test { .returning(move || Ok(indexer_registry.clone())); let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_set_synced() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); state_manager.expect_list().returning(move || { Ok(vec![IndexerState { account_id: config.account_id.clone(), @@ -416,6 +449,11 @@ mod test { .once(); let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_set_synced() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); state_manager.expect_list().returning(move || { Ok(vec![IndexerState { account_id: config.account_id.clone(), @@ -603,5 +641,15 @@ mod test { .await .unwrap(); } + + #[tokio::test] + async fn handles_synchronisation_failures() {} + + #[tokio::test] + async fn stops_disabled_indexers() {} + #[tokio::test] + async fn ignores_disabled_indexers() {} + #[tokio::test] + async fn flushes_state_after_synchronisation() {} } } From 9c595c81a78605370bc0c1d6c1feb20eecb66d86 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Fri, 31 May 2024 15:49:07 +1200 Subject: [PATCH 09/32] feat: Handle sync failures on new indexers --- coordinator/src/synchroniser.rs | 242 ++++++++++++++++++++++++++++++-- 1 file changed, 230 insertions(+), 12 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 2008fd51a..59111d028 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -39,7 +39,10 @@ impl<'a> Synchroniser<'a> { } async fn sync_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { - self.executors_handler.start(config).await?; + if let Err(err) = self.executors_handler.start(config).await { + tracing::error!(?err, "Failed to start Executor"); + return Ok(()); + } let start_block = match config.start_block { StartBlock::Height(height) => height, @@ -52,10 +55,12 @@ impl<'a> Synchroniser<'a> { } }; - self.block_streams_handler - .start(start_block, config) - .await?; + if let Err(err) = self.block_streams_handler.start(start_block, config).await { + tracing::error!(?err, "Failed to start Block Stream"); + return Ok(()); + } + // TODO handle failures self.state_manager.set_synced(config).await?; Ok(()) @@ -174,16 +179,20 @@ impl<'a> Synchroniser<'a> { executor: Option<&ExecutorInfo>, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { - let result = tokio::try_join!( - self.sync_existing_executor(config, executor), - self.sync_existing_block_stream(config, state, block_stream) - ); + if let Err(error) = self.sync_existing_executor(config, executor).await { + tracing::error!(?error, "Failed to sync executor"); + return Ok(()); + } - if let Err(error) = result { - tracing::error!(?error, "Failed to sync Indexer"); + if let Err(error) = self + .sync_existing_block_stream(config, state, block_stream) + .await + { + tracing::error!(?error, "Failed to sync block stream"); return Ok(()); } + // TODO handle failures self.state_manager.set_synced(config).await?; Ok(()) @@ -310,6 +319,144 @@ mod test { synchroniser.sync().await.unwrap(); } + + #[tokio::test] + async fn configures_block_stream() { + let config_with_latest = IndexerConfig { + start_block: StartBlock::Latest, + ..IndexerConfig::default() + }; + let height = 5; + let config_with_height = IndexerConfig { + start_block: StartBlock::Height(height), + ..IndexerConfig::default() + }; + let config_with_continue = IndexerConfig { + start_block: StartBlock::Continue, + ..IndexerConfig::default() + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with( + eq(config_with_continue.get_registry_version()), + eq(config_with_continue.clone()), + ) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with( + eq(config_with_latest.get_registry_version()), + eq(config_with_latest.clone()), + ) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with(eq(height), eq(config_with_height.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_set_synced() + .with(eq(config_with_continue.clone())) + .returning(|_| Ok(())) + .once(); + state_manager + .expect_set_synced() + .with(eq(config_with_latest.clone())) + .returning(|_| Ok(())) + .once(); + state_manager + .expect_set_synced() + .with(eq(config_with_height.clone())) + .returning(|_| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler + .expect_start() + .returning(|_| Ok(())) + .times(3); + + let redis_client = RedisClient::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_new_indexer(&config_with_latest) + .await + .unwrap(); + synchroniser + .sync_new_indexer(&config_with_height) + .await + .unwrap(); + synchroniser + .sync_new_indexer(&config_with_continue) + .await + .unwrap(); + } + + #[tokio::test] + async fn handles_synchronisation_failures() { + let config = IndexerConfig::default(); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler + .expect_start() + .with(eq(config.clone())) + .returning(|_| anyhow::bail!("")) + .once(); + executors_handler + .expect_start() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .times(2); + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with(eq(100), eq(config.clone())) + .returning(|_, _| anyhow::bail!("")) + .once(); + block_streams_handler + .expect_start() + .with(eq(100), eq(config.clone())) + .returning(|_, _| Ok(())) + .once(); + + let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_set_synced() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .once(); + + let redis_client = RedisClient::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync_new_indexer(&config).await.unwrap(); // fail + synchroniser.sync_new_indexer(&config).await.unwrap(); // fail + synchroniser.sync_new_indexer(&config).await.unwrap(); // success + } } mod existing { @@ -642,8 +789,79 @@ mod test { .unwrap(); } - #[tokio::test] - async fn handles_synchronisation_failures() {} + //#[tokio::test] + //async fn handles_synchronisation_failures() { + // let config = IndexerConfig::default(); + // let state = IndexerState { + // account_id: config.account_id.clone(), + // function_name: config.function_name.clone(), + // block_stream_synced_at: Some(config.get_registry_version()), + // enabled: true, + // }; + // + // let mut executors_handler = ExecutorsHandler::default(); + // executors_handler + // .expect_stop() + // .with(always()) + // .returning(|_| anyhow::bail!("")) + // .once(); + // executors_handler + // .expect_stop() + // .with(always()) + // .returning(|_| Ok(())); + // executors_handler + // .expect_start() + // .with(eq(config.clone())) + // .returning(|_| anyhow::bail!("")) + // .once(); + // executors_handler + // .expect_start() + // .with(eq(config.clone())) + // .returning(|_| Ok(())); + // + // let mut block_streams_handler = BlockStreamsHandler::default(); + // block_streams_handler + // .expect_start() + // .with(eq(100), eq(config.clone())) + // .returning(|_, _| anyhow::bail!("")) + // .once(); + // block_streams_handler + // .expect_start() + // .with(eq(100), eq(config.clone())) + // .returning(|_, _| Ok(())) + // .once(); + // + // let mut state_manager = IndexerStateManager::default(); + // state_manager + // .expect_set_synced() + // .with(eq(config.clone())) + // .returning(|_| Ok(())) + // .once(); + // + // let redis_client = RedisClient::default(); + // let registry = Registry::default(); + // + // let synchroniser = Synchroniser::new( + // &block_streams_handler, + // &executors_handler, + // ®istry, + // &state_manager, + // &redis_client, + // ); + // + // synchroniser + // .sync_existing_indexer(&config, &state) + // .await + // .unwrap(); // fail + // synchroniser + // .sync_existing_indexer(&config, &state) + // .await + // .unwrap(); // fail + // synchroniser + // .sync_existing_indexer(&config, &state) + // .await + // .unwrap(); // success + //} #[tokio::test] async fn stops_disabled_indexers() {} From 54ca2c1a0756d9915171b544dd19d8f909e3d7ed Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 11:29:01 +1200 Subject: [PATCH 10/32] feat: Stop diabled indexers --- coordinator/src/indexer_state.rs | 1 + coordinator/src/synchroniser.rs | 178 +++++++++++++++++-------------- 2 files changed, 99 insertions(+), 80 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index dfbce9e60..9339abbdb 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -95,6 +95,7 @@ impl IndexerStateManagerImpl { indexer_state.block_stream_synced_at = Some(indexer_config.get_registry_version()); self.set_state(indexer_config, indexer_state).await?; + Ok(()) } diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 59111d028..0ff6c05c2 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -9,7 +9,7 @@ use crate::{ indexer_config::IndexerConfig, indexer_state::{IndexerState, IndexerStateManager}, redis::RedisClient, - registry::{IndexerRegistry, Registry}, + registry::Registry, }; pub struct Synchroniser<'a> { @@ -69,8 +69,21 @@ impl<'a> Synchroniser<'a> { async fn sync_existing_executor( &self, config: &IndexerConfig, + state: &IndexerState, executor: Option<&ExecutorInfo>, ) -> anyhow::Result<()> { + if !state.enabled { + if let Some(executor) = executor { + tracing::info!("Stopping disabled executor"); + + self.executors_handler + .stop(executor.executor_id.clone()) + .await?; + } + + return Ok(()); + } + if let Some(executor) = executor { if executor.version == config.get_registry_version() { return Ok(()); @@ -135,6 +148,18 @@ impl<'a> Synchroniser<'a> { state: &IndexerState, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { + if !state.enabled { + if let Some(block_stream) = block_stream { + tracing::info!("Stopping disabled block stream"); + + self.block_streams_handler + .stop(block_stream.stream_id.clone()) + .await?; + } + + return Ok(()); + } + if let Some(block_stream) = block_stream { if block_stream.version == config.get_registry_version() { return Ok(()); @@ -179,7 +204,7 @@ impl<'a> Synchroniser<'a> { executor: Option<&ExecutorInfo>, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { - if let Err(error) = self.sync_existing_executor(config, executor).await { + if let Err(error) = self.sync_existing_executor(config, state, executor).await { tracing::error!(?error, "Failed to sync executor"); return Ok(()); } @@ -192,8 +217,10 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - // TODO handle failures - self.state_manager.set_synced(config).await?; + if state.enabled { + // TODO handle failures + self.state_manager.set_synced(config).await?; + } Ok(()) } @@ -221,7 +248,6 @@ impl<'a> Synchroniser<'a> { self.sync_existing_indexer(&config, &state, executor, block_stream) .await?; - // handle_existing() } else { // handle_deleted() } @@ -243,6 +269,8 @@ mod test { use mockall::predicate::*; use std::collections::HashMap; + use crate::registry::IndexerRegistry; + mod new { use super::*; @@ -789,82 +817,72 @@ mod test { .unwrap(); } - //#[tokio::test] - //async fn handles_synchronisation_failures() { - // let config = IndexerConfig::default(); - // let state = IndexerState { - // account_id: config.account_id.clone(), - // function_name: config.function_name.clone(), - // block_stream_synced_at: Some(config.get_registry_version()), - // enabled: true, - // }; - // - // let mut executors_handler = ExecutorsHandler::default(); - // executors_handler - // .expect_stop() - // .with(always()) - // .returning(|_| anyhow::bail!("")) - // .once(); - // executors_handler - // .expect_stop() - // .with(always()) - // .returning(|_| Ok(())); - // executors_handler - // .expect_start() - // .with(eq(config.clone())) - // .returning(|_| anyhow::bail!("")) - // .once(); - // executors_handler - // .expect_start() - // .with(eq(config.clone())) - // .returning(|_| Ok(())); - // - // let mut block_streams_handler = BlockStreamsHandler::default(); - // block_streams_handler - // .expect_start() - // .with(eq(100), eq(config.clone())) - // .returning(|_, _| anyhow::bail!("")) - // .once(); - // block_streams_handler - // .expect_start() - // .with(eq(100), eq(config.clone())) - // .returning(|_, _| Ok(())) - // .once(); - // - // let mut state_manager = IndexerStateManager::default(); - // state_manager - // .expect_set_synced() - // .with(eq(config.clone())) - // .returning(|_| Ok(())) - // .once(); - // - // let redis_client = RedisClient::default(); - // let registry = Registry::default(); - // - // let synchroniser = Synchroniser::new( - // &block_streams_handler, - // &executors_handler, - // ®istry, - // &state_manager, - // &redis_client, - // ); - // - // synchroniser - // .sync_existing_indexer(&config, &state) - // .await - // .unwrap(); // fail - // synchroniser - // .sync_existing_indexer(&config, &state) - // .await - // .unwrap(); // fail - // synchroniser - // .sync_existing_indexer(&config, &state) - // .await - // .unwrap(); // success - //} - #[tokio::test] - async fn stops_disabled_indexers() {} + async fn stops_disabled_indexers() { + let config = IndexerConfig::default(); + let state = IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version()), + enabled: false, + }; + let executor = ExecutorInfo { + executor_id: "executor_id".to_string(), + account_id: config.account_id.to_string(), + function_name: config.function_name.clone(), + version: config.get_registry_version(), + status: "running".to_string(), + }; + let block_stream = StreamInfo { + stream_id: "stream_id".to_string(), + account_id: config.account_id.to_string(), + function_name: config.function_name.clone(), + version: config.get_registry_version(), + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_stop() + .with(eq("stream_id".to_string())) + .returning(|_| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler + .expect_stop() + .with(eq("executor_id".to_string())) + .returning(|_| Ok(())) + .once(); + + let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_set_synced() + .with(eq(config.clone())) + .returning(|_| Ok(())) + .never(); + + let registry = Registry::default(); + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_existing_indexer(&config, &state, Some(&executor), Some(&block_stream)) + .await + .unwrap(); + // Simulate second run, start/stop etc should not be called + synchroniser + .sync_existing_indexer(&config, &state, None, None) + .await + .unwrap(); + } + #[tokio::test] async fn ignores_disabled_indexers() {} #[tokio::test] From c96ab4ff52e5887ab0a7468502b5dbedfc6a0258 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 13:51:34 +1200 Subject: [PATCH 11/32] refactor: Separate sorting logic from acting logic --- coordinator/src/synchroniser.rs | 75 +++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 18 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 0ff6c05c2..c46abc7c0 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -12,6 +12,17 @@ use crate::{ registry::Registry, }; +pub enum SynchronisationState { + New(IndexerConfig), + Existing( + IndexerConfig, + IndexerState, + Option, + Option, + ), + Deleted(IndexerState, Option, Option), +} + pub struct Synchroniser<'a> { block_streams_handler: &'a BlockStreamsHandler, executors_handler: &'a ExecutorsHandler, @@ -225,37 +236,65 @@ impl<'a> Synchroniser<'a> { Ok(()) } - pub async fn sync(&self) -> anyhow::Result<()> { + async fn generate_synchronisation_states(&self) -> anyhow::Result> { let states = self.state_manager.list().await?; let mut registry = self.registry.fetch().await?; - // TODO get instead of list? - let executors = self.executors_handler.list().await?; - let block_streams = self.block_streams_handler.list().await?; + let mut executors_iter = self.executors_handler.list().await?.into_iter(); + let mut block_streams_iter = self.block_streams_handler.list().await?.into_iter(); + + let mut contexts = vec![]; for state in states { - let config = registry - .get(&state.account_id, &state.function_name) - .cloned(); - let executor = executors.iter().find(|e| { - e.account_id == state.account_id && e.function_name == state.function_name + let config = registry.remove(&state.account_id, &state.function_name); + let executor = executors_iter.find(|executor| { + executor.account_id == state.account_id + && executor.function_name == state.function_name }); - let block_stream = block_streams.iter().find(|b| { - b.account_id == state.account_id && b.function_name == state.function_name + let block_stream = block_streams_iter.find(|block_stream| { + block_stream.account_id == state.account_id + && block_stream.function_name == state.function_name }); if let Some(config) = config { - registry.remove(&state.account_id, &state.function_name); - - self.sync_existing_indexer(&config, &state, executor, block_stream) - .await?; + contexts.push(SynchronisationState::Existing( + config, + state, + executor, + block_stream, + )) } else { - // handle_deleted() + contexts.push(SynchronisationState::Deleted(state, executor, block_stream)) } } for config in registry.iter() { - // shouldn't be any executor/block_stream - self.sync_new_indexer(config).await?; + contexts.push(SynchronisationState::New(config.clone())); + } + + Ok(contexts) + } + + pub async fn sync(&self) -> anyhow::Result<()> { + let sync_states = self.generate_synchronisation_states().await?; + + for sync_state in sync_states { + match sync_state { + SynchronisationState::New(config) => { + self.sync_new_indexer(&config).await?; + } + SynchronisationState::Existing(config, state, executor, block_stream) => { + self.sync_existing_indexer( + &config, + &state, + executor.as_ref(), + block_stream.as_ref(), + ) + .await?; + } + SynchronisationState::Deleted(..) => { + // handle deleted + } + } } Ok(()) From 6a35eb651bf8e8e155acff224119047899374cd1 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 15:54:58 +1200 Subject: [PATCH 12/32] feat: Migrate indexer redis state --- coordinator/src/indexer_state.rs | 112 ++++++++++++++++++++++++++++++- coordinator/src/main.rs | 4 +- coordinator/src/redis.rs | 46 ++++++++++++- coordinator/src/synchroniser.rs | 8 +-- 4 files changed, 158 insertions(+), 12 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 9339abbdb..b85594f0c 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -1,5 +1,6 @@ #![cfg_attr(test, allow(dead_code))] +use anyhow::Context; use near_primitives::types::AccountId; use std::cmp::Ordering; use std::str::FromStr; @@ -15,6 +16,12 @@ pub enum SyncStatus { New, } +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct OldIndexerState { + pub block_stream_synced_at: Option, + pub enabled: bool, +} + // NOTE We'll need to add more fields here - is there a way to gracefully handle non-existant // fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -51,13 +58,50 @@ pub struct IndexerStateManagerImpl { redis_client: RedisClient, } -// NOTE we probably need a "list" method, which means storing all state ids in a Redis set #[cfg_attr(test, mockall::automock)] impl IndexerStateManagerImpl { pub fn new(redis_client: RedisClient) -> Self { Self { redis_client } } + pub async fn migrate(&self, registry: &IndexerRegistry) -> anyhow::Result<()> { + if self.redis_client.indexer_states_set_exists().await? { + return Ok(()); + } + + tracing::info!("Migrating indexer state"); + + for config in registry.iter() { + let raw_state = self.redis_client.get_indexer_state(config).await?; + + let state = if let Some(raw_state) = raw_state { + let old_state: OldIndexerState = + serde_json::from_str(&raw_state).context(format!( + "Failed to deserialize OldIndexerState for {}", + config.get_full_name() + ))?; + + IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: old_state.block_stream_synced_at, + enabled: old_state.enabled, + } + } else { + self.get_default_state(config) + }; + + self.set_state(config, state).await.context(format!( + "Failed to set state for {}", + config.get_full_name() + ))?; + } + + tracing::info!("Migration complete"); + + Ok(()) + } + fn get_default_state(&self, indexer_config: &IndexerConfig) -> IndexerState { IndexerState { account_id: indexer_config.account_id.clone(), @@ -186,6 +230,72 @@ mod tests { use mockall::predicate; use registry_types::{Rule, StartBlock, Status}; + #[tokio::test] + async fn migrate() { + let config1 = IndexerConfig::default(); + let config2 = IndexerConfig { + account_id: "darunrs.near".parse().unwrap(), + function_name: "test".to_string(), + ..Default::default() + }; + + let registry = IndexerRegistry::from(&[ + ( + "morgs.near".parse().unwrap(), + HashMap::from([("test".to_string(), config1.clone())]), + ), + ( + "darunrs.near".parse().unwrap(), + HashMap::from([("test".to_string(), config2.clone())]), + ), + ]); + + let mut mock_redis_client = RedisClient::default(); + mock_redis_client + .expect_indexer_states_set_exists() + .returning(|| Ok(false)) + .once(); + mock_redis_client + .expect_get_indexer_state() + .with(predicate::eq(config1.clone())) + .returning(|_| { + Ok(Some( + serde_json::json!({ "block_stream_synced_at": 200, "enabled": false }) + .to_string(), + )) + }) + .once(); + mock_redis_client + .expect_get_indexer_state() + .with(predicate::eq(config2.clone())) + .returning(|_| Ok(None)) + .once(); + mock_redis_client + .expect_set_indexer_state() + .with( + predicate::eq(config1), + predicate::eq( + "{\"account_id\":\"morgs.near\",\"function_name\":\"test\",\"block_stream_synced_at\":200,\"enabled\":false}".to_string(), + ), + ) + .returning(|_, _| Ok(())) + .once(); + mock_redis_client + .expect_set_indexer_state() + .with( + predicate::eq(config2), + predicate::eq( + "{\"account_id\":\"darunrs.near\",\"function_name\":\"test\",\"block_stream_synced_at\":null,\"enabled\":true}".to_string() + ), + ) + .returning(|_, _| Ok(())) + .once(); + + let indexer_manager = IndexerStateManagerImpl::new(mock_redis_client); + + indexer_manager.migrate(®istry).await.unwrap(); + } + #[tokio::test] async fn filters_disabled_indexers() { let morgs_config = IndexerConfig { diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 80e640d63..58c9ff110 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -78,9 +78,7 @@ async fn main() -> anyhow::Result<()> { // This will also allow us to determine when an Indexer has been deleted, rather than // implicitly relying on the existance of executors/block_streams. This is going to be // important for deprovisioning. - let indexer_registry = indexer_state_manager - .filter_disabled_indexers(&indexer_registry) - .await?; + indexer_state_manager.migrate(&indexer_registry).await?; tokio::try_join!( // NOTE this may need to be regactored in to a combined "synchronise" function. diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index 8fe49c4f8..3aa8c2cd8 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -18,6 +18,8 @@ pub struct RedisClientImpl { } impl RedisClientImpl { + const INDEXER_STATES_SET: &'static str = "indexer_states"; + pub async fn connect(redis_url: &str) -> anyhow::Result { let connection = redis::Client::open(redis_url)? .get_connection_manager() @@ -73,6 +75,38 @@ impl RedisClientImpl { Ok(()) } + pub async fn sadd(&self, set: S, value: V) -> anyhow::Result<()> + where + S: ToRedisArgs + Debug + Send + Sync + 'static, + V: ToRedisArgs + Debug + Send + Sync + 'static, + { + tracing::debug!("SADD {set:?} {value:?}"); + + redis::cmd("SADD") + .arg(&set) + .arg(&value) + .query_async(&mut self.connection.clone()) + .await + .context(format!("SADD {set:?} {value:?}")) + } + + pub async fn exists(&self, key: K) -> anyhow::Result + where + K: ToRedisArgs + Debug + Send + Sync + 'static, + { + tracing::debug!("EXISTS {key:?}"); + + redis::cmd("EXISTS") + .arg(&key) + .query_async(&mut self.connection.clone()) + .await + .context(format!("EXISTS {key:?}")) + } + + pub async fn indexer_states_set_exists(&self) -> anyhow::Result { + self.exists(Self::INDEXER_STATES_SET).await + } + pub async fn get_last_published_block( &self, indexer_config: &IndexerConfig, @@ -100,7 +134,10 @@ impl RedisClientImpl { indexer_config: &IndexerConfig, state: String, ) -> anyhow::Result<()> { - self.set(indexer_config.get_state_key(), state).await + self.set(indexer_config.get_state_key(), state).await?; + + self.sadd(Self::INDEXER_STATES_SET, indexer_config.get_state_key()) + .await } } @@ -133,6 +170,13 @@ mockall::mock! { where K: ToRedisArgs + Debug + Send + Sync + 'static, V: ToRedisArgs + Debug + Send + Sync + 'static; + + pub async fn indexer_states_set_exists(&self) -> anyhow::Result; + + pub async fn sadd(&self, set: S, value: V) -> anyhow::Result<()> + where + S: ToRedisArgs + Debug + Send + Sync + 'static, + V: ToRedisArgs + Debug + Send + Sync + 'static; } impl Clone for RedisClientImpl { diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index c46abc7c0..e3fb7e821 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -12,6 +12,7 @@ use crate::{ registry::Registry, }; +#[allow(clippy::large_enum_variant)] pub enum SynchronisationState { New(IndexerConfig), Existing( @@ -32,7 +33,6 @@ pub struct Synchroniser<'a> { } impl<'a> Synchroniser<'a> { - // TODO use builder? pub fn new( block_streams_handler: &'a BlockStreamsHandler, executors_handler: &'a ExecutorsHandler, @@ -71,7 +71,6 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - // TODO handle failures self.state_manager.set_synced(config).await?; Ok(()) @@ -921,10 +920,5 @@ mod test { .await .unwrap(); } - - #[tokio::test] - async fn ignores_disabled_indexers() {} - #[tokio::test] - async fn flushes_state_after_synchronisation() {} } } From 622f8e3db7e48452b2c190cc8249dc8354ca044d Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 16:03:53 +1200 Subject: [PATCH 13/32] feat: List indexer states --- coordinator/src/indexer_state.rs | 27 ++++++++++++++++++++++++++- coordinator/src/redis.rs | 19 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index b85594f0c..2aab7df72 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -217,7 +217,14 @@ impl IndexerStateManagerImpl { } pub async fn list(&self) -> anyhow::Result> { - todo!() + self.redis_client + .list_indexer_states() + .await? + .iter() + .try_fold(Vec::new(), |mut acc, state| { + acc.push(serde_json::from_str(state)?); + Ok(acc) + }) } } @@ -230,6 +237,24 @@ mod tests { use mockall::predicate; use registry_types::{Rule, StartBlock, Status}; + #[tokio::test] + async fn list_indexer_states() { + let mut mock_redis_client = RedisClient::default(); + mock_redis_client + .expect_list_indexer_states() + .returning(|| Ok(vec![serde_json::json!({ "account_id": "morgs.near", "function_name": "test", "block_stream_synced_at": 200, "enabled": true }).to_string()])) + .once(); + mock_redis_client + .expect_list_indexer_states() + .returning(|| Ok(vec![serde_json::json!({}).to_string()])) + .once(); + + let indexer_manager = IndexerStateManagerImpl::new(mock_redis_client); + + assert_eq!(indexer_manager.list().await.unwrap().len(), 1); + assert!(indexer_manager.list().await.is_err()); + } + #[tokio::test] async fn migrate() { let config1 = IndexerConfig::default(); diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index 3aa8c2cd8..b0f4303fc 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -75,6 +75,19 @@ impl RedisClientImpl { Ok(()) } + pub async fn smembers(&self, set: S) -> anyhow::Result> + where + S: ToRedisArgs + Debug + Send + Sync + 'static, + { + tracing::debug!("SMEMBERS {set:?}"); + + redis::cmd("SMEMBERS") + .arg(&set) + .query_async(&mut self.connection.clone()) + .await + .context(format!("SMEMBERS {set:?}")) + } + pub async fn sadd(&self, set: S, value: V) -> anyhow::Result<()> where S: ToRedisArgs + Debug + Send + Sync + 'static, @@ -139,6 +152,10 @@ impl RedisClientImpl { self.sadd(Self::INDEXER_STATES_SET, indexer_config.get_state_key()) .await } + + pub async fn list_indexer_states(&self) -> anyhow::Result> { + self.smembers(Self::INDEXER_STATES_SET).await + } } #[cfg(test)] @@ -177,6 +194,8 @@ mockall::mock! { where S: ToRedisArgs + Debug + Send + Sync + 'static, V: ToRedisArgs + Debug + Send + Sync + 'static; + + pub async fn list_indexer_states(&self) -> anyhow::Result>; } impl Clone for RedisClientImpl { From 709b6ed467f825cd7097e3753316a45232c09008 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 16:21:11 +1200 Subject: [PATCH 14/32] fix: Correctly list indexer states --- coordinator/src/indexer_state.rs | 10 +++++++--- coordinator/src/redis.rs | 18 +++++++++++++++++- coordinator/src/synchroniser.rs | 1 + 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 2aab7df72..3d229cd6e 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -221,10 +221,14 @@ impl IndexerStateManagerImpl { .list_indexer_states() .await? .iter() - .try_fold(Vec::new(), |mut acc, state| { - acc.push(serde_json::from_str(state)?); - Ok(acc) + .try_fold(Vec::new(), |mut acc, raw_state| { + acc.push( + serde_json::from_str(raw_state) + .context(format!("failed to deserailize {raw_state}"))?, + ); + anyhow::Ok(acc) }) + .context("Failed to deserialize indexer states") } } diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index b0f4303fc..fc0381c12 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -154,7 +154,23 @@ impl RedisClientImpl { } pub async fn list_indexer_states(&self) -> anyhow::Result> { - self.smembers(Self::INDEXER_STATES_SET).await + let mut states = vec![]; + + for state_key in self.smembers(Self::INDEXER_STATES_SET).await? { + let state = self.get(state_key.clone()).await?; + + if state.is_none() { + anyhow::bail!( + "Key: {} from Set: {} set, does not exist", + state_key, + Self::INDEXER_STATES_SET + ); + } + + states.push(state.unwrap()); + } + + Ok(states) } } diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index e3fb7e821..2726fe32f 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -13,6 +13,7 @@ use crate::{ }; #[allow(clippy::large_enum_variant)] +#[derive(Debug)] pub enum SynchronisationState { New(IndexerConfig), Existing( From 160e8ded8342936276ece69837598f9c75eaaf33 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 21:27:03 +1200 Subject: [PATCH 15/32] fix: Ensure iterator is not consumed on first loop --- coordinator/src/synchroniser.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 2726fe32f..0cee49194 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -239,39 +239,43 @@ impl<'a> Synchroniser<'a> { async fn generate_synchronisation_states(&self) -> anyhow::Result> { let states = self.state_manager.list().await?; let mut registry = self.registry.fetch().await?; - let mut executors_iter = self.executors_handler.list().await?.into_iter(); - let mut block_streams_iter = self.block_streams_handler.list().await?.into_iter(); + let executors = self.executors_handler.list().await?; + let block_streams = self.block_streams_handler.list().await?; - let mut contexts = vec![]; + let mut sync_states = vec![]; for state in states { let config = registry.remove(&state.account_id, &state.function_name); - let executor = executors_iter.find(|executor| { + let executor = executors.iter().find(|executor| { executor.account_id == state.account_id && executor.function_name == state.function_name }); - let block_stream = block_streams_iter.find(|block_stream| { + let block_stream = block_streams.iter().find(|block_stream| { block_stream.account_id == state.account_id && block_stream.function_name == state.function_name }); if let Some(config) = config { - contexts.push(SynchronisationState::Existing( + sync_states.push(SynchronisationState::Existing( config, state, - executor, - block_stream, + executor.cloned(), + block_stream.cloned(), )) } else { - contexts.push(SynchronisationState::Deleted(state, executor, block_stream)) + sync_states.push(SynchronisationState::Deleted( + state, + executor.cloned(), + block_stream.cloned(), + )) } } for config in registry.iter() { - contexts.push(SynchronisationState::New(config.clone())); + sync_states.push(SynchronisationState::New(config.clone())); } - Ok(contexts) + Ok(sync_states) } pub async fn sync(&self) -> anyhow::Result<()> { From c6bb4924703e005f61ae68e44fd10ff6b7ad22ab Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 21:27:53 +1200 Subject: [PATCH 16/32] refactor: Handle disabled indexers --- coordinator/src/synchroniser.rs | 64 ++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 0cee49194..e94515018 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -2,6 +2,7 @@ use block_streamer::StreamInfo; use registry_types::StartBlock; use runner::ExecutorInfo; +use tracing::instrument; use crate::{ block_streams::BlockStreamsHandler, @@ -50,6 +51,14 @@ impl<'a> Synchroniser<'a> { } } + #[instrument( + skip_all, + fields( + account_id = config.account_id.to_string(), + function_name = config.function_name, + version = config.get_registry_version() + ) + )] async fn sync_new_indexer(&self, config: &IndexerConfig) -> anyhow::Result<()> { if let Err(err) = self.executors_handler.start(config).await { tracing::error!(?err, "Failed to start Executor"); @@ -83,18 +92,6 @@ impl<'a> Synchroniser<'a> { state: &IndexerState, executor: Option<&ExecutorInfo>, ) -> anyhow::Result<()> { - if !state.enabled { - if let Some(executor) = executor { - tracing::info!("Stopping disabled executor"); - - self.executors_handler - .stop(executor.executor_id.clone()) - .await?; - } - - return Ok(()); - } - if let Some(executor) = executor { if executor.version == config.get_registry_version() { return Ok(()); @@ -159,18 +156,6 @@ impl<'a> Synchroniser<'a> { state: &IndexerState, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { - if !state.enabled { - if let Some(block_stream) = block_stream { - tracing::info!("Stopping disabled block stream"); - - self.block_streams_handler - .stop(block_stream.stream_id.clone()) - .await?; - } - - return Ok(()); - } - if let Some(block_stream) = block_stream { if block_stream.version == config.get_registry_version() { return Ok(()); @@ -207,14 +192,37 @@ impl<'a> Synchroniser<'a> { Ok(()) } + #[instrument( + skip_all, + fields( + account_id = config.account_id.to_string(), + function_name = config.function_name, + version = config.get_registry_version() + ) + )] async fn sync_existing_indexer( &self, config: &IndexerConfig, - // TODO handle disabled indexers state: &IndexerState, executor: Option<&ExecutorInfo>, block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { + if !state.enabled { + if let Some(executor) = executor { + self.executors_handler + .stop(executor.executor_id.clone()) + .await?; + } + + if let Some(block_stream) = block_stream { + self.block_streams_handler + .stop(block_stream.stream_id.clone()) + .await?; + } + + return Ok(()); + } + if let Err(error) = self.sync_existing_executor(config, state, executor).await { tracing::error!(?error, "Failed to sync executor"); return Ok(()); @@ -228,10 +236,8 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - if state.enabled { - // TODO handle failures - self.state_manager.set_synced(config).await?; - } + // TODO handle failures + self.state_manager.set_synced(config).await?; Ok(()) } From 95cb0798fbfe40431363085f7ba1e1bdcaf766d3 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Mon, 10 Jun 2024 21:28:11 +1200 Subject: [PATCH 17/32] fix: Handle block streams with no last published block --- coordinator/src/synchroniser.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index e94515018..dbb57b67c 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -112,11 +112,18 @@ impl<'a> Synchroniser<'a> { } async fn get_continuation_block_height(&self, config: &IndexerConfig) -> anyhow::Result { - self.redis_client + let height = self + .redis_client .get_last_published_block(config) .await? .map(|height| height + 1) - .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")) + .unwrap_or_else(|| { + tracing::warn!("Failed to get continuation block height, using registry version"); + + config.get_registry_version() + }); + + Ok(height) } async fn reconfigure_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { From b5e33275d177a355c9c754269fb82e862bc14543 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 13:21:30 +1200 Subject: [PATCH 18/32] fix: Dont swallow start executor errors --- coordinator/src/executors/handler.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/coordinator/src/executors/handler.rs b/coordinator/src/executors/handler.rs index fcbbb6a6b..a62886430 100644 --- a/coordinator/src/executors/handler.rs +++ b/coordinator/src/executors/handler.rs @@ -65,13 +65,10 @@ impl ExecutorsHandlerImpl { .clone() .start_executor(Request::new(request.clone())) .await - .map_err(|error| { - tracing::error!( - account_id = indexer_config.account_id.as_str(), - function_name = indexer_config.function_name, - "Failed to start executor\n{error:?}" - ); - }); + .context(format!( + "Failed to start executor: {}", + indexer_config.get_full_name() + ))?; tracing::debug!( account_id = indexer_config.account_id.as_str(), From 536a71780885b8a091eb342aea280fa25957f03a Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 13:21:56 +1200 Subject: [PATCH 19/32] chore: Add some notes --- coordinator/src/indexer_state.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 3d229cd6e..5585a8eb9 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -26,14 +26,14 @@ pub struct OldIndexerState { // fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct IndexerState { - // TODO need to migrate existing state pub account_id: AccountId, pub function_name: String, - // TODO this no longer needs to be optional, because the existance of the state object implies + // NOTE this no longer needs to be optional, because the existance of the state object implies // that the stream has been started // // but we might need to write the state object before we register, so therefore may need this - // to be none + // to be none, or we might create state before synchronisation for other reasons, so maybe its + // best to keep it optional pub block_stream_synced_at: Option, pub enabled: bool, } @@ -64,6 +64,14 @@ impl IndexerStateManagerImpl { Self { redis_client } } + // NOTE the synchronisation logic uses the existence of state to signify "existing", so by + // using the current registry we assume that all current indexers have be "seen" before, and + // can therefore be treated as "existing", but if an indexer is registered during deployment of + // this, we will incorrectly treat a "new" indexer as "existing". + // + // So what does this mean, we'll miss the "new" flow, which eventually include provisioning, + // but since it does not yet, I think this is actually fine. The "existing" flow will just + // start the indexer as is. pub async fn migrate(&self, registry: &IndexerRegistry) -> anyhow::Result<()> { if self.redis_client.indexer_states_set_exists().await? { return Ok(()); From 93554315c26332b71773c630e49bf9847dd7a55b Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 13:22:12 +1200 Subject: [PATCH 20/32] refactor: Use new synchronisation logic --- coordinator/src/main.rs | 52 +++++++++++------------------------------ 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 58c9ff110..68f6b5b65 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -5,11 +5,12 @@ use near_primitives::types::AccountId; use tokio::time::sleep; use tracing_subscriber::prelude::*; -use crate::block_streams::{synchronise_block_streams, BlockStreamsHandler}; -use crate::executors::{synchronise_executors, ExecutorsHandler}; +use crate::block_streams::BlockStreamsHandler; +use crate::executors::ExecutorsHandler; use crate::indexer_state::IndexerStateManager; use crate::redis::RedisClient; use crate::registry::Registry; +use crate::synchroniser::Synchroniser; mod block_streams; mod executors; @@ -55,6 +56,13 @@ async fn main() -> anyhow::Result<()> { let block_streams_handler = BlockStreamsHandler::connect(&block_streamer_url)?; let executors_handler = ExecutorsHandler::connect(&runner_url)?; let indexer_state_manager = Arc::new(IndexerStateManager::new(redis_client.clone())); + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &indexer_state_manager, + &redis_client, + ); tokio::spawn({ let indexer_state_manager = indexer_state_manager.clone(); @@ -64,43 +72,11 @@ async fn main() -> anyhow::Result<()> { loop { let indexer_registry = registry.fetch().await?; - // fetch state - // fetch executors - // fetch block streams - // - // iterate over the longest of state/registry - // pass Option of each to sync method - // sync method decides what to do - - // NOTE Rather than filtering them here, we can pass `IndexerState` to the sync methods, - // and let them decide what to do. That would be a bit cleaner? - // - // This will also allow us to determine when an Indexer has been deleted, rather than - // implicitly relying on the existance of executors/block_streams. This is going to be - // important for deprovisioning. indexer_state_manager.migrate(&indexer_registry).await?; - tokio::try_join!( - // NOTE this may need to be regactored in to a combined "synchronise" function. - // The addition of DataLayer provisioning makes the process a bit more stateful, i.e. - // we need to do provisioning first, wait till it completes, and can then kick off - // executor/block_stream sync processes - // - // It's probably still helpful to encapsulate the block_stream/executor sync methods, - // as they are quite involved, but call them from an overall synchronise method - // - // We'll need to store the `ProvisioningStatus` in Redis, so we know when to poll - synchronise_executors(&indexer_registry, &executors_handler), - synchronise_block_streams( - &indexer_registry, - &indexer_state_manager, - &redis_client, - &block_streams_handler - ), - async { - sleep(CONTROL_LOOP_THROTTLE_SECONDS).await; - Ok(()) - } - )?; + tokio::try_join!(synchroniser.sync(), async { + sleep(CONTROL_LOOP_THROTTLE_SECONDS).await; + Ok(()) + })?; } } From 834ae8a73a30d98669e635921574be943b54c8a8 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 13:24:37 +1200 Subject: [PATCH 21/32] fix: Treat existing/unsynced block streams as new --- coordinator/src/redis.rs | 11 ++++---- coordinator/src/synchroniser.rs | 50 ++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index fc0381c12..2d16bb675 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -53,11 +53,12 @@ impl RedisClientImpl { { tracing::debug!("SET: {:?}, {:?}", key, value); - let mut cmd = redis::cmd("SET"); - cmd.arg(key).arg(value); - cmd.query_async(&mut self.connection.clone()).await?; - - Ok(()) + redis::cmd("SET") + .arg(&key) + .arg(&value) + .query_async(&mut self.connection.clone()) + .await + .context(format!("SET: {key:?} {value:?}")) } pub async fn del(&self, key: K) -> anyhow::Result<()> diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index dbb57b67c..d2e596560 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -51,6 +51,21 @@ impl<'a> Synchroniser<'a> { } } + async fn start_new_block_stream(&self, config: &IndexerConfig) -> anyhow::Result<()> { + let start_block = match config.start_block { + StartBlock::Height(height) => height, + StartBlock::Latest => config.get_registry_version(), + StartBlock::Continue => { + tracing::warn!( + "Attempted to start new Block Stream with CONTINUE, using LATEST instead" + ); + config.get_registry_version() + } + }; + + self.block_streams_handler.start(start_block, config).await + } + #[instrument( skip_all, fields( @@ -65,18 +80,9 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - let start_block = match config.start_block { - StartBlock::Height(height) => height, - StartBlock::Latest => config.get_registry_version(), - StartBlock::Continue => { - tracing::warn!( - "Attempted to start new Block Stream with CONTINUE, using LATEST instead" - ); - config.get_registry_version() - } - }; - - if let Err(err) = self.block_streams_handler.start(start_block, config).await { + // FIX if this fails, then subsequent control loops will perpetually fail since the + // above will error with ALREADY_EXISTS + if let Err(err) = self.start_new_block_stream(config).await { tracing::error!(?err, "Failed to start Block Stream"); return Ok(()); } @@ -89,7 +95,6 @@ impl<'a> Synchroniser<'a> { async fn sync_existing_executor( &self, config: &IndexerConfig, - state: &IndexerState, executor: Option<&ExecutorInfo>, ) -> anyhow::Result<()> { if let Some(executor) = executor { @@ -118,7 +123,9 @@ impl<'a> Synchroniser<'a> { .await? .map(|height| height + 1) .unwrap_or_else(|| { - tracing::warn!("Failed to get continuation block height, using registry version"); + tracing::warn!( + "Failed to get continuation block height, using registry version instead" + ); config.get_registry_version() }); @@ -183,9 +190,13 @@ impl<'a> Synchroniser<'a> { } if state.block_stream_synced_at.is_none() { - // This would indicate that the state was inisialized before the Indexer was - // registered, which is currently not possible, but may be in future - anyhow::bail!("Existing Indexer has no `block_stream_synced_at` field") + // NOTE: A value of `None` would suggest that `state` was created before initialisation, + // which is currently not possible, but may be in future + tracing::warn!("Existing block stream has no previous sync state, treating as new"); + + self.start_new_block_stream(config).await?; + + return Ok(()); } if state.block_stream_synced_at.unwrap() != config.get_registry_version() { @@ -230,7 +241,7 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - if let Err(error) = self.sync_existing_executor(config, state, executor).await { + if let Err(error) = self.sync_existing_executor(config, executor).await { tracing::error!(?error, "Failed to sync executor"); return Ok(()); } @@ -243,7 +254,6 @@ impl<'a> Synchroniser<'a> { return Ok(()); } - // TODO handle failures self.state_manager.set_synced(config).await?; Ok(()) @@ -259,6 +269,8 @@ impl<'a> Synchroniser<'a> { for state in states { let config = registry.remove(&state.account_id, &state.function_name); + // TODO find a way to create the iterator once outside of the loop? previous attempts + // created weird behaviour with finding results let executor = executors.iter().find(|executor| { executor.account_id == state.account_id && executor.function_name == state.function_name From b3ceb362bf3881b5a47278602d9e35b92357e2a3 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 15:02:41 +1200 Subject: [PATCH 22/32] fix: Create iter everytime to avoid missing elements --- coordinator/src/registry.rs | 6 + coordinator/src/synchroniser.rs | 388 +++++++++++++++++++++++++++----- 2 files changed, 340 insertions(+), 54 deletions(-) diff --git a/coordinator/src/registry.rs b/coordinator/src/registry.rs index a5b197589..64175d58c 100644 --- a/coordinator/src/registry.rs +++ b/coordinator/src/registry.rs @@ -69,6 +69,12 @@ impl<'a> Iterator for IndexerRegistryIter<'a> { } } +impl std::ops::DerefMut for IndexerRegistry { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + impl std::ops::Deref for IndexerRegistry { type Target = HashMap>; fn deref(&self) -> &Self::Target { diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index d2e596560..71a1303a5 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -261,16 +261,14 @@ impl<'a> Synchroniser<'a> { async fn generate_synchronisation_states(&self) -> anyhow::Result> { let states = self.state_manager.list().await?; - let mut registry = self.registry.fetch().await?; let executors = self.executors_handler.list().await?; let block_streams = self.block_streams_handler.list().await?; + let mut registry = self.registry.fetch().await?; let mut sync_states = vec![]; for state in states { let config = registry.remove(&state.account_id, &state.function_name); - // TODO find a way to create the iterator once outside of the loop? previous attempts - // created weird behaviour with finding results let executor = executors.iter().find(|executor| { executor.account_id == state.account_id && executor.function_name == state.function_name @@ -339,6 +337,303 @@ mod test { use crate::registry::IndexerRegistry; + #[tokio::test] + async fn generates_sync_states() { + let existing_account_ids = vec![ + "account1.near".to_string(), + "account2.near".to_string(), + "account3.near".to_string(), + "account4.near".to_string(), + ]; + let new_account_ids = vec![ + "new_account1.near".to_string(), + "new_account2.near".to_string(), + ]; + let deleted_account_ids = vec![ + "deleted_account1.near".to_string(), + "deleted_account2.near".to_string(), + ]; + + let mut existing_indexer_configs: Vec = Vec::new(); + for (i, account_id) in existing_account_ids.iter().enumerate() { + for j in 1..=5 { + existing_indexer_configs.push(IndexerConfig { + account_id: account_id.parse().unwrap(), + function_name: format!("existing_indexer{}_{}", i + 1, j), + ..Default::default() + }); + } + } + + let mut new_indexer_configs: Vec = Vec::new(); + for (i, account_id) in new_account_ids.iter().enumerate() { + for j in 1..=3 { + new_indexer_configs.push(IndexerConfig { + account_id: account_id.parse().unwrap(), + function_name: format!("new_indexer{}_{}", i + 1, j), + ..Default::default() + }); + } + } + + let mut deleted_indexer_configs: Vec = Vec::new(); + for (i, account_id) in deleted_account_ids.iter().enumerate() { + for j in 1..=2 { + deleted_indexer_configs.push(IndexerConfig { + account_id: account_id.parse().unwrap(), + function_name: format!("deleted_indexer{}_{}", i + 1, j), + ..Default::default() + }); + } + } + + let mut indexer_registry = IndexerRegistry::new(); + for indexer in existing_indexer_configs + .iter() + .chain(new_indexer_configs.iter()) + { + indexer_registry + .entry(indexer.account_id.clone()) + .or_default() + .insert(indexer.function_name.clone(), indexer.clone()); + } + + let mut block_streams_handler = BlockStreamsHandler::default(); + let block_streams: Vec = existing_indexer_configs + .iter() + // generate some "randomness" + .rev() + .enumerate() + .map(|(i, indexer)| StreamInfo { + stream_id: format!("stream_id{}", i + 1), + account_id: indexer.account_id.to_string(), + function_name: indexer.function_name.clone(), + version: indexer.get_registry_version(), + }) + .collect(); + block_streams_handler + .expect_list() + .returning(move || Ok(block_streams.clone())); + + let mut executors_handler = ExecutorsHandler::default(); + let executors: Vec = existing_indexer_configs + .iter() + // generate some "randomness" + .rev() + .enumerate() + .map(|(i, indexer)| ExecutorInfo { + executor_id: format!("executor_id{}", i + 1), + account_id: indexer.account_id.to_string(), + function_name: indexer.function_name.clone(), + version: indexer.get_registry_version(), + status: "running".to_string(), + }) + .collect(); + + executors_handler + .expect_list() + .returning(move || Ok(executors.clone())); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + let states: Vec = existing_indexer_configs + .iter() + .map(|indexer| IndexerState { + account_id: indexer.account_id.clone(), + function_name: indexer.function_name.clone(), + block_stream_synced_at: Some(indexer.get_registry_version()), + enabled: true, + }) + .chain(deleted_indexer_configs.iter().map(|indexer| IndexerState { + account_id: indexer.account_id.clone(), + function_name: indexer.function_name.clone(), + block_stream_synced_at: Some(indexer.get_registry_version()), + enabled: true, + })) + .collect(); + state_manager + .expect_list() + .returning(move || Ok(states.clone())); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + let synchronisation_states = synchroniser + .generate_synchronisation_states() + .await + .unwrap(); + + let mut new_count = 0; + let mut existing_count = 0; + let mut deleted_count = 0; + + for state in &synchronisation_states { + match state { + SynchronisationState::New(_) => new_count += 1, + SynchronisationState::Existing(_, _, executor, block_stream) => { + assert!(executor.is_some(), "Executor should exist for the indexer"); + assert!( + block_stream.is_some(), + "Block stream should exist for the indexer" + ); + existing_count += 1; + } + SynchronisationState::Deleted(_, _, _) => { + deleted_count += 1; + } + } + } + + assert_eq!(new_count, 6); + assert_eq!(existing_count, 20); + assert_eq!(deleted_count, 4); + } + + #[tokio::test] + async fn handles_various_configurations() { + let new_indexer = IndexerConfig::default(); + let existing_indexer1 = IndexerConfig { + function_name: "test2".to_string(), + start_block: StartBlock::Latest, + ..Default::default() + }; + let existing_indexer2 = IndexerConfig { + account_id: "darunrs.near".parse().unwrap(), + function_name: "test1".to_string(), + start_block: StartBlock::Latest, + ..Default::default() + }; + + let indexer_registry = IndexerRegistry::from(&[( + new_indexer.account_id.clone(), + HashMap::from([ + (new_indexer.function_name.clone(), new_indexer.clone()), + ( + existing_indexer1.function_name.clone(), + existing_indexer1.clone(), + ), + ( + existing_indexer2.function_name.clone(), + existing_indexer2.clone(), + ), + ]), + )]); + + let mut block_streams_handler = BlockStreamsHandler::default(); + let existing_block_stream1 = StreamInfo { + stream_id: "stream_id1".to_string(), + account_id: existing_indexer1.account_id.to_string(), + function_name: existing_indexer1.function_name.clone(), + version: existing_indexer1.get_registry_version(), + }; + block_streams_handler + .expect_list() + .returning(move || Ok(vec![existing_block_stream1.clone()])); + block_streams_handler + .expect_start() + .with(eq(100), eq(new_indexer.clone())) + .returning(|_, _| Ok(())) + .once(); + block_streams_handler + .expect_start() + .with(always(), eq(existing_indexer1.clone())) + .never(); + block_streams_handler + .expect_start() + .with( + eq(existing_indexer2.get_registry_version()), + eq(existing_indexer2.clone()), + ) + .returning(|_, _| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + let existing_executor1 = ExecutorInfo { + executor_id: "executor_id1".to_string(), + account_id: existing_indexer1.account_id.to_string(), + function_name: existing_indexer1.function_name.clone(), + version: existing_indexer1.get_registry_version(), + status: "running".to_string(), + }; + executors_handler + .expect_list() + .returning(move || Ok(vec![existing_executor1.clone()])); + executors_handler + .expect_start() + .with(eq(new_indexer.clone())) + .returning(|_| Ok(())) + .once(); + executors_handler + .expect_start() + .with(eq(existing_indexer1.clone())) + .never(); + executors_handler + .expect_start() + .with(eq(existing_indexer2.clone())) + .returning(|_| Ok(())) + .once(); + + let mut registry = Registry::default(); + registry + .expect_fetch() + .returning(move || Ok(indexer_registry.clone())); + + let mut state_manager = IndexerStateManager::default(); + let existing_state1 = IndexerState { + account_id: existing_indexer1.account_id.clone(), + function_name: existing_indexer1.function_name.clone(), + block_stream_synced_at: Some(existing_indexer1.get_registry_version()), + enabled: true, + }; + let existing_state2 = IndexerState { + account_id: existing_indexer2.account_id.clone(), + function_name: existing_indexer2.function_name.clone(), + block_stream_synced_at: Some(existing_indexer2.get_registry_version()), + enabled: true, + }; + state_manager + .expect_list() + .returning(move || Ok(vec![existing_state1.clone(), existing_state2.clone()])); + state_manager + .expect_set_synced() + .with(eq(new_indexer)) + .returning(|_| Ok(())) + .once(); + state_manager + .expect_set_synced() + .with(eq(existing_indexer1)) + .returning(|_| Ok(())) + .once(); + state_manager + .expect_set_synced() + .with(eq(existing_indexer2)) + .returning(|_| Ok(())) + .once(); + + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser.sync().await.unwrap(); + } + mod new { use super::*; @@ -502,57 +797,6 @@ mod test { .await .unwrap(); } - - #[tokio::test] - async fn handles_synchronisation_failures() { - let config = IndexerConfig::default(); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler - .expect_start() - .with(eq(config.clone())) - .returning(|_| anyhow::bail!("")) - .once(); - executors_handler - .expect_start() - .with(eq(config.clone())) - .returning(|_| Ok(())) - .times(2); - - let mut block_streams_handler = BlockStreamsHandler::default(); - block_streams_handler - .expect_start() - .with(eq(100), eq(config.clone())) - .returning(|_, _| anyhow::bail!("")) - .once(); - block_streams_handler - .expect_start() - .with(eq(100), eq(config.clone())) - .returning(|_, _| Ok(())) - .once(); - - let mut state_manager = IndexerStateManager::default(); - state_manager - .expect_set_synced() - .with(eq(config.clone())) - .returning(|_| Ok(())) - .once(); - - let redis_client = RedisClient::default(); - let registry = Registry::default(); - - let synchroniser = Synchroniser::new( - &block_streams_handler, - &executors_handler, - ®istry, - &state_manager, - &redis_client, - ); - - synchroniser.sync_new_indexer(&config).await.unwrap(); // fail - synchroniser.sync_new_indexer(&config).await.unwrap(); // fail - synchroniser.sync_new_indexer(&config).await.unwrap(); // success - } } mod existing { @@ -717,6 +961,42 @@ mod test { synchroniser.sync().await.unwrap(); } + #[tokio::test] + async fn treats_unsynced_blocks_streams_as_new() { + let config = IndexerConfig::default(); + let state = IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: None, + enabled: true, + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_start() + .with(eq(100), eq(config.clone())) + .returning(|_, _| Ok(())) + .once(); + + let redis_client = RedisClient::default(); + let state_manager = IndexerStateManager::default(); + let executors_handler = ExecutorsHandler::default(); + let registry = Registry::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_existing_block_stream(&config, &state, None) + .await + .unwrap(); + } + #[tokio::test] async fn restarts_stopped_and_outdated_block_stream() { let config = IndexerConfig::default(); From fc7459b19b3dc63251fd983ab4d6959b1479e814 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 15:45:31 +1200 Subject: [PATCH 23/32] feat: Handle deleted indexer --- coordinator/src/indexer_state.rs | 15 +- coordinator/src/redis.rs | 36 ++++- coordinator/src/synchroniser.rs | 227 +++++++++++++------------------ 3 files changed, 135 insertions(+), 143 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 5585a8eb9..c72427860 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -24,7 +24,7 @@ pub struct OldIndexerState { // NOTE We'll need to add more fields here - is there a way to gracefully handle non-existant // fields during serde deserialization? it's annoying to always have to migrate this -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)] pub struct IndexerState { pub account_id: AccountId, pub function_name: String, @@ -38,6 +38,15 @@ pub struct IndexerState { pub enabled: bool, } +impl IndexerState { + // FIX `IndexerConfig` does not exist after an Indexer is deleted, and we need a way to + // construct the state key without it. But, this isn't ideal as we now have two places which + // define this key - we need to consolidate these somehow. + pub fn get_state_key(&self) -> String { + format!("{}/{}:state", self.account_id, self.function_name) + } +} + impl Default for IndexerState { fn default() -> Self { Self { @@ -129,6 +138,10 @@ impl IndexerStateManagerImpl { Ok(self.get_default_state(indexer_config)) } + pub async fn delete_state(&self, indexer_state: &IndexerState) -> anyhow::Result<()> { + self.redis_client.delete_indexer_state(indexer_state).await + } + async fn set_state( &self, indexer_config: &IndexerConfig, diff --git a/coordinator/src/redis.rs b/coordinator/src/redis.rs index 2d16bb675..74f536c70 100644 --- a/coordinator/src/redis.rs +++ b/coordinator/src/redis.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use anyhow::Context; use redis::{aio::ConnectionManager, FromRedisValue, ToRedisArgs}; -use crate::indexer_config::IndexerConfig; +use crate::{indexer_config::IndexerConfig, indexer_state::IndexerState}; #[cfg(test)] pub use MockRedisClientImpl as RedisClient; @@ -89,19 +89,34 @@ impl RedisClientImpl { .context(format!("SMEMBERS {set:?}")) } - pub async fn sadd(&self, set: S, value: V) -> anyhow::Result<()> + pub async fn sadd(&self, set: S, member: M) -> anyhow::Result<()> where S: ToRedisArgs + Debug + Send + Sync + 'static, - V: ToRedisArgs + Debug + Send + Sync + 'static, + M: ToRedisArgs + Debug + Send + Sync + 'static, { - tracing::debug!("SADD {set:?} {value:?}"); + tracing::debug!("SADD {set:?} {member:?}"); redis::cmd("SADD") .arg(&set) - .arg(&value) + .arg(&member) + .query_async(&mut self.connection.clone()) + .await + .context(format!("SADD {set:?} {member:?}")) + } + + pub async fn srem(&self, set: S, member: M) -> anyhow::Result<()> + where + S: ToRedisArgs + Debug + Send + Sync + 'static, + M: ToRedisArgs + Debug + Send + Sync + 'static, + { + tracing::debug!("SADD {set:?} {member:?}"); + + redis::cmd("SREM") + .arg(&set) + .arg(&member) .query_async(&mut self.connection.clone()) .await - .context(format!("SADD {set:?} {value:?}")) + .context(format!("SADD {set:?} {member:?}")) } pub async fn exists(&self, key: K) -> anyhow::Result @@ -154,6 +169,13 @@ impl RedisClientImpl { .await } + pub async fn delete_indexer_state(&self, state: &IndexerState) -> anyhow::Result<()> { + self.del(state.get_state_key()).await?; + + self.srem(Self::INDEXER_STATES_SET, state.get_state_key()) + .await + } + pub async fn list_indexer_states(&self) -> anyhow::Result> { let mut states = vec![]; @@ -213,6 +235,8 @@ mockall::mock! { V: ToRedisArgs + Debug + Send + Sync + 'static; pub async fn list_indexer_states(&self) -> anyhow::Result>; + + pub async fn delete_indexer_state(&self, state: &IndexerState) -> anyhow::Result<()>; } impl Clone for RedisClientImpl { diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 71a1303a5..744bb9737 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -259,6 +259,29 @@ impl<'a> Synchroniser<'a> { Ok(()) } + async fn sync_deleted_indexer( + &self, + state: &IndexerState, + executor: Option<&ExecutorInfo>, + block_stream: Option<&StreamInfo>, + ) -> anyhow::Result<()> { + if let Some(executor) = executor { + self.executors_handler + .stop(executor.executor_id.clone()) + .await?; + } + + if let Some(block_stream) = block_stream { + self.block_streams_handler + .stop(block_stream.stream_id.clone()) + .await?; + } + + self.state_manager.delete_state(state).await?; + + Ok(()) + } + async fn generate_synchronisation_states(&self) -> anyhow::Result> { let states = self.state_manager.list().await?; let executors = self.executors_handler.list().await?; @@ -318,8 +341,9 @@ impl<'a> Synchroniser<'a> { ) .await?; } - SynchronisationState::Deleted(..) => { - // handle deleted + SynchronisationState::Deleted(state, executor, block_stream) => { + self.sync_deleted_indexer(&state, executor.as_ref(), block_stream.as_ref()) + .await?; } } } @@ -500,140 +524,6 @@ mod test { assert_eq!(deleted_count, 4); } - #[tokio::test] - async fn handles_various_configurations() { - let new_indexer = IndexerConfig::default(); - let existing_indexer1 = IndexerConfig { - function_name: "test2".to_string(), - start_block: StartBlock::Latest, - ..Default::default() - }; - let existing_indexer2 = IndexerConfig { - account_id: "darunrs.near".parse().unwrap(), - function_name: "test1".to_string(), - start_block: StartBlock::Latest, - ..Default::default() - }; - - let indexer_registry = IndexerRegistry::from(&[( - new_indexer.account_id.clone(), - HashMap::from([ - (new_indexer.function_name.clone(), new_indexer.clone()), - ( - existing_indexer1.function_name.clone(), - existing_indexer1.clone(), - ), - ( - existing_indexer2.function_name.clone(), - existing_indexer2.clone(), - ), - ]), - )]); - - let mut block_streams_handler = BlockStreamsHandler::default(); - let existing_block_stream1 = StreamInfo { - stream_id: "stream_id1".to_string(), - account_id: existing_indexer1.account_id.to_string(), - function_name: existing_indexer1.function_name.clone(), - version: existing_indexer1.get_registry_version(), - }; - block_streams_handler - .expect_list() - .returning(move || Ok(vec![existing_block_stream1.clone()])); - block_streams_handler - .expect_start() - .with(eq(100), eq(new_indexer.clone())) - .returning(|_, _| Ok(())) - .once(); - block_streams_handler - .expect_start() - .with(always(), eq(existing_indexer1.clone())) - .never(); - block_streams_handler - .expect_start() - .with( - eq(existing_indexer2.get_registry_version()), - eq(existing_indexer2.clone()), - ) - .returning(|_, _| Ok(())) - .once(); - - let mut executors_handler = ExecutorsHandler::default(); - let existing_executor1 = ExecutorInfo { - executor_id: "executor_id1".to_string(), - account_id: existing_indexer1.account_id.to_string(), - function_name: existing_indexer1.function_name.clone(), - version: existing_indexer1.get_registry_version(), - status: "running".to_string(), - }; - executors_handler - .expect_list() - .returning(move || Ok(vec![existing_executor1.clone()])); - executors_handler - .expect_start() - .with(eq(new_indexer.clone())) - .returning(|_| Ok(())) - .once(); - executors_handler - .expect_start() - .with(eq(existing_indexer1.clone())) - .never(); - executors_handler - .expect_start() - .with(eq(existing_indexer2.clone())) - .returning(|_| Ok(())) - .once(); - - let mut registry = Registry::default(); - registry - .expect_fetch() - .returning(move || Ok(indexer_registry.clone())); - - let mut state_manager = IndexerStateManager::default(); - let existing_state1 = IndexerState { - account_id: existing_indexer1.account_id.clone(), - function_name: existing_indexer1.function_name.clone(), - block_stream_synced_at: Some(existing_indexer1.get_registry_version()), - enabled: true, - }; - let existing_state2 = IndexerState { - account_id: existing_indexer2.account_id.clone(), - function_name: existing_indexer2.function_name.clone(), - block_stream_synced_at: Some(existing_indexer2.get_registry_version()), - enabled: true, - }; - state_manager - .expect_list() - .returning(move || Ok(vec![existing_state1.clone(), existing_state2.clone()])); - state_manager - .expect_set_synced() - .with(eq(new_indexer)) - .returning(|_| Ok(())) - .once(); - state_manager - .expect_set_synced() - .with(eq(existing_indexer1)) - .returning(|_| Ok(())) - .once(); - state_manager - .expect_set_synced() - .with(eq(existing_indexer2)) - .returning(|_| Ok(())) - .once(); - - let redis_client = RedisClient::default(); - - let synchroniser = Synchroniser::new( - &block_streams_handler, - &executors_handler, - ®istry, - &state_manager, - &redis_client, - ); - - synchroniser.sync().await.unwrap(); - } - mod new { use super::*; @@ -1231,4 +1121,69 @@ mod test { .unwrap(); } } + + mod deleted { + use super::*; + + #[tokio::test] + async fn stops_and_deletes() { + let config = IndexerConfig::default(); + let state = IndexerState { + account_id: config.account_id.clone(), + function_name: config.function_name.clone(), + block_stream_synced_at: Some(config.get_registry_version()), + enabled: false, + }; + let executor = ExecutorInfo { + executor_id: "executor_id".to_string(), + account_id: config.account_id.to_string(), + function_name: config.function_name.clone(), + version: config.get_registry_version(), + status: "running".to_string(), + }; + let block_stream = StreamInfo { + stream_id: "stream_id".to_string(), + account_id: config.account_id.to_string(), + function_name: config.function_name.clone(), + version: config.get_registry_version(), + }; + + let mut block_streams_handler = BlockStreamsHandler::default(); + block_streams_handler + .expect_stop() + .with(eq("stream_id".to_string())) + .returning(|_| Ok(())) + .once(); + + let mut executors_handler = ExecutorsHandler::default(); + executors_handler + .expect_stop() + .with(eq("executor_id".to_string())) + .returning(|_| Ok(())) + .once(); + + let mut state_manager = IndexerStateManager::default(); + state_manager + .expect_delete_state() + .with(eq(state.clone())) + .returning(|_| Ok(())) + .once(); + + let registry = Registry::default(); + let redis_client = RedisClient::default(); + + let synchroniser = Synchroniser::new( + &block_streams_handler, + &executors_handler, + ®istry, + &state_manager, + &redis_client, + ); + + synchroniser + .sync_deleted_indexer(&state, Some(&executor), Some(&block_stream)) + .await + .unwrap(); + } + } } From 9eb98937e7ab217d78578a3377a1491e7657ae8b Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 15:47:22 +1200 Subject: [PATCH 24/32] chore: Remove notes --- TODO.md | 7 ------- coordinator/src/executors/handler.rs | 1 - coordinator/src/indexer_state.rs | 19 ------------------- .../src/server/indexer_manager_service.rs | 2 -- runner/protos/data-layer.proto | 14 -------------- runner/src/indexer/indexer.ts | 3 --- 6 files changed, 46 deletions(-) delete mode 100644 TODO.md diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 24838d516..000000000 --- a/TODO.md +++ /dev/null @@ -1,7 +0,0 @@ -- Implement `DataLayerService` gRPC, to provide control over provisioning, ignoring de-provisioning for now -- Remove implicit provisioning from execution -- Refactor Coordinator executor/block_stream synchronization so that it is a single stateful process? -- Integrate `DataLayerService` in to Coordinator and add synchronization step to manage this -- Add gRPC method to deprovision data layer -- Refactor Coordinator synchronization so that we capture indexer "removals" -- Add de-provisioning to Coordinator synchronization diff --git a/coordinator/src/executors/handler.rs b/coordinator/src/executors/handler.rs index a62886430..1282e9598 100644 --- a/coordinator/src/executors/handler.rs +++ b/coordinator/src/executors/handler.rs @@ -32,7 +32,6 @@ impl ExecutorsHandlerImpl { } pub async fn list(&self) -> anyhow::Result> { - // TODO remove retry logic and just let it propogate up to the main loop exponential_retry(|| async { let response = self .client diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index c72427860..93396ea4b 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -22,26 +22,15 @@ pub struct OldIndexerState { pub enabled: bool, } -// NOTE We'll need to add more fields here - is there a way to gracefully handle non-existant -// fields during serde deserialization? it's annoying to always have to migrate this #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)] pub struct IndexerState { pub account_id: AccountId, pub function_name: String, - // NOTE this no longer needs to be optional, because the existance of the state object implies - // that the stream has been started - // - // but we might need to write the state object before we register, so therefore may need this - // to be none, or we might create state before synchronisation for other reasons, so maybe its - // best to keep it optional pub block_stream_synced_at: Option, pub enabled: bool, } impl IndexerState { - // FIX `IndexerConfig` does not exist after an Indexer is deleted, and we need a way to - // construct the state key without it. But, this isn't ideal as we now have two places which - // define this key - we need to consolidate these somehow. pub fn get_state_key(&self) -> String { format!("{}/{}:state", self.account_id, self.function_name) } @@ -73,14 +62,6 @@ impl IndexerStateManagerImpl { Self { redis_client } } - // NOTE the synchronisation logic uses the existence of state to signify "existing", so by - // using the current registry we assume that all current indexers have be "seen" before, and - // can therefore be treated as "existing", but if an indexer is registered during deployment of - // this, we will incorrectly treat a "new" indexer as "existing". - // - // So what does this mean, we'll miss the "new" flow, which eventually include provisioning, - // but since it does not yet, I think this is actually fine. The "existing" flow will just - // start the indexer as is. pub async fn migrate(&self, registry: &IndexerRegistry) -> anyhow::Result<()> { if self.redis_client.indexer_states_set_exists().await? { return Ok(()); diff --git a/coordinator/src/server/indexer_manager_service.rs b/coordinator/src/server/indexer_manager_service.rs index 412124a15..36a91ca31 100644 --- a/coordinator/src/server/indexer_manager_service.rs +++ b/coordinator/src/server/indexer_manager_service.rs @@ -40,8 +40,6 @@ impl indexer_manager::indexer_manager_server::IndexerManager for IndexerManagerS .parse() .map_err(|_| Status::invalid_argument("Invalid account ID"))?; - // NOTE we probably need a way to write the state before publishing, so that we can trigger - // features before it starts let indexer_config = self .registry .fetch_indexer(account_id, request.function_name) diff --git a/runner/protos/data-layer.proto b/runner/protos/data-layer.proto index 97445e01f..eaef8034a 100644 --- a/runner/protos/data-layer.proto +++ b/runner/protos/data-layer.proto @@ -2,19 +2,7 @@ syntax = "proto3"; package data_layer; -// NOTE this will eventually be expanded to handle more granular operations -// such as truncating the database, or perhaps running migrations service DataLayer { - // NOTE As this process can take a while, we need to handle this asynchronously. - // Therefore, this will trigger the provisioning process and return immediately. - // The client can then poll the CheckProvisioningStatus method to determine when the - // provisioning process has completed. - // - // Maybe we should call this TriggerProvisioning instead of Provision? - // - // Need to figure out how this process will actually be kicked off asynchronously, - // can we just fire a promise or do we need worker threads? - // Provisions the data layer (PostgreSQL + Hasura) rpc Provision (ProvisionRequest) returns (ProvisionResponse); @@ -23,8 +11,6 @@ service DataLayer { } message ProvisionRequest { - // TODO This is only a partial `IndexerConfig`, which may pose an issue as - // all the provisioning methods expect a full `IndexerConfig` string account_id = 1; string function_name = 2; string schema = 3; diff --git a/runner/src/indexer/indexer.ts b/runner/src/indexer/indexer.ts index 18f9f3497..344deae86 100644 --- a/runner/src/indexer/indexer.ts +++ b/runner/src/indexer/indexer.ts @@ -48,9 +48,6 @@ const defaultConfig: Config = { hasuraEndpoint: process.env.HASURA_ENDPOINT, }; -// NOTE We need to remove provisioning from here, -// I'm thinking we remove knowledge of it entirely? and just inject he db parameters -// in from `StreamHandler`? Maybe a good opportunity to rename `StreamHandler` in to something more appropriate export default class Indexer { DEFAULT_HASURA_ROLE: string; IS_FIRST_EXECUTION: boolean = true; From a7ec4317faaefdfa92aa1dfe838457105c0fb847 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 15:49:26 +1200 Subject: [PATCH 25/32] refactor: Remove old synchronisation code --- coordinator/src/block_streams/mod.rs | 5 - coordinator/src/block_streams/synchronise.rs | 651 ------------------ .../handler.rs => block_streams_handler.rs} | 0 coordinator/src/executors/mod.rs | 5 - coordinator/src/executors/synchronise.rs | 238 ------- .../handler.rs => executors_handler.rs} | 0 coordinator/src/indexer_state.rs | 244 +------ coordinator/src/main.rs | 8 +- coordinator/src/synchroniser.rs | 4 +- 9 files changed, 7 insertions(+), 1148 deletions(-) delete mode 100644 coordinator/src/block_streams/mod.rs delete mode 100644 coordinator/src/block_streams/synchronise.rs rename coordinator/src/{block_streams/handler.rs => block_streams_handler.rs} (100%) delete mode 100644 coordinator/src/executors/mod.rs delete mode 100644 coordinator/src/executors/synchronise.rs rename coordinator/src/{executors/handler.rs => executors_handler.rs} (100%) diff --git a/coordinator/src/block_streams/mod.rs b/coordinator/src/block_streams/mod.rs deleted file mode 100644 index cd8b6fd96..000000000 --- a/coordinator/src/block_streams/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod handler; -mod synchronise; - -pub use handler::BlockStreamsHandler; -pub use synchronise::synchronise_block_streams; diff --git a/coordinator/src/block_streams/synchronise.rs b/coordinator/src/block_streams/synchronise.rs deleted file mode 100644 index 5a8af2ecb..000000000 --- a/coordinator/src/block_streams/synchronise.rs +++ /dev/null @@ -1,651 +0,0 @@ -use registry_types::StartBlock; - -use crate::indexer_config::IndexerConfig; -use crate::indexer_state::{IndexerStateManager, SyncStatus}; -use crate::redis::RedisClient; -use crate::registry::IndexerRegistry; - -use super::handler::{BlockStreamsHandler, StreamInfo}; - -pub async fn synchronise_block_streams( - indexer_registry: &IndexerRegistry, - indexer_manager: &IndexerStateManager, - redis_client: &RedisClient, - block_streams_handler: &BlockStreamsHandler, -) -> anyhow::Result<()> { - let mut active_block_streams = block_streams_handler.list().await?; - - for indexer_config in indexer_registry.iter() { - let active_block_stream = active_block_streams - .iter() - .position(|stream| { - stream.account_id == *indexer_config.account_id - && stream.function_name == indexer_config.function_name - }) - .map(|index| active_block_streams.swap_remove(index)); - - let _ = synchronise_block_stream( - active_block_stream, - indexer_config, - indexer_manager, - redis_client, - block_streams_handler, - ) - .await - .map_err(|err| { - tracing::error!( - account_id = indexer_config.account_id.as_str(), - function_name = indexer_config.function_name, - version = indexer_config.get_registry_version(), - "failed to sync block stream: {err:?}" - ) - }); - } - - for unregistered_block_stream in active_block_streams { - tracing::info!( - account_id = unregistered_block_stream.account_id.as_str(), - function_name = unregistered_block_stream.function_name, - version = unregistered_block_stream.version, - "Stopping unregistered block stream" - ); - - block_streams_handler - .stop(unregistered_block_stream.stream_id) - .await?; - } - - Ok(()) -} - -#[tracing::instrument( - skip_all, - fields( - account_id = %indexer_config.account_id, - function_name = indexer_config.function_name, - version = indexer_config.get_registry_version() - ) -)] -async fn synchronise_block_stream( - active_block_stream: Option, - indexer_config: &IndexerConfig, - indexer_manager: &IndexerStateManager, - redis_client: &RedisClient, - block_streams_handler: &BlockStreamsHandler, -) -> anyhow::Result<()> { - if let Some(active_block_stream) = active_block_stream { - if active_block_stream.version == indexer_config.get_registry_version() { - return Ok(()); - } - - tracing::info!( - previous_version = active_block_stream.version, - "Stopping outdated block stream" - ); - - block_streams_handler - .stop(active_block_stream.stream_id) - .await?; - } - - let sync_status = indexer_manager - .get_block_stream_sync_status(indexer_config) - .await?; - - clear_block_stream_if_needed(&sync_status, indexer_config, redis_client).await?; - - let start_block_height = - determine_start_block_height(&sync_status, indexer_config, redis_client).await?; - - block_streams_handler - .start(start_block_height, indexer_config) - .await?; - - indexer_manager - .set_block_stream_synced(indexer_config) - .await?; - - Ok(()) -} - -async fn clear_block_stream_if_needed( - sync_status: &SyncStatus, - indexer_config: &IndexerConfig, - redis_client: &RedisClient, -) -> anyhow::Result<()> { - if matches!(sync_status, SyncStatus::Synced | SyncStatus::New) - || indexer_config.start_block == StartBlock::Continue - { - return Ok(()); - } - - tracing::info!("Clearing redis stream"); - - redis_client.clear_block_stream(indexer_config).await -} - -async fn determine_start_block_height( - sync_status: &SyncStatus, - indexer_config: &IndexerConfig, - redis_client: &RedisClient, -) -> anyhow::Result { - if sync_status == &SyncStatus::Synced { - let height = get_continuation_block_height(indexer_config, redis_client).await?; - - tracing::info!(height, "Resuming block stream"); - - return Ok(height); - } - - let height = match indexer_config.start_block { - StartBlock::Latest => Ok(indexer_config.get_registry_version()), - StartBlock::Height(height) => Ok(height), - StartBlock::Continue => get_continuation_block_height(indexer_config, redis_client).await, - }?; - - tracing::info!(height, "Starting block stream"); - - Ok(height) -} - -async fn get_continuation_block_height( - indexer_config: &IndexerConfig, - redis_client: &RedisClient, -) -> anyhow::Result { - redis_client - .get_last_published_block(indexer_config) - .await? - .map(|height| height + 1) - .ok_or(anyhow::anyhow!("Indexer has no `last_published_block`")) -} - -#[cfg(test)] -mod tests { - use super::*; - - use std::collections::HashMap; - - use mockall::predicate; - use registry_types::{Rule, Status}; - - #[tokio::test] - async fn resumes_previously_synced_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Height(100), - }; - - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Synced)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_last_published_block() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(Some(500))) - .once(); - redis_client.expect_clear_block_stream().never(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler - .expect_start() - .with(predicate::eq(501), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn starts_stream_with_latest() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Latest, - }; - - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Outdated)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_clear_block_stream() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler.expect_stop().never(); - block_stream_handler - .expect_start() - .with(predicate::eq(200), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn starts_stream_with_height() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Height(100), - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Outdated)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_clear_block_stream() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler.expect_stop().never(); - block_stream_handler - .expect_start() - .with(predicate::eq(100), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn starts_stream_with_continue() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Continue, - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Outdated)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_last_published_block() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(Some(100))) - .once(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler.expect_stop().never(); - block_stream_handler - .expect_start() - .with(predicate::eq(101), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn stops_stream_not_in_registry() { - let indexer_registry = IndexerRegistry::from(&[]); - - let redis_client = RedisClient::default(); - - let mock_indexer_manager = IndexerStateManager::default(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| { - Ok(vec![block_streamer::StreamInfo { - stream_id: "stream_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - version: 1, - }]) - }); - block_stream_handler - .expect_stop() - .with(predicate::eq("stream_id".to_string())) - .returning(|_| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn ignores_synced_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 101, - updated_at_block_height: None, - start_block: StartBlock::Latest, - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let redis_client = RedisClient::default(); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Synced)); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| { - Ok(vec![block_streamer::StreamInfo { - stream_id: "stream_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - version: 101, - }]) - }); - block_stream_handler.expect_stop().never(); - block_stream_handler.expect_start().never(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn restarts_unsynced_streams() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 101, - updated_at_block_height: Some(199), - start_block: StartBlock::Height(1000), - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Outdated)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_clear_block_stream() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| { - Ok(vec![block_streamer::StreamInfo { - stream_id: "stream_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - version: 101, - }]) - }); - block_stream_handler - .expect_stop() - .with(predicate::eq("stream_id".to_string())) - .returning(|_| Ok(())) - .once(); - block_stream_handler - .expect_start() - .with(predicate::eq(1000), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn skips_stream_without_last_published_block() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 101, - updated_at_block_height: Some(200), - start_block: StartBlock::Continue, - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::Outdated)); - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_last_published_block() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| anyhow::bail!("no last_published_block")) - .once(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler.expect_stop().never(); - block_stream_handler.expect_start().never(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn starts_new_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 101, - updated_at_block_height: None, - start_block: StartBlock::Height(50), - }; - let indexer_registry = IndexerRegistry::from(&[( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )]); - - let mut mock_indexer_manager = IndexerStateManager::default(); - mock_indexer_manager - .expect_get_block_stream_sync_status() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(SyncStatus::New)); - mock_indexer_manager - .expect_set_block_stream_synced() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(())) - .once(); - - let redis_client = RedisClient::default(); - - let mut block_stream_handler = BlockStreamsHandler::default(); - block_stream_handler.expect_list().returning(|| Ok(vec![])); - block_stream_handler.expect_stop().never(); - block_stream_handler - .expect_start() - .with(predicate::eq(50), predicate::eq(indexer_config)) - .returning(|_, _| Ok(())) - .once(); - - synchronise_block_streams( - &indexer_registry, - &mock_indexer_manager, - &redis_client, - &block_stream_handler, - ) - .await - .unwrap(); - } -} diff --git a/coordinator/src/block_streams/handler.rs b/coordinator/src/block_streams_handler.rs similarity index 100% rename from coordinator/src/block_streams/handler.rs rename to coordinator/src/block_streams_handler.rs diff --git a/coordinator/src/executors/mod.rs b/coordinator/src/executors/mod.rs deleted file mode 100644 index 1b68609c6..000000000 --- a/coordinator/src/executors/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod handler; -mod synchronise; - -pub use handler::ExecutorsHandler; -pub use synchronise::synchronise_executors; diff --git a/coordinator/src/executors/synchronise.rs b/coordinator/src/executors/synchronise.rs deleted file mode 100644 index 2b22c8a93..000000000 --- a/coordinator/src/executors/synchronise.rs +++ /dev/null @@ -1,238 +0,0 @@ -use crate::indexer_config::IndexerConfig; -use crate::registry::IndexerRegistry; - -use super::handler::{ExecutorInfo, ExecutorsHandler}; - -pub async fn synchronise_executors( - indexer_registry: &IndexerRegistry, - executors_handler: &ExecutorsHandler, -) -> anyhow::Result<()> { - let mut active_executors = executors_handler.list().await?; - - for indexer_config in indexer_registry.iter() { - let active_executor = active_executors - .iter() - .position(|stream| { - stream.account_id == *indexer_config.account_id - && stream.function_name == indexer_config.function_name - }) - .map(|index| active_executors.swap_remove(index)); - - let _ = synchronise_executor(active_executor, indexer_config, executors_handler) - .await - .map_err(|err| { - tracing::error!( - account_id = indexer_config.account_id.as_str(), - function_name = indexer_config.function_name, - version = indexer_config.get_registry_version(), - "failed to sync executor: {err:?}" - ) - }); - } - - for unregistered_executor in active_executors { - tracing::info!( - account_id = unregistered_executor.account_id.as_str(), - function_name = unregistered_executor.function_name, - registry_version = unregistered_executor.version, - "Stopping unregistered executor" - ); - - executors_handler - .stop(unregistered_executor.executor_id) - .await?; - } - - Ok(()) -} - -#[tracing::instrument( - skip_all, - fields( - account_id = %indexer_config.account_id, - function_name = indexer_config.function_name, - version = indexer_config.get_registry_version() - ) -)] -async fn synchronise_executor( - active_executor: Option, - indexer_config: &IndexerConfig, - executors_handler: &ExecutorsHandler, -) -> anyhow::Result<()> { - let registry_version = indexer_config.get_registry_version(); - - if let Some(active_executor) = active_executor { - if active_executor.version == registry_version { - return Ok(()); - } - - tracing::info!("Stopping outdated executor"); - - executors_handler.stop(active_executor.executor_id).await?; - } - - tracing::info!("Starting new executor"); - - executors_handler.start(indexer_config).await?; - - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - - use std::collections::HashMap; - - use mockall::predicate; - use registry_types::{Rule, StartBlock, Status}; - - use crate::indexer_config::IndexerConfig; - - #[tokio::test] - async fn starts_executor() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: "code".to_string(), - schema: "schema".to_string(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: None, - start_block: StartBlock::Height(100), - }; - let indexer_registry = IndexerRegistry(HashMap::from([( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )])); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler.expect_list().returning(|| Ok(vec![])); - executors_handler - .expect_start() - .with(predicate::eq(indexer_config)) - .returning(|_| Ok(())) - .once(); - - synchronise_executors(&indexer_registry, &executors_handler) - .await - .unwrap(); - } - - #[tokio::test] - async fn restarts_executor_when_registry_version_differs() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: "code".to_string(), - schema: "schema".to_string(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(2), - start_block: StartBlock::Height(100), - }; - let indexer_registry = IndexerRegistry(HashMap::from([( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), indexer_config.clone())]), - )])); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler.expect_list().returning(|| { - Ok(vec![runner::ExecutorInfo { - executor_id: "executor_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - status: "running".to_string(), - version: 1, - }]) - }); - executors_handler - .expect_stop() - .with(predicate::eq("executor_id".to_string())) - .returning(|_| Ok(())) - .once(); - - executors_handler - .expect_start() - .with(predicate::eq(indexer_config)) - .returning(|_| Ok(())) - .once(); - - synchronise_executors(&indexer_registry, &executors_handler) - .await - .unwrap(); - } - - #[tokio::test] - async fn ignores_executor_with_matching_registry_version() { - let indexer_registry = IndexerRegistry(HashMap::from([( - "morgs.near".parse().unwrap(), - HashMap::from([( - "test".to_string(), - IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: "code".to_string(), - schema: "schema".to_string(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(2), - start_block: StartBlock::Height(100), - }, - )]), - )])); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler.expect_list().returning(|| { - Ok(vec![runner::ExecutorInfo { - executor_id: "executor_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - status: "running".to_string(), - version: 2, - }]) - }); - executors_handler.expect_stop().never(); - - executors_handler.expect_start().never(); - - synchronise_executors(&indexer_registry, &executors_handler) - .await - .unwrap(); - } - - #[tokio::test] - async fn stops_executor_not_in_registry() { - let indexer_registry = IndexerRegistry::from(&[]); - - let mut executors_handler = ExecutorsHandler::default(); - executors_handler.expect_list().returning(|| { - Ok(vec![runner::ExecutorInfo { - executor_id: "executor_id".to_string(), - account_id: "morgs.near".to_string(), - function_name: "test".to_string(), - status: "running".to_string(), - version: 2, - }]) - }); - - executors_handler - .expect_stop() - .with(predicate::eq("executor_id".to_string())) - .returning(|_| Ok(())) - .once(); - - synchronise_executors(&indexer_registry, &executors_handler) - .await - .unwrap(); - } -} diff --git a/coordinator/src/executors/handler.rs b/coordinator/src/executors_handler.rs similarity index 100% rename from coordinator/src/executors/handler.rs rename to coordinator/src/executors_handler.rs diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 93396ea4b..18752e99e 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -9,13 +9,6 @@ use crate::indexer_config::IndexerConfig; use crate::redis::RedisClient; use crate::registry::IndexerRegistry; -#[derive(Debug, PartialEq, Eq)] -pub enum SyncStatus { - Synced, - Outdated, - New, -} - #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct OldIndexerState { pub block_stream_synced_at: Option, @@ -158,66 +151,6 @@ impl IndexerStateManagerImpl { Ok(()) } - pub async fn filter_disabled_indexers( - &self, - indexer_registry: &IndexerRegistry, - ) -> anyhow::Result { - let mut filtered_registry = IndexerRegistry::new(); - - for indexer_config in indexer_registry.iter() { - let indexer_state = self.get_state(indexer_config).await?; - - if indexer_state.enabled { - filtered_registry - .0 - .entry(indexer_config.account_id.clone()) - .or_default() - .insert(indexer_config.function_name.clone(), indexer_config.clone()); - } - } - - Ok(filtered_registry) - } - - pub async fn get_block_stream_sync_status( - &self, - indexer_config: &IndexerConfig, - ) -> anyhow::Result { - let indexer_state = self.get_state(indexer_config).await?; - - if indexer_state.block_stream_synced_at.is_none() { - return Ok(SyncStatus::New); - } - - match indexer_config - .get_registry_version() - .cmp(&indexer_state.block_stream_synced_at.unwrap()) - { - Ordering::Equal => Ok(SyncStatus::Synced), - Ordering::Greater => Ok(SyncStatus::Outdated), - Ordering::Less => { - tracing::warn!( - "Found stream with version greater than registry, treating as outdated" - ); - - Ok(SyncStatus::Outdated) - } - } - } - - pub async fn set_block_stream_synced( - &self, - indexer_config: &IndexerConfig, - ) -> anyhow::Result<()> { - let mut indexer_state = self.get_state(indexer_config).await?; - - indexer_state.block_stream_synced_at = Some(indexer_config.get_registry_version()); - - self.set_state(indexer_config, indexer_state).await?; - - Ok(()) - } - pub async fn list(&self) -> anyhow::Result> { self.redis_client .list_indexer_states() @@ -327,181 +260,6 @@ mod tests { indexer_manager.migrate(®istry).await.unwrap(); } - #[tokio::test] - async fn filters_disabled_indexers() { - let morgs_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Height(100), - }; - let darunrs_config = IndexerConfig { - account_id: "darunrs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: None, - start_block: StartBlock::Height(100), - }; - - let indexer_registry = IndexerRegistry::from(&[ - ( - "morgs.near".parse().unwrap(), - HashMap::from([("test".to_string(), morgs_config.clone())]), - ), - ( - "darunrs.near".parse().unwrap(), - HashMap::from([("test".to_string(), darunrs_config.clone())]), - ), - ]); - - let mut mock_redis_client = RedisClient::default(); - mock_redis_client - .expect_get_indexer_state() - .with(predicate::eq(morgs_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 200, "enabled": true }) - .to_string(), - )) - }) - .once(); - mock_redis_client - .expect_get_indexer_state() - .with(predicate::eq(darunrs_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 1, "enabled": false }) - .to_string(), - )) - }) - .once(); - - let indexer_manager = IndexerStateManagerImpl::new(mock_redis_client); - - let filtered_registry = indexer_manager - .filter_disabled_indexers(&indexer_registry) - .await - .unwrap(); - - assert!(filtered_registry.contains_key(&morgs_config.account_id)); - } - - #[tokio::test] - pub async fn outdated_block_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Continue, - }; - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_indexer_state() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 300, "enabled": true }) - .to_string(), - )) - }); - - let indexer_manager = IndexerStateManagerImpl::new(redis_client); - let result = indexer_manager - .get_block_stream_sync_status(&indexer_config) - .await - .unwrap(); - - assert_eq!(result, SyncStatus::Outdated); - } - - #[tokio::test] - pub async fn synced_block_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: Some(200), - start_block: StartBlock::Continue, - }; - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_indexer_state() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| { - Ok(Some( - serde_json::json!({ "block_stream_synced_at": 200, "enabled": true }) - .to_string(), - )) - }); - - let indexer_manager = IndexerStateManagerImpl::new(redis_client); - let result = indexer_manager - .get_block_stream_sync_status(&indexer_config) - .await - .unwrap(); - - assert_eq!(result, SyncStatus::Synced); - } - - #[tokio::test] - pub async fn new_block_stream() { - let indexer_config = IndexerConfig { - account_id: "morgs.near".parse().unwrap(), - function_name: "test".to_string(), - code: String::new(), - schema: String::new(), - rule: Rule::ActionAny { - affected_account_id: "queryapi.dataplatform.near".to_string(), - status: Status::Any, - }, - created_at_block_height: 1, - updated_at_block_height: None, - start_block: StartBlock::Continue, - }; - - let mut redis_client = RedisClient::default(); - redis_client - .expect_get_indexer_state() - .with(predicate::eq(indexer_config.clone())) - .returning(|_| Ok(None)); - - let indexer_manager = IndexerStateManagerImpl::new(redis_client); - let result = indexer_manager - .get_block_stream_sync_status(&indexer_config) - .await - .unwrap(); - - assert_eq!(result, SyncStatus::New); - } - #[tokio::test] pub async fn disable_indexer() { let indexer_config = IndexerConfig { @@ -543,7 +301,7 @@ mod tests { let indexer_manager = IndexerStateManagerImpl::new(redis_client); indexer_manager - .set_enabled(&indexer_config.into(), false) + .set_enabled(&indexer_config, false) .await .unwrap(); } diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 68f6b5b65..7a9730453 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -5,15 +5,15 @@ use near_primitives::types::AccountId; use tokio::time::sleep; use tracing_subscriber::prelude::*; -use crate::block_streams::BlockStreamsHandler; -use crate::executors::ExecutorsHandler; +use crate::block_streams_handler::BlockStreamsHandler; +use crate::executors_handler::ExecutorsHandler; use crate::indexer_state::IndexerStateManager; use crate::redis::RedisClient; use crate::registry::Registry; use crate::synchroniser::Synchroniser; -mod block_streams; -mod executors; +mod block_streams_handler; +mod executors_handler; mod indexer_config; mod indexer_state; mod redis; diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 744bb9737..5a6b0ce23 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -5,8 +5,8 @@ use runner::ExecutorInfo; use tracing::instrument; use crate::{ - block_streams::BlockStreamsHandler, - executors::ExecutorsHandler, + block_streams_handler::BlockStreamsHandler, + executors_handler::ExecutorsHandler, indexer_config::IndexerConfig, indexer_state::{IndexerState, IndexerStateManager}, redis::RedisClient, From d57ff580a3122c89b23c2042aaac71ce84efe6d2 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 15:58:31 +1200 Subject: [PATCH 26/32] feat: Add error context to executors_handler --- coordinator/src/executors_handler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coordinator/src/executors_handler.rs b/coordinator/src/executors_handler.rs index 1282e9598..28717b924 100644 --- a/coordinator/src/executors_handler.rs +++ b/coordinator/src/executors_handler.rs @@ -89,7 +89,8 @@ impl ExecutorsHandlerImpl { .client .clone() .stop_executor(Request::new(request.clone())) - .await?; + .await + .context(format!("Failed to stop executor: {executor_id}"))?; tracing::debug!(executor_id, "Stop executor response: {:#?}", response); From 9e6d16f519cb6301ed99422053939b734acb4657 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 16:08:05 +1200 Subject: [PATCH 27/32] test: Fix indexer state tests --- coordinator/src/indexer_state.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 18752e99e..41e7b88cd 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -2,7 +2,6 @@ use anyhow::Context; use near_primitives::types::AccountId; -use std::cmp::Ordering; use std::str::FromStr; use crate::indexer_config::IndexerConfig; @@ -122,6 +121,7 @@ impl IndexerStateManagerImpl { state: IndexerState, ) -> anyhow::Result<()> { let raw_state = serde_json::to_string(&state)?; + eprintln!("raw_state = {:#?}", raw_state); self.redis_client .set_indexer_state(indexer_config, raw_state) @@ -282,18 +282,15 @@ mod tests { .with(predicate::eq(indexer_config.clone())) .returning(|_| { Ok(Some( - serde_json::json!({ "block_stream_synced_at": 123, "enabled": true }) + serde_json::json!({ "account_id": "morgs.near", "function_name": "test", "block_stream_synced_at": 123, "enabled": true }) .to_string(), )) }); redis_client .expect_set_indexer_state() .with( - predicate::eq(indexer_config.clone()), - predicate::eq( - serde_json::json!({ "block_stream_synced_at":123, "enabled": false }) - .to_string(), - ), + predicate::always(), + predicate::eq("{\"account_id\":\"morgs.near\",\"function_name\":\"test\",\"block_stream_synced_at\":123,\"enabled\":false}".to_string()), ) .returning(|_, _| Ok(())) .once(); From 84d1495db47133dd817a8392bbbd907a889a2558 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 16:20:53 +1200 Subject: [PATCH 28/32] chore: Remove print statement --- coordinator/src/indexer_state.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 41e7b88cd..87c1430d3 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -121,7 +121,6 @@ impl IndexerStateManagerImpl { state: IndexerState, ) -> anyhow::Result<()> { let raw_state = serde_json::to_string(&state)?; - eprintln!("raw_state = {:#?}", raw_state); self.redis_client .set_indexer_state(indexer_config, raw_state) From 66c232d418db557af30c076d386bdc88e444b550 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 16:56:53 +1200 Subject: [PATCH 29/32] chore: Add fix note --- coordinator/src/indexer_state.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/coordinator/src/indexer_state.rs b/coordinator/src/indexer_state.rs index 87c1430d3..431fb3345 100644 --- a/coordinator/src/indexer_state.rs +++ b/coordinator/src/indexer_state.rs @@ -23,6 +23,9 @@ pub struct IndexerState { } impl IndexerState { + // FIX `IndexerConfig` does not exist after an Indexer is deleted, and we need a way to + // construct the state key without it. But, this isn't ideal as we now have two places which + // define this key - we need to consolidate these somehow. pub fn get_state_key(&self) -> String { format!("{}/{}:state", self.account_id, self.function_name) } From 3eb1be79553b6530070a284da93b8befc7939491 Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Tue, 11 Jun 2024 17:00:19 +1200 Subject: [PATCH 30/32] refactor: Use re-exports --- coordinator/src/synchroniser.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 5a6b0ce23..0780b681f 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -1,12 +1,9 @@ -// TODO re-export these from handler -use block_streamer::StreamInfo; use registry_types::StartBlock; -use runner::ExecutorInfo; use tracing::instrument; use crate::{ - block_streams_handler::BlockStreamsHandler, - executors_handler::ExecutorsHandler, + block_streams_handler::{BlockStreamsHandler, StreamInfo}, + executors_handler::{ExecutorInfo, ExecutorsHandler}, indexer_config::IndexerConfig, indexer_state::{IndexerState, IndexerStateManager}, redis::RedisClient, From 91866986678f2076f0fccf918c7e318f98f8791a Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Wed, 12 Jun 2024 14:08:36 +1200 Subject: [PATCH 31/32] refactor: Move migration out of control loop --- coordinator/src/main.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 7a9730453..676d04196 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -2,7 +2,6 @@ use std::sync::Arc; use std::time::Duration; use near_primitives::types::AccountId; -use tokio::time::sleep; use tracing_subscriber::prelude::*; use crate::block_streams_handler::BlockStreamsHandler; @@ -24,6 +23,12 @@ mod utils; const CONTROL_LOOP_THROTTLE_SECONDS: Duration = Duration::from_secs(1); +async fn sleep(duration: Duration) -> anyhow::Result<()> { + tokio::time::sleep(duration).await; + + Ok(()) +} + #[tokio::main] async fn main() -> anyhow::Result<()> { tracing_subscriber::registry() @@ -70,13 +75,10 @@ async fn main() -> anyhow::Result<()> { async move { server::init(grpc_port, indexer_state_manager, registry).await } }); - loop { - let indexer_registry = registry.fetch().await?; - indexer_state_manager.migrate(&indexer_registry).await?; + let indexer_registry = registry.fetch().await?; + indexer_state_manager.migrate(&indexer_registry).await?; - tokio::try_join!(synchroniser.sync(), async { - sleep(CONTROL_LOOP_THROTTLE_SECONDS).await; - Ok(()) - })?; + loop { + tokio::try_join!(synchroniser.sync(), sleep(CONTROL_LOOP_THROTTLE_SECONDS))?; } } From 5ddd38a851f9f7689f1e452c3617a21f5df23b2f Mon Sep 17 00:00:00 2001 From: Morgan Mccauley Date: Wed, 12 Jun 2024 14:41:04 +1200 Subject: [PATCH 32/32] feat: Add deletion logs --- coordinator/src/synchroniser.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/coordinator/src/synchroniser.rs b/coordinator/src/synchroniser.rs index 0780b681f..4e47c1f04 100644 --- a/coordinator/src/synchroniser.rs +++ b/coordinator/src/synchroniser.rs @@ -256,6 +256,13 @@ impl<'a> Synchroniser<'a> { Ok(()) } + #[instrument( + skip_all, + fields( + account_id = state.account_id.to_string(), + function_name = state.function_name + ) + )] async fn sync_deleted_indexer( &self, state: &IndexerState, @@ -263,12 +270,16 @@ impl<'a> Synchroniser<'a> { block_stream: Option<&StreamInfo>, ) -> anyhow::Result<()> { if let Some(executor) = executor { + tracing::info!("Stopping executor"); + self.executors_handler .stop(executor.executor_id.clone()) .await?; } if let Some(block_stream) = block_stream { + tracing::info!("Stopping block stream"); + self.block_streams_handler .stop(block_stream.stream_id.clone()) .await?;