From 21607e7f0e20b571ca0a509f55e378b71830c79d Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 22 Mar 2024 13:34:19 +0100 Subject: [PATCH 01/25] RunningWorkerEnumerationService --- .../src/services/mod.rs | 1 + .../src/services/worker_enumeration.rs | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 golem-worker-executor-base/src/services/worker_enumeration.rs diff --git a/golem-worker-executor-base/src/services/mod.rs b/golem-worker-executor-base/src/services/mod.rs index 5b17d91ed..a62459af1 100644 --- a/golem-worker-executor-base/src/services/mod.rs +++ b/golem-worker-executor-base/src/services/mod.rs @@ -36,6 +36,7 @@ pub mod shard_manager; pub mod template; pub mod worker; pub mod worker_activator; +pub mod worker_enumeration; pub mod worker_event; // HasXXX traits for fine-grained control of which dependencies a function needs diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs new file mode 100644 index 000000000..7dd5b6b3a --- /dev/null +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -0,0 +1,79 @@ +use crate::error::GolemError; +use crate::services::active_workers::ActiveWorkers; +use crate::services::shard::ShardService; +use crate::workerctx::WorkerCtx; +use async_trait::async_trait; +use golem_common::model::{TemplateId, WorkerMetadata, WorkerStatus}; +use golem_common::redis::RedisPool; +use std::sync::Arc; + +#[async_trait] +pub trait RunningWorkerEnumerationService { + async fn get(&self, template_id: &TemplateId) -> Result, GolemError>; +} + +#[derive(Clone)] +pub struct RunningWorkerEnumerationServiceDefault { + active_workers: Arc>, +} + +#[async_trait] +impl RunningWorkerEnumerationService + for crate::services::worker_enumeration::RunningWorkerEnumerationServiceDefault +{ + async fn get(&self, template_id: &TemplateId) -> Result, GolemError> { + let active_workers = self.active_workers.enum_workers(); + + let mut template_workers: Vec = vec![]; + for worker in active_workers { + if worker.0.template_id == *template_id + && worker.1.metadata.last_known_status.status == WorkerStatus::Running + { + template_workers.push(worker.1.metadata.clone()); + } + } + + Ok(template_workers) + } +} + +impl + crate::services::worker_enumeration::RunningWorkerEnumerationServiceDefault +{ + pub fn new(active_workers: Arc>) -> Self { + Self { active_workers } + } +} + +#[async_trait] +pub trait WorkerEnumerationService { + async fn get( + &self, + template_id: &TemplateId, + precise: bool, + ) -> Result, GolemError>; +} + +#[derive(Clone)] +pub struct WorkerEnumerationServiceRedis { + redis: RedisPool, + shard_service: Arc, +} + +impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { + pub fn new(redis: RedisPool, shard_service: Arc) -> Self { + Self { + redis, + shard_service, + } + } +} + +#[async_trait] +impl WorkerEnumerationService + for crate::services::worker_enumeration::WorkerEnumerationServiceRedis +{ + async fn get(&self, template_id: &TemplateId, precise: bool) -> Result, GolemError> { + todo!() + } +} From 9b120f1aa18b2fe7bcf16f2423791a29ddf47d97 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 22 Mar 2024 16:15:05 +0100 Subject: [PATCH 02/25] WorkerEnumerationServiceInMemory --- .../src/services/mod.rs | 12 +++++ .../src/services/worker_enumeration.rs | 48 ++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/golem-worker-executor-base/src/services/mod.rs b/golem-worker-executor-base/src/services/mod.rs index a62459af1..4fb20936e 100644 --- a/golem-worker-executor-base/src/services/mod.rs +++ b/golem-worker-executor-base/src/services/mod.rs @@ -61,6 +61,18 @@ pub trait HasWorkerService { fn worker_service(&self) -> Arc; } +pub trait HasWorkerEnumerationService { + fn worker_enumeration_service( + &self, + ) -> Arc; +} + +pub trait HasRunningWorkerEnumerationService { + fn running_worker_enumeration_service( + &self, + ) -> Arc; +} + pub trait HasInvocationKeyService { fn invocation_key_service(&self) -> Arc; diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 7dd5b6b3a..2c3b3b08b 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -1,6 +1,7 @@ use crate::error::GolemError; use crate::services::active_workers::ActiveWorkers; use crate::services::shard::ShardService; +use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; use crate::workerctx::WorkerCtx; use async_trait::async_trait; use golem_common::model::{TemplateId, WorkerMetadata, WorkerStatus}; @@ -58,13 +59,19 @@ pub trait WorkerEnumerationService { pub struct WorkerEnumerationServiceRedis { redis: RedisPool, shard_service: Arc, + worker_service: Arc, } impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { - pub fn new(redis: RedisPool, shard_service: Arc) -> Self { + pub fn new( + redis: RedisPool, + shard_service: Arc, + worker_service: Arc, + ) -> Self { Self { redis, shard_service, + worker_service, } } } @@ -73,7 +80,44 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { impl WorkerEnumerationService for crate::services::worker_enumeration::WorkerEnumerationServiceRedis { - async fn get(&self, template_id: &TemplateId, precise: bool) -> Result, GolemError> { + async fn get( + &self, + template_id: &TemplateId, + precise: bool, + ) -> Result, GolemError> { todo!() } } + +#[derive(Clone)] +pub struct WorkerEnumerationServiceInMemory { + worker_service: Arc, +} + +impl crate::services::worker_enumeration::WorkerEnumerationServiceInMemory { + pub fn new(worker_service: Arc) -> Self { + Self { worker_service } + } +} + +#[async_trait] +impl WorkerEnumerationService + for crate::services::worker_enumeration::WorkerEnumerationServiceInMemory +{ + async fn get( + &self, + template_id: &TemplateId, + precise: bool, + ) -> Result, GolemError> { + let workers = self.worker_service.enumerate().await; + + let mut template_workers: Vec = vec![]; + for worker in workers { + if worker.worker_id.worker_id.template_id == *template_id { + template_workers.push(worker); + } + } + + Ok(template_workers) + } +} From 123ccb3a6e534c73b821ac6e44c1255dd1c85ea8 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 22 Mar 2024 17:11:34 +0100 Subject: [PATCH 03/25] redis scan - wip --- golem-common/src/redis.rs | 74 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index a23ee6496..3cc96f09e 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -19,7 +19,9 @@ use std::time::Instant; use bincode::{Decode, Encode}; use bytes::Bytes; +use fred::bytes_utils::Str; use fred::clients::Transaction; +use fred::cmd; use fred::prelude::{RedisPool as FredRedisPool, *}; use fred::types::{ InfoKind, Limit, MultipleKeys, MultipleOrderedPairs, MultipleValues, MultipleZaddValues, @@ -634,6 +636,78 @@ impl<'a> RedisLabelledApi<'a> { .unwrap_or(0); Ok(connected_slaves) } + + + pub async fn scan( + &self, + pattern: K, + cursor: u32, + count: u32, + ) -> RedisResult<(u32, Vec)> + where + K: AsRef, + { + self.ensure_connected().await?; + let start = Instant::now(); + + //https://redis.io/commands/scan/ + let mut args: Vec = Vec::with_capacity(5); + args.push(cursor.to_string()); + args.push("MATCH".to_string()); + args.push(self.prefixed_key(pattern)); + args.push("COUNT".to_string()); + args.push(count.to_string()); + + //https://github.com/aembke/fred.rs/blob/3a91ee9bc12faff9d32627c0db2c9b24c54efa03/examples/custom.rs#L7 + + self.record( + start, + "SCAN", + self.pool + .next() + .custom_raw(cmd!("SCAN"), args) + .await + .and_then(|f| self.parse_key_scan_frame(f)), + ) + } + + // FIXME + fn parse_key_scan_frame(frame: Resp3Frame) -> RedisResult<(u32, Vec)> { + use fred::prelude::*; + use std::convert::TryInto; + if let Resp3Frame::Array { mut data, .. } = frame { + if data.len() == 2 { + let cursor: u32 = (data[0].clone()).try_into()?; + + + if let Some(Resp3Frame::Array { data, .. }) = data.pop() { + let mut keys = Vec::with_capacity(data.len()); + + for frame in data.into_iter() { + let key: RedisKey = (frame).try_into()?; + keys.push(key); + } + + Ok((cursor, keys)) + } else { + Err(RedisError::new( + RedisErrorKind::Protocol, + "Expected second SCAN result element to be an array.", + )) + } + } else { + Err(RedisError::new( + RedisErrorKind::Protocol, + "Expected two-element bulk string array from SCAN.", + )) + } + } else { + Err(RedisError::new( + RedisErrorKind::Protocol, + "Expected bulk string array from SCAN.", + )) + } + } } pub struct RedisTransaction { From 51b5556be888610a5d47776d37ace8f8437cd3a9 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Sat, 23 Mar 2024 17:03:50 +0100 Subject: [PATCH 04/25] WorkerFilter - wip --- golem-common/src/model/mod.rs | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index d21945318..f6c369ac7 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -777,6 +777,51 @@ pub fn parse_function_name(name: &str) -> ParsedFunctionName { } } +#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +pub struct WorkerFilter { + pub filters: Vec, +} + +#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +pub enum WorkerAttributeFilter { + Name { + comparator: FilterStringComparator, + value: String, + }, + Status { + value: WorkerStatus, + }, + Version { + comparator: FilterComparator, + value: String, + }, + CreatedAt { + comparator: FilterComparator, + value: String, + }, + Env { + name: String, + comparator: FilterStringComparator, + value: String, + }, +} + +#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +pub enum FilterStringComparator { + Equal, + Like, +} + +#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +pub enum FilterComparator { + Equal, + NotEqual, + GreaterEqual, + Greater, + LessEqual, + Less, +} + #[cfg(test)] mod tests { use bincode::{Decode, Encode}; From f4ca0324b6bbcf169d39892bef6e69f1b1db02f5 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Sat, 23 Mar 2024 21:36:51 +0100 Subject: [PATCH 05/25] WorkerFilter - wip --- golem-common/src/model/mod.rs | 157 +++++++++++++++++++++++++++++++--- 1 file changed, 146 insertions(+), 11 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index f6c369ac7..cbc7dcd0e 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -778,12 +778,8 @@ pub fn parse_function_name(name: &str) -> ParsedFunctionName { } #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -pub struct WorkerFilter { - pub filters: Vec, -} - -#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -pub enum WorkerAttributeFilter { +pub enum WorkerFilter { + Empty, Name { comparator: FilterStringComparator, value: String, @@ -793,17 +789,156 @@ pub enum WorkerAttributeFilter { }, Version { comparator: FilterComparator, - value: String, - }, - CreatedAt { - comparator: FilterComparator, - value: String, + value: i32, }, + // CreatedAt { + // comparator: FilterComparator, + // value: String, + // }, Env { name: String, comparator: FilterStringComparator, value: String, }, + And(Vec), + Or(Vec), + Not(Box), +} + +impl WorkerFilter { + pub fn and(&self, filters: Vec) -> Self { + match self { + WorkerFilter::Empty => Self::new_and(filters), + _ => { + let new_filters = [vec![self.clone()], filters].concat(); + + Self::new_and(new_filters) + } + } + } + + pub fn or(&self, filters: Vec) -> Self { + match self { + WorkerFilter::Empty => Self::new_or(filters), + _ => { + let new_filters = [vec![self.clone()], filters].concat(); + Self::new_or(new_filters) + } + } + } + + pub fn not(&self) -> Self { + match self { + WorkerFilter::Empty => self.clone(), + _ => Self::new_not(self.clone()), + } + } + + pub fn eval(&self, metadata: &WorkerMetadata) -> bool { + match self.clone() { + WorkerFilter::Empty => true, + WorkerFilter::Name { comparator, value } => { + match comparator { + FilterStringComparator::Equal => { + metadata.worker_id.worker_id.worker_name == value + } + FilterStringComparator::Like => metadata + .worker_id + .worker_id + .worker_name + .contains(value.as_str()), // FIXME + } + } + WorkerFilter::Version { comparator, value } => { + let version = metadata.worker_id.template_version; + match comparator { + FilterComparator::Equal => version == value, + FilterComparator::NotEqual => version != value, + FilterComparator::Less => version < value, + FilterComparator::LessEqual => version <= value, + FilterComparator::Greater => version > value, + FilterComparator::GreaterEqual => version >= value, + } + } + WorkerFilter::Env { + name, + comparator, + value, + } => { + let mut result = false; + for env_value in metadata.env.clone() { + if env_value.0 == name { + result = match comparator { + FilterStringComparator::Equal => env_value.1 == value, + FilterStringComparator::Like => env_value.1.contains(value.as_str()), // FIXME + }; + + if result == false { + break; + } + } + } + result + } + WorkerFilter::Not(filter) => !filter.eval(metadata), + WorkerFilter::Status { value } => metadata.last_known_status.status == value, + WorkerFilter::And(filters) => { + let mut result = true; + for filter in filters { + result = filter.eval(metadata); + if result == false { + break; + } + } + result + } + WorkerFilter::Or(filters) => { + let mut result = true; + if !filters.is_empty() { + result = false; + for filter in filters { + result = filter.eval(metadata); + if result == true { + break; + } + } + } + result + } + } + } + + pub fn new_and(filters: Vec) -> Self { + WorkerFilter::And(filters) + } + + pub fn new_or(filters: Vec) -> Self { + WorkerFilter::Or(filters) + } + + pub fn new_not(filter: WorkerFilter) -> Self { + WorkerFilter::Not(Box::new(filter)) + } + + pub fn new_name(comparator: FilterStringComparator, value: String) -> Self { + WorkerFilter::Name { comparator, value } + } + + pub fn new_env(name: String, comparator: FilterStringComparator, value: String) -> Self { + WorkerFilter::Env { + name, + comparator, + value, + } + } + + pub fn new_version(comparator: FilterComparator, value: i32) -> Self { + WorkerFilter::Version { comparator, value } + } + + pub fn new_status(value: WorkerStatus) -> Self { + WorkerFilter::Status { value } + } } #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] From 08ead8c77396c2a34a94f6f12456d65d53917671 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Sun, 24 Mar 2024 17:51:34 +0100 Subject: [PATCH 06/25] WorkerFilter - wip --- golem-common/src/model/mod.rs | 107 ++++++++++++++---- .../src/services/worker_enumeration.rs | 20 +++- 2 files changed, 102 insertions(+), 25 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index cbc7dcd0e..41f42e329 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -806,24 +806,23 @@ pub enum WorkerFilter { } impl WorkerFilter { - pub fn and(&self, filters: Vec) -> Self { + pub fn and(&self, filter: WorkerFilter) -> Self { match self { - WorkerFilter::Empty => Self::new_and(filters), - _ => { - let new_filters = [vec![self.clone()], filters].concat(); - - Self::new_and(new_filters) - } + WorkerFilter::Empty => filter, + _ => match filter { + WorkerFilter::And(filters) => Self::new_and([vec![self.clone()], filters].concat()), + _ => Self::new_and(vec![self.clone(), filter]), + }, } } - pub fn or(&self, filters: Vec) -> Self { + pub fn or(&self, filter: WorkerFilter) -> Self { match self { - WorkerFilter::Empty => Self::new_or(filters), - _ => { - let new_filters = [vec![self.clone()], filters].concat(); - Self::new_or(new_filters) - } + WorkerFilter::Empty => filter, + _ => match filter { + WorkerFilter::Or(filters) => Self::new_or([vec![self.clone()], filters].concat()), + _ => Self::new_or(vec![self.clone(), filter]), + }, } } @@ -834,7 +833,7 @@ impl WorkerFilter { } } - pub fn eval(&self, metadata: &WorkerMetadata) -> bool { + pub fn matches(&self, metadata: &WorkerMetadata) -> bool { match self.clone() { WorkerFilter::Empty => true, WorkerFilter::Name { comparator, value } => { @@ -873,19 +872,17 @@ impl WorkerFilter { FilterStringComparator::Like => env_value.1.contains(value.as_str()), // FIXME }; - if result == false { - break; - } + break; } } result } - WorkerFilter::Not(filter) => !filter.eval(metadata), WorkerFilter::Status { value } => metadata.last_known_status.status == value, + WorkerFilter::Not(filter) => !filter.matches(metadata), WorkerFilter::And(filters) => { let mut result = true; for filter in filters { - result = filter.eval(metadata); + result = filter.matches(metadata); if result == false { break; } @@ -897,7 +894,7 @@ impl WorkerFilter { if !filters.is_empty() { result = false; for filter in filters { - result = filter.eval(metadata); + result = filter.matches(metadata); if result == true { break; } @@ -962,7 +959,7 @@ mod tests { use bincode::{Decode, Encode}; use serde::{Deserialize, Serialize}; - use crate::model::{parse_function_name, AccountId}; + use crate::model::{parse_function_name, AccountId, FilterComparator, FilterStringComparator, TemplateId, VersionedWorkerId, WorkerFilter, WorkerMetadata, WorkerStatus, WorkerStatusRecord, WorkerId}; #[test] fn parse_function_name_global() { @@ -1085,4 +1082,72 @@ mod tests { let json = serde_json::to_string(&example).unwrap(); assert_eq!(json, "{\"account_id\":\"account-1\"}"); } + + #[test] + fn worker_filter_matches() { + let template_id = TemplateId::new_v4(); + let worker_metadata = WorkerMetadata { + worker_id: VersionedWorkerId { + worker_id: WorkerId { + worker_name: "worker-1".to_string(), + template_id, + }, + template_version: 1, + }, + args: vec![], + env: vec![ + ("env1".to_string(), "value1".to_string()), + ("env2".to_string(), "value2".to_string()), + ], + account_id: AccountId { + value: "account-1".to_string(), + }, + last_known_status: WorkerStatusRecord::default(), + }; + + assert!(WorkerFilter::Empty.matches(&worker_metadata)); + + assert!( + WorkerFilter::new_name(FilterStringComparator::Equal, "worker-1".to_string()) + .and(WorkerFilter::new_status(WorkerStatus::Idle)) + .matches(&worker_metadata) + ); + + assert!(WorkerFilter::new_env( + "env1".to_string(), + FilterStringComparator::Equal, + "value1".to_string() + ) + .and(WorkerFilter::new_status(WorkerStatus::Idle)) + .matches(&worker_metadata)); + + assert!(WorkerFilter::new_env( + "env1".to_string(), + FilterStringComparator::Equal, + "value2".to_string() + ) + .not() + .and(WorkerFilter::new_status(WorkerStatus::Idle)) + .matches(&worker_metadata)); + + assert!( + WorkerFilter::new_name(FilterStringComparator::Equal, "worker-1".to_string()) + .and(WorkerFilter::new_version(FilterComparator::Equal, 1)) + .matches(&worker_metadata) + ); + + assert!( + WorkerFilter::new_name(FilterStringComparator::Equal, "worker-2".to_string()) + .or(WorkerFilter::new_version(FilterComparator::Equal, 1)) + .matches(&worker_metadata) + ); + + assert!(WorkerFilter::new_version(FilterComparator::GreaterEqual, 1) + .and(WorkerFilter::new_version(FilterComparator::Less, 2)) + .or(WorkerFilter::new_name( + FilterStringComparator::Equal, + "worker-2".to_string() + )) + .matches(&worker_metadata)); + } } diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 2c3b3b08b..24e987026 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -4,13 +4,17 @@ use crate::services::shard::ShardService; use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; use crate::workerctx::WorkerCtx; use async_trait::async_trait; -use golem_common::model::{TemplateId, WorkerMetadata, WorkerStatus}; +use golem_common::model::{TemplateId, WorkerFilter, WorkerMetadata, WorkerStatus}; use golem_common::redis::RedisPool; use std::sync::Arc; #[async_trait] pub trait RunningWorkerEnumerationService { - async fn get(&self, template_id: &TemplateId) -> Result, GolemError>; + async fn get( + &self, + template_id: &TemplateId, + filter: &WorkerFilter, + ) -> Result, GolemError>; } #[derive(Clone)] @@ -22,13 +26,18 @@ pub struct RunningWorkerEnumerationServiceDefault { impl RunningWorkerEnumerationService for crate::services::worker_enumeration::RunningWorkerEnumerationServiceDefault { - async fn get(&self, template_id: &TemplateId) -> Result, GolemError> { + async fn get( + &self, + template_id: &TemplateId, + filter: &WorkerFilter, + ) -> Result, GolemError> { let active_workers = self.active_workers.enum_workers(); let mut template_workers: Vec = vec![]; for worker in active_workers { if worker.0.template_id == *template_id && worker.1.metadata.last_known_status.status == WorkerStatus::Running + && filter.matches(&worker.1.metadata) { template_workers.push(worker.1.metadata.clone()); } @@ -51,6 +60,7 @@ pub trait WorkerEnumerationService { async fn get( &self, template_id: &TemplateId, + filter: &WorkerFilter, precise: bool, ) -> Result, GolemError>; } @@ -83,6 +93,7 @@ impl WorkerEnumerationService async fn get( &self, template_id: &TemplateId, + filter: &WorkerFilter, precise: bool, ) -> Result, GolemError> { todo!() @@ -107,13 +118,14 @@ impl WorkerEnumerationService async fn get( &self, template_id: &TemplateId, + filter: &WorkerFilter, precise: bool, ) -> Result, GolemError> { let workers = self.worker_service.enumerate().await; let mut template_workers: Vec = vec![]; for worker in workers { - if worker.worker_id.worker_id.template_id == *template_id { + if worker.worker_id.worker_id.template_id == *template_id && filter.matches(&worker) { template_workers.push(worker); } } From d53fcc8f489d1117e16fb2b88bb318015cc99354 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Sun, 24 Mar 2024 19:48:56 +0100 Subject: [PATCH 07/25] redis scan --- golem-common/src/model/mod.rs | 17 ++++++++++++----- golem-common/src/redis.rs | 1 - 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index 41f42e329..c2c3125f0 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -959,7 +959,11 @@ mod tests { use bincode::{Decode, Encode}; use serde::{Deserialize, Serialize}; - use crate::model::{parse_function_name, AccountId, FilterComparator, FilterStringComparator, TemplateId, VersionedWorkerId, WorkerFilter, WorkerMetadata, WorkerStatus, WorkerStatusRecord, WorkerId}; + use crate::model::{ + parse_function_name, AccountId, FilterComparator, FilterStringComparator, TemplateId, + VersionedWorkerId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus, + WorkerStatusRecord, + }; #[test] fn parse_function_name_global() { @@ -1116,7 +1120,7 @@ mod tests { assert!(WorkerFilter::new_env( "env1".to_string(), FilterStringComparator::Equal, - "value1".to_string() + "value1".to_string(), ) .and(WorkerFilter::new_status(WorkerStatus::Idle)) .matches(&worker_metadata)); @@ -1124,10 +1128,13 @@ mod tests { assert!(WorkerFilter::new_env( "env1".to_string(), FilterStringComparator::Equal, - "value2".to_string() + "value2".to_string(), ) .not() - .and(WorkerFilter::new_status(WorkerStatus::Idle)) + .and( + WorkerFilter::new_status(WorkerStatus::Running) + .or(WorkerFilter::new_status(WorkerStatus::Idle)) + ) .matches(&worker_metadata)); assert!( @@ -1146,7 +1153,7 @@ mod tests { .and(WorkerFilter::new_version(FilterComparator::Less, 2)) .or(WorkerFilter::new_name( FilterStringComparator::Equal, - "worker-2".to_string() + "worker-2".to_string(), )) .matches(&worker_metadata)); } diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index 3cc96f09e..1194df7a2 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -19,7 +19,6 @@ use std::time::Instant; use bincode::{Decode, Encode}; use bytes::Bytes; -use fred::bytes_utils::Str; use fred::clients::Transaction; use fred::cmd; use fred::prelude::{RedisPool as FredRedisPool, *}; From 2487b4680d4ca06a6542d387fa99c387aad10c5f Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Sun, 24 Mar 2024 21:15:35 +0100 Subject: [PATCH 08/25] redis scan --- golem-common/src/redis.rs | 20 ++++++++------- .../src/services/worker_enumeration.rs | 25 +++++++++++++++---- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index 1194df7a2..2f4a27c05 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -642,9 +642,9 @@ impl<'a> RedisLabelledApi<'a> { pattern: K, cursor: u32, count: u32, - ) -> RedisResult<(u32, Vec)> - where - K: AsRef, + ) -> RedisResult<(u32, Vec)> + where + K: AsRef, { self.ensure_connected().await?; let start = Instant::now(); @@ -670,20 +670,22 @@ impl<'a> RedisLabelledApi<'a> { ) } - // FIXME - fn parse_key_scan_frame(frame: Resp3Frame) -> RedisResult<(u32, Vec)> { + fn parse_key_scan_frame(&self, frame: Resp3Frame) -> RedisResult<(u32, Vec)> { use fred::prelude::*; - use std::convert::TryInto; if let Resp3Frame::Array { mut data, .. } = frame { if data.len() == 2 { - let cursor: u32 = (data[0].clone()).try_into()?; - + let cursor: u32 = data[0] + .clone() + .try_into() + .and_then(|value: RedisValue| value.convert())?; if let Some(Resp3Frame::Array { data, .. }) = data.pop() { let mut keys = Vec::with_capacity(data.len()); for frame in data.into_iter() { - let key: RedisKey = (frame).try_into()?; + let key: String = frame + .try_into() + .and_then(|value: RedisValue| value.convert())?; keys.push(key); } diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 24e987026..7176c3eb0 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -4,7 +4,7 @@ use crate::services::shard::ShardService; use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; use crate::workerctx::WorkerCtx; use async_trait::async_trait; -use golem_common::model::{TemplateId, WorkerFilter, WorkerMetadata, WorkerStatus}; +use golem_common::model::{TemplateId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus}; use golem_common::redis::RedisPool; use std::sync::Arc; @@ -61,8 +61,9 @@ pub trait WorkerEnumerationService { &self, template_id: &TemplateId, filter: &WorkerFilter, + cursor: u32, precise: bool, - ) -> Result, GolemError>; + ) -> Result<(u32, Vec), GolemError>; } #[derive(Clone)] @@ -94,12 +95,24 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: &WorkerFilter, + cursor: u32, precise: bool, - ) -> Result, GolemError> { + ) -> Result<(u32, Vec), GolemError> { + let worker_templates_redis_key = get_worker_details_redis_key(&template_id); + let (new_cursor, worker_redis_keys) = self + .redis + .with("instance", "scan") + .scan(worker_templates_redis_key, cursor, 20) + .await + .map_err(|e| GolemError::unknown(e.details()))?; todo!() } } +fn get_worker_details_redis_key(template_id: &TemplateId) -> String { + format!("instance:instance:{}*", template_id.0) +} + #[derive(Clone)] pub struct WorkerEnumerationServiceInMemory { worker_service: Arc, @@ -119,8 +132,10 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: &WorkerFilter, + cursor: u32, precise: bool, - ) -> Result, GolemError> { + ) -> Result<(u32, Vec), GolemError> { + // TODO implement precise and cursor let workers = self.worker_service.enumerate().await; let mut template_workers: Vec = vec![]; @@ -130,6 +145,6 @@ impl WorkerEnumerationService } } - Ok(template_workers) + Ok((template_workers.len() as u32, template_workers)) } } From c034988967a0743ea872ec5a63b7443ed4fb705e Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Mon, 25 Mar 2024 10:21:17 +0100 Subject: [PATCH 09/25] WorkerEnumeration - wip --- golem-common/src/redis.rs | 23 +++--- .../src/services/worker_enumeration.rs | 76 +++++++++++++++---- 2 files changed, 72 insertions(+), 27 deletions(-) diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index 2f4a27c05..f3ffc97e2 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -640,9 +640,9 @@ impl<'a> RedisLabelledApi<'a> { pub async fn scan( &self, pattern: K, - cursor: u32, - count: u32, - ) -> RedisResult<(u32, Vec)> + cursor: usize, + count: usize, + ) -> RedisResult<(usize, Vec)> where K: AsRef, { @@ -650,12 +650,13 @@ impl<'a> RedisLabelledApi<'a> { let start = Instant::now(); //https://redis.io/commands/scan/ - let mut args: Vec = Vec::with_capacity(5); - args.push(cursor.to_string()); - args.push("MATCH".to_string()); - args.push(self.prefixed_key(pattern)); - args.push("COUNT".to_string()); - args.push(count.to_string()); + let args: Vec = vec![ + cursor.to_string(), + "MATCH".to_string(), + self.prefixed_key(pattern), + "COUNT".to_string(), + count.to_string(), + ]; //https://github.com/aembke/fred.rs/blob/3a91ee9bc12faff9d32627c0db2c9b24c54efa03/examples/custom.rs#L7 @@ -670,11 +671,11 @@ impl<'a> RedisLabelledApi<'a> { ) } - fn parse_key_scan_frame(&self, frame: Resp3Frame) -> RedisResult<(u32, Vec)> { + fn parse_key_scan_frame(&self, frame: Resp3Frame) -> RedisResult<(usize, Vec)> { use fred::prelude::*; if let Resp3Frame::Array { mut data, .. } = frame { if data.len() == 2 { - let cursor: u32 = data[0] + let cursor: usize = data[0] .clone() .try_into() .and_then(|value: RedisValue| value.convert())?; diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 7176c3eb0..559f22283 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -61,9 +61,10 @@ pub trait WorkerEnumerationService { &self, template_id: &TemplateId, filter: &WorkerFilter, - cursor: u32, + cursor: usize, + count: usize, precise: bool, - ) -> Result<(u32, Vec), GolemError>; + ) -> Result<(Option, Vec), GolemError>; } #[derive(Clone)] @@ -95,24 +96,45 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: &WorkerFilter, - cursor: u32, + cursor: usize, + count: usize, precise: bool, - ) -> Result<(u32, Vec), GolemError> { - let worker_templates_redis_key = get_worker_details_redis_key(&template_id); + ) -> Result<(Option, Vec), GolemError> { + let template_worker_redis_key = get_template_worker_redis_key(&template_id); let (new_cursor, worker_redis_keys) = self .redis .with("instance", "scan") - .scan(worker_templates_redis_key, cursor, 20) + .scan(template_worker_redis_key, cursor, count) .await .map_err(|e| GolemError::unknown(e.details()))?; todo!() } } -fn get_worker_details_redis_key(template_id: &TemplateId) -> String { +fn get_template_worker_redis_key(template_id: &TemplateId) -> String { format!("instance:instance:{}*", template_id.0) } +fn get_worker_id_from_redis_key( + worker_redis_key: &str, + template_id: &TemplateId, +) -> Result { + let template_prefix = format!("instance:instance:{}:", template_id.0); + + if worker_redis_key.starts_with(&template_prefix) { + let worker_name = &worker_redis_key[template_prefix.len()..]; + + Ok(WorkerId { + worker_name: worker_name.to_string(), + template_id: template_id.clone(), + }) + } else { + Err(GolemError::unknown( + "Failed to get worker id from redis key", + )) + } +} + #[derive(Clone)] pub struct WorkerEnumerationServiceInMemory { worker_service: Arc, @@ -132,19 +154,41 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: &WorkerFilter, - cursor: u32, + cursor: usize, + count: usize, precise: bool, - ) -> Result<(u32, Vec), GolemError> { - // TODO implement precise and cursor + ) -> Result<(Option, Vec), GolemError> { + // TODO implement precise let workers = self.worker_service.enumerate().await; + let all_workers_count = workers.len(); + let mut template_workers: Vec = vec![]; - for worker in workers { - if worker.worker_id.worker_id.template_id == *template_id && filter.matches(&worker) { - template_workers.push(worker); - } - } - Ok((template_workers.len() as u32, template_workers)) + let new_cursor = if all_workers_count > cursor { + let mut index = 0; + for worker in workers { + if index >= cursor + && worker.worker_id.worker_id.template_id == *template_id + && filter.matches(&worker) + { + template_workers.push(worker); + } + + index = index + 1; + + if template_workers.len() == count { + break; + } + } + if index >= all_workers_count { + None + } else { + Some(index) + } + } else { + None + }; + Ok((new_cursor, template_workers)) } } From 3ee2f8b3e2cda004fba10547dd04a81f8e264a3f Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Mon, 25 Mar 2024 20:18:01 +0100 Subject: [PATCH 10/25] WorkerEnumeration - wip --- .../src/services/worker_enumeration.rs | 78 +++++++++++++++---- 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 559f22283..a717564e5 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -70,29 +70,18 @@ pub trait WorkerEnumerationService { #[derive(Clone)] pub struct WorkerEnumerationServiceRedis { redis: RedisPool, - shard_service: Arc, worker_service: Arc, } impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { - pub fn new( - redis: RedisPool, - shard_service: Arc, - worker_service: Arc, - ) -> Self { + pub fn new(redis: RedisPool, worker_service: Arc) -> Self { Self { redis, - shard_service, worker_service, } } -} -#[async_trait] -impl WorkerEnumerationService - for crate::services::worker_enumeration::WorkerEnumerationServiceRedis -{ - async fn get( + async fn get_internal( &self, template_id: &TemplateId, filter: &WorkerFilter, @@ -100,14 +89,72 @@ impl WorkerEnumerationService count: usize, precise: bool, ) -> Result<(Option, Vec), GolemError> { + let mut new_cursor: Option = None; + let mut template_workers: Vec = vec![]; + let template_worker_redis_key = get_template_worker_redis_key(&template_id); - let (new_cursor, worker_redis_keys) = self + + let (new_redis_cursor, worker_redis_keys) = self .redis .with("instance", "scan") .scan(template_worker_redis_key, cursor, count) .await .map_err(|e| GolemError::unknown(e.details()))?; - todo!() + + for worker_redis_key in worker_redis_keys { + let worker_id = get_worker_id_from_redis_key(&worker_redis_key, &template_id)?; + + let worker_metadata = self.worker_service.get(&worker_id).await; + + if let Some(worker_metadata) = worker_metadata { + if filter.matches(&worker_metadata) { + template_workers.push(worker_metadata); + } + } + } + + if new_redis_cursor > 0 { + new_cursor = Some(new_redis_cursor); + } + + Ok((new_cursor, template_workers)) + } +} + +#[async_trait] +impl WorkerEnumerationService + for crate::services::worker_enumeration::WorkerEnumerationServiceRedis +{ + async fn get( + &self, + template_id: &TemplateId, + filter: &WorkerFilter, + cursor: usize, + count: usize, + precise: bool, + ) -> Result<(Option, Vec), GolemError> { + let mut new_cursor: Option = Some(cursor); + let mut template_workers: Vec = vec![]; + + while new_cursor.is_some() && template_workers.len() < count { + let next_count = template_workers.len() - count; + + let (next_cursor, workers) = self + .get_internal( + &template_id, + &filter, + new_cursor.unwrap_or(0), + next_count, + precise, + ) + .await?; + + template_workers.extend(workers); + + new_cursor = next_cursor; + } + + Ok((new_cursor, template_workers)) } } @@ -123,7 +170,6 @@ fn get_worker_id_from_redis_key( if worker_redis_key.starts_with(&template_prefix) { let worker_name = &worker_redis_key[template_prefix.len()..]; - Ok(WorkerId { worker_name: worker_name.to_string(), template_id: template_id.clone(), From 5145dbb456ccb821e58ad9d9943b10bfbd3c3bef Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Mon, 25 Mar 2024 22:02:50 +0100 Subject: [PATCH 11/25] WorkerEnumeration Mocks, add to services --- golem-worker-executor-base/src/lib.rs | 55 +++++++++++-- .../src/services/mod.rs | 40 +++++++++ .../src/services/recovery.rs | 45 +++++++++- .../src/services/rpc.rs | 38 ++++++++- .../src/services/worker_enumeration.rs | 82 ++++++++++++++++++- .../tests/common/mod.rs | 11 +++ golem-worker-executor/src/lib.rs | 11 +++ 7 files changed, 261 insertions(+), 21 deletions(-) diff --git a/golem-worker-executor-base/src/lib.rs b/golem-worker-executor-base/src/lib.rs index 2626d6c88..8ad2dd027 100644 --- a/golem-worker-executor-base/src/lib.rs +++ b/golem-worker-executor-base/src/lib.rs @@ -40,7 +40,7 @@ use crate::grpc::WorkerExecutorImpl; use crate::http_server::HttpServerImpl; use crate::services::active_workers::ActiveWorkers; use crate::services::blob_store::BlobStoreService; -use crate::services::golem_config::GolemConfig; +use crate::services::golem_config::{GolemConfig, WorkersServiceConfig}; use crate::services::invocation_key::{InvocationKeyService, InvocationKeyServiceDefault}; use crate::services::key_value::KeyValueService; use crate::services::oplog::{OplogService, OplogServiceDefault}; @@ -49,8 +49,12 @@ use crate::services::scheduler::{SchedulerService, SchedulerServiceDefault}; use crate::services::shard::{ShardService, ShardServiceDefault}; use crate::services::shard_manager::ShardManagerService; use crate::services::template::TemplateService; -use crate::services::worker::WorkerService; +use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; use crate::services::worker_activator::{LazyWorkerActivator, WorkerActivator}; +use crate::services::worker_enumeration::{ + RunningWorkerEnumerationService, RunningWorkerEnumerationServiceDefault, + WorkerEnumerationService, WorkerEnumerationServiceInMemory, WorkerEnumerationServiceRedis, +}; use crate::services::{blob_store, key_value, promise, shard_manager, template, All}; use crate::workerctx::WorkerCtx; @@ -74,6 +78,8 @@ pub trait Bootstrap { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: Arc, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -141,13 +147,44 @@ pub trait Bootstrap { let promise_service = promise::configured(&golem_config.promises, pool.clone()); let shard_service = Arc::new(ShardServiceDefault::new()); let lazy_worker_activator = Arc::new(LazyWorkerActivator::new()); - let worker_service = services::worker::configured( - &golem_config.workers, - pool.clone(), - shard_service.clone(), - ); + + let oplog_service = Arc::new(OplogServiceDefault::new(pool.clone()).await); + + let (worker_service, worker_enumeration_service) = match &golem_config.workers { + WorkersServiceConfig::InMemory => { + let worker_service = Arc::new(WorkerServiceInMemory::new()); + let enumeration_service: Arc = Arc::new( + WorkerEnumerationServiceInMemory::new(worker_service.clone()), + ); + + ( + worker_service as Arc, + enumeration_service, + ) + } + WorkersServiceConfig::Redis => { + let worker_service = + Arc::new(WorkerServiceRedis::new(pool.clone(), shard_service.clone())); + let enumeration_service: Arc = + Arc::new(WorkerEnumerationServiceRedis::new( + pool.clone(), + worker_service.clone(), + oplog_service.clone(), + )); + + ( + worker_service as Arc, + enumeration_service, + ) + } + }; + let active_workers = self.create_active_workers(&golem_config); + let running_worker_enumeration_service = Arc::new( + RunningWorkerEnumerationServiceDefault::new(active_workers.clone()), + ); + let shard_manager_service = shard_manager::configured(&golem_config.shard_manager_service); let config = self.create_wasmtime_config(); @@ -175,8 +212,6 @@ pub trait Bootstrap { let blob_store_service = blob_store::configured(&golem_config.blob_store_service).await; - let oplog_service = Arc::new(OplogServiceDefault::new(pool.clone()).await); - let scheduler_service = SchedulerServiceDefault::new( pool.clone(), shard_service.clone(), @@ -194,6 +229,8 @@ pub trait Bootstrap { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config.clone(), invocation_key_service, diff --git a/golem-worker-executor-base/src/services/mod.rs b/golem-worker-executor-base/src/services/mod.rs index 4fb20936e..6b1fbd57d 100644 --- a/golem-worker-executor-base/src/services/mod.rs +++ b/golem-worker-executor-base/src/services/mod.rs @@ -16,6 +16,7 @@ use std::sync::Arc; #[cfg(any(feature = "mocks", test))] use std::time::Duration; +use crate::services::worker_enumeration::RunningWorkerEnumerationServiceDefault; use tokio::runtime::Handle; use crate::workerctx::WorkerCtx; @@ -126,6 +127,8 @@ pub trait HasAll: + HasTemplateService + HasConfig + HasWorkerService + + HasWorkerEnumerationService + + HasRunningWorkerEnumerationService + HasInvocationKeyService + HasPromiseService + HasWasmtimeEngine @@ -146,6 +149,8 @@ impl< + HasTemplateService + HasConfig + HasWorkerService + + HasWorkerEnumerationService + + HasRunningWorkerEnumerationService + HasInvocationKeyService + HasPromiseService + HasWasmtimeEngine @@ -171,6 +176,9 @@ pub struct All { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: + Arc, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -194,6 +202,8 @@ impl Clone for All { template_service: self.template_service.clone(), shard_manager_service: self.shard_manager_service.clone(), worker_service: self.worker_service.clone(), + worker_enumeration_service: self.worker_enumeration_service.clone(), + running_worker_enumeration_service: self.running_worker_enumeration_service.clone(), promise_service: self.promise_service.clone(), golem_config: self.golem_config.clone(), invocation_key_service: self.invocation_key_service.clone(), @@ -218,6 +228,12 @@ impl All { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc< + dyn worker_enumeration::WorkerEnumerationService + Send + Sync, + >, + running_worker_enumeration_service: Arc< + dyn worker_enumeration::RunningWorkerEnumerationService + Send + Sync, + >, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -238,6 +254,8 @@ impl All { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config, invocation_key_service, @@ -264,6 +282,10 @@ impl All { let runtime = Handle::current(); let template_service = Arc::new(template::TemplateServiceMock::new()); let worker_service = Arc::new(worker::WorkerServiceMock::new()); + let worker_enumeration_service = + Arc::new(worker_enumeration::WorkerEnumerationServiceMock::new()); + let running_worker_enumeration_service = + Arc::new(worker_enumeration::RunningWorkerEnumerationServiceMock::new()); let promise_service = Arc::new(promise::PromiseServiceMock::new()); let golem_config = Arc::new(golem_config::GolemConfig::default()); let invocation_key_service = @@ -284,6 +306,8 @@ impl All { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config, invocation_key_service, @@ -343,6 +367,22 @@ impl> HasWorkerService for T { } } +impl> HasWorkerEnumerationService for T { + fn worker_enumeration_service( + &self, + ) -> Arc { + self.all().worker_enumeration_service.clone() + } +} + +impl> HasRunningWorkerEnumerationService for T { + fn running_worker_enumeration_service( + &self, + ) -> Arc { + self.all().running_worker_enumeration_service.clone() + } +} + impl> HasInvocationKeyService for T { fn invocation_key_service( &self, diff --git a/golem-worker-executor-base/src/services/recovery.rs b/golem-worker-executor-base/src/services/recovery.rs index 818c2d89d..3bc3d8147 100644 --- a/golem-worker-executor-base/src/services/recovery.rs +++ b/golem-worker-executor-base/src/services/recovery.rs @@ -20,10 +20,10 @@ use crate::model::{InterruptKind, LastError, TrapType}; use crate::services::rpc::Rpc; use crate::services::{ active_workers, blob_store, golem_config, invocation_key, key_value, oplog, promise, scheduler, - template, worker, HasActiveWorkers, HasAll, HasBlobStoreService, HasConfig, HasExtraDeps, - HasInvocationKeyService, HasKeyValueService, HasOplogService, HasPromiseService, - HasRecoveryManagement, HasRpc, HasSchedulerService, HasTemplateService, HasWasmtimeEngine, - HasWorkerService, + template, worker, worker_enumeration, HasActiveWorkers, HasAll, HasBlobStoreService, HasConfig, + HasExtraDeps, HasInvocationKeyService, HasKeyValueService, HasOplogService, HasPromiseService, + HasRecoveryManagement, HasRpc, HasRunningWorkerEnumerationService, HasSchedulerService, + HasTemplateService, HasWasmtimeEngine, HasWorkerEnumerationService, HasWorkerService, }; use crate::worker::Worker; use crate::workerctx::WorkerCtx; @@ -83,6 +83,9 @@ pub struct RecoveryManagementDefault { runtime: Handle, template_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: + Arc, oplog_service: Arc, promise_service: Arc, scheduler_service: Arc, @@ -105,6 +108,8 @@ impl Clone for RecoveryManagementDefault { runtime: self.runtime.clone(), template_service: self.template_service.clone(), worker_service: self.worker_service.clone(), + worker_enumeration_service: self.worker_enumeration_service.clone(), + running_worker_enumeration_service: self.running_worker_enumeration_service.clone(), oplog_service: self.oplog_service.clone(), promise_service: self.promise_service.clone(), scheduler_service: self.scheduler_service.clone(), @@ -143,6 +148,22 @@ impl HasWorkerService for RecoveryManagementDefault { } } +impl HasWorkerEnumerationService for RecoveryManagementDefault { + fn worker_enumeration_service( + &self, + ) -> Arc { + self.worker_enumeration_service.clone() + } +} + +impl HasRunningWorkerEnumerationService for RecoveryManagementDefault { + fn running_worker_enumeration_service( + &self, + ) -> Arc { + self.running_worker_enumeration_service.clone() + } +} + impl HasInvocationKeyService for RecoveryManagementDefault { fn invocation_key_service( &self, @@ -221,6 +242,12 @@ impl RecoveryManagementDefault { runtime: Handle, template_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc< + dyn worker_enumeration::WorkerEnumerationService + Send + Sync, + >, + running_worker_enumeration_service: Arc< + dyn worker_enumeration::RunningWorkerEnumerationService + Send + Sync, + >, oplog_service: Arc, promise_service: Arc, scheduler_service: Arc, @@ -239,6 +266,8 @@ impl RecoveryManagementDefault { runtime, template_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, oplog_service, promise_service, scheduler_service, @@ -260,6 +289,12 @@ impl RecoveryManagementDefault { runtime: Handle, template_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc< + dyn worker_enumeration::WorkerEnumerationService + Send + Sync, + >, + running_worker_enumeration_service: Arc< + dyn worker_enumeration::RunningWorkerEnumerationService + Send + Sync, + >, oplog_service: Arc, promise_service: Arc, scheduler_service: Arc, @@ -282,6 +317,8 @@ impl RecoveryManagementDefault { runtime, template_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, oplog_service, promise_service, scheduler_service, diff --git a/golem-worker-executor-base/src/services/rpc.rs b/golem-worker-executor-base/src/services/rpc.rs index d68652590..1ab6baadc 100644 --- a/golem-worker-executor-base/src/services/rpc.rs +++ b/golem-worker-executor-base/src/services/rpc.rs @@ -16,10 +16,11 @@ use crate::error::GolemError; use crate::grpc::{authorised_grpc_request, UriBackConversion}; use crate::services::{ active_workers, blob_store, golem_config, invocation_key, key_value, oplog, promise, recovery, - scheduler, shard, shard_manager, template, worker, HasActiveWorkers, HasBlobStoreService, - HasConfig, HasExtraDeps, HasInvocationKeyService, HasKeyValueService, HasOplogService, - HasPromiseService, HasRecoveryManagement, HasRpc, HasSchedulerService, HasShardService, - HasTemplateService, HasWasmtimeEngine, HasWorkerService, + scheduler, shard, shard_manager, template, worker, worker_enumeration, HasActiveWorkers, + HasBlobStoreService, HasConfig, HasExtraDeps, HasInvocationKeyService, HasKeyValueService, + HasOplogService, HasPromiseService, HasRecoveryManagement, HasRpc, + HasRunningWorkerEnumerationService, HasSchedulerService, HasShardService, HasTemplateService, + HasWasmtimeEngine, HasWorkerEnumerationService, HasWorkerService, }; use crate::worker::{invoke_and_await, Worker}; use crate::workerctx::WorkerCtx; @@ -280,6 +281,9 @@ pub struct DirectWorkerInvocationRpc { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: + Arc, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -303,6 +307,8 @@ impl Clone for DirectWorkerInvocationRpc { template_service: self.template_service.clone(), shard_manager_service: self.shard_manager_service.clone(), worker_service: self.worker_service.clone(), + worker_enumeration_service: self.worker_enumeration_service.clone(), + running_worker_enumeration_service: self.running_worker_enumeration_service.clone(), promise_service: self.promise_service.clone(), golem_config: self.golem_config.clone(), invocation_key_service: self.invocation_key_service.clone(), @@ -341,6 +347,22 @@ impl HasWorkerService for DirectWorkerInvocationRpc { } } +impl HasWorkerEnumerationService for DirectWorkerInvocationRpc { + fn worker_enumeration_service( + &self, + ) -> Arc { + self.worker_enumeration_service.clone() + } +} + +impl HasRunningWorkerEnumerationService for DirectWorkerInvocationRpc { + fn running_worker_enumeration_service( + &self, + ) -> Arc { + self.running_worker_enumeration_service.clone() + } +} + impl HasInvocationKeyService for DirectWorkerInvocationRpc { fn invocation_key_service( &self, @@ -431,6 +453,12 @@ impl DirectWorkerInvocationRpc { runtime: Handle, template_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc< + dyn worker_enumeration::WorkerEnumerationService + Send + Sync, + >, + running_worker_enumeration_service: Arc< + dyn worker_enumeration::RunningWorkerEnumerationService + Send + Sync, + >, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -451,6 +479,8 @@ impl DirectWorkerInvocationRpc { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config, invocation_key_service, diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index a717564e5..4994b9860 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -1,6 +1,6 @@ use crate::error::GolemError; use crate::services::active_workers::ActiveWorkers; -use crate::services::shard::ShardService; +use crate::services::oplog::OplogServiceDefault; use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; use crate::workerctx::WorkerCtx; use async_trait::async_trait; @@ -55,6 +55,39 @@ impl } } +#[cfg(any(feature = "mocks", test))] +pub struct RunningWorkerEnumerationServiceMock {} + +#[cfg(any(feature = "mocks", test))] +impl Default + for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock +{ + fn default() -> Self { + Self::new() + } +} + +#[cfg(any(feature = "mocks", test))] +impl crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock { + pub fn new() -> Self { + Self {} + } +} + +#[cfg(any(feature = "mocks", test))] +#[async_trait] +impl RunningWorkerEnumerationService + for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock +{ + async fn get( + &self, + _template_id: &TemplateId, + _filter: &WorkerFilter, + ) -> Result, GolemError> { + unimplemented!() + } +} + #[async_trait] pub trait WorkerEnumerationService { async fn get( @@ -71,13 +104,19 @@ pub trait WorkerEnumerationService { pub struct WorkerEnumerationServiceRedis { redis: RedisPool, worker_service: Arc, + oplog_service: Arc, } impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { - pub fn new(redis: RedisPool, worker_service: Arc) -> Self { + pub fn new( + redis: RedisPool, + worker_service: Arc, + oplog_service: Arc, + ) -> Self { Self { redis, worker_service, + oplog_service, } } @@ -89,6 +128,7 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { count: usize, precise: bool, ) -> Result<(Option, Vec), GolemError> { + // TODO implement precise let mut new_cursor: Option = None; let mut template_workers: Vec = vec![]; @@ -137,14 +177,14 @@ impl WorkerEnumerationService let mut template_workers: Vec = vec![]; while new_cursor.is_some() && template_workers.len() < count { - let next_count = template_workers.len() - count; + let new_count = template_workers.len() - count; let (next_cursor, workers) = self .get_internal( &template_id, &filter, new_cursor.unwrap_or(0), - next_count, + new_count, precise, ) .await?; @@ -238,3 +278,37 @@ impl WorkerEnumerationService Ok((new_cursor, template_workers)) } } + +#[cfg(any(feature = "mocks", test))] +pub struct WorkerEnumerationServiceMock {} + +#[cfg(any(feature = "mocks", test))] +impl Default for crate::services::worker_enumeration::WorkerEnumerationServiceMock { + fn default() -> Self { + Self::new() + } +} + +#[cfg(any(feature = "mocks", test))] +impl crate::services::worker_enumeration::WorkerEnumerationServiceMock { + pub fn new() -> Self { + Self {} + } +} + +#[cfg(any(feature = "mocks", test))] +#[async_trait] +impl WorkerEnumerationService + for crate::services::worker_enumeration::WorkerEnumerationServiceMock +{ + async fn get( + &self, + _template_id: &TemplateId, + _filter: &WorkerFilter, + _cursor: usize, + _count: usize, + _precise: bool, + ) -> Result<(Option, Vec), GolemError> { + unimplemented!() + } +} diff --git a/golem-worker-executor-base/tests/common/mod.rs b/golem-worker-executor-base/tests/common/mod.rs index 5670f6d75..0dcf05bad 100644 --- a/golem-worker-executor-base/tests/common/mod.rs +++ b/golem-worker-executor-base/tests/common/mod.rs @@ -84,6 +84,9 @@ use golem_worker_executor_base::preview2::golem; use golem_worker_executor_base::services::rpc::{ DirectWorkerInvocationRpc, RemoteInvocationRpc, Rpc, }; +use golem_worker_executor_base::services::worker_enumeration::{ + RunningWorkerEnumerationService, WorkerEnumerationService, +}; use tonic::transport::Channel; use tracing::{debug, error, info}; use uuid::Uuid; @@ -1367,6 +1370,8 @@ impl Bootstrap for ServerBootstrap { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: Arc, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -1392,6 +1397,8 @@ impl Bootstrap for ServerBootstrap { runtime.clone(), template_service.clone(), worker_service.clone(), + worker_enumeration_service.clone(), + running_worker_enumeration_service.clone(), promise_service.clone(), golem_config.clone(), invocation_key_service.clone(), @@ -1410,6 +1417,8 @@ impl Bootstrap for ServerBootstrap { runtime.clone(), template_service.clone(), worker_service.clone(), + worker_enumeration_service.clone(), + running_worker_enumeration_service.clone(), oplog_service.clone(), promise_service.clone(), scheduler_service.clone(), @@ -1430,6 +1439,8 @@ impl Bootstrap for ServerBootstrap { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config, invocation_key_service, diff --git a/golem-worker-executor/src/lib.rs b/golem-worker-executor/src/lib.rs index d432bac70..e0705945b 100644 --- a/golem-worker-executor/src/lib.rs +++ b/golem-worker-executor/src/lib.rs @@ -35,6 +35,9 @@ use golem_worker_executor_base::services::shard_manager::ShardManagerService; use golem_worker_executor_base::services::template::TemplateService; use golem_worker_executor_base::services::worker::WorkerService; use golem_worker_executor_base::services::worker_activator::WorkerActivator; +use golem_worker_executor_base::services::worker_enumeration::{ + RunningWorkerEnumerationService, WorkerEnumerationService, +}; use golem_worker_executor_base::services::All; use golem_worker_executor_base::wasi_host::create_linker; use golem_worker_executor_base::Bootstrap; @@ -65,6 +68,8 @@ impl Bootstrap for ServerBootstrap { template_service: Arc, shard_manager_service: Arc, worker_service: Arc, + worker_enumeration_service: Arc, + running_worker_enumeration_service: Arc, promise_service: Arc, golem_config: Arc, invocation_key_service: Arc, @@ -92,6 +97,8 @@ impl Bootstrap for ServerBootstrap { runtime.clone(), template_service.clone(), worker_service.clone(), + worker_enumeration_service.clone(), + running_worker_enumeration_service.clone(), promise_service.clone(), golem_config.clone(), invocation_key_service.clone(), @@ -110,6 +117,8 @@ impl Bootstrap for ServerBootstrap { runtime.clone(), template_service.clone(), worker_service.clone(), + worker_enumeration_service.clone(), + running_worker_enumeration_service.clone(), oplog_service.clone(), promise_service.clone(), scheduler_service.clone(), @@ -130,6 +139,8 @@ impl Bootstrap for ServerBootstrap { template_service, shard_manager_service, worker_service, + worker_enumeration_service, + running_worker_enumeration_service, promise_service, golem_config.clone(), invocation_key_service, From 09b95baa4d80750cc5b33aba54f8067d541e71e3 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Tue, 26 Mar 2024 10:52:21 +0100 Subject: [PATCH 12/25] test compilation fixes, WorkerFilter improvements --- golem-common/src/model/mod.rs | 60 +++++++++++-------- .../src/services/mod.rs | 1 - .../src/services/recovery.rs | 6 +- .../src/services/worker.rs | 13 ---- .../src/services/worker_enumeration.rs | 41 ++++++------- 5 files changed, 57 insertions(+), 64 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index c2c3125f0..762ba72fe 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -837,27 +837,10 @@ impl WorkerFilter { match self.clone() { WorkerFilter::Empty => true, WorkerFilter::Name { comparator, value } => { - match comparator { - FilterStringComparator::Equal => { - metadata.worker_id.worker_id.worker_name == value - } - FilterStringComparator::Like => metadata - .worker_id - .worker_id - .worker_name - .contains(value.as_str()), // FIXME - } + comparator.matches(&metadata.worker_id.worker_id.worker_name, &value) } WorkerFilter::Version { comparator, value } => { - let version = metadata.worker_id.template_version; - match comparator { - FilterComparator::Equal => version == value, - FilterComparator::NotEqual => version != value, - FilterComparator::Less => version < value, - FilterComparator::LessEqual => version <= value, - FilterComparator::Greater => version > value, - FilterComparator::GreaterEqual => version >= value, - } + comparator.matches(&metadata.worker_id.template_version, &value) } WorkerFilter::Env { name, @@ -867,10 +850,7 @@ impl WorkerFilter { let mut result = false; for env_value in metadata.env.clone() { if env_value.0 == name { - result = match comparator { - FilterStringComparator::Equal => env_value.1 == value, - FilterStringComparator::Like => env_value.1.contains(value.as_str()), // FIXME - }; + result = comparator.matches(&env_value.1, &value); break; } @@ -883,7 +863,7 @@ impl WorkerFilter { let mut result = true; for filter in filters { result = filter.matches(metadata); - if result == false { + if !result { break; } } @@ -895,7 +875,7 @@ impl WorkerFilter { result = false; for filter in filters { result = filter.matches(metadata); - if result == true { + if result { break; } } @@ -944,6 +924,17 @@ pub enum FilterStringComparator { Like, } +impl FilterStringComparator { + pub fn matches(&self, value1: &T, value2: &T) -> bool { + match self { + FilterStringComparator::Equal => value1.to_string() == value2.to_string(), + FilterStringComparator::Like => { + value1.to_string().contains(value2.to_string().as_str()) + } // FIXME + } + } +} + #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] pub enum FilterComparator { Equal, @@ -954,6 +945,25 @@ pub enum FilterComparator { Less, } +impl FilterComparator { + pub fn matches(&self, value1: &T, value2: &T) -> bool { + match self { + FilterComparator::Equal => value1.cmp(value2) == std::cmp::Ordering::Equal, + FilterComparator::NotEqual => value1.cmp(value2) != std::cmp::Ordering::Equal, + FilterComparator::Less => value1.cmp(value2) == std::cmp::Ordering::Less, + FilterComparator::LessEqual => { + let r = value1.cmp(value2); + r == std::cmp::Ordering::Equal || r == std::cmp::Ordering::Less + } + FilterComparator::Greater => value1.cmp(value2) == std::cmp::Ordering::Greater, + FilterComparator::GreaterEqual => { + let r = value1.cmp(value2); + r == std::cmp::Ordering::Equal || r == std::cmp::Ordering::Greater + } + } + } +} + #[cfg(test)] mod tests { use bincode::{Decode, Encode}; diff --git a/golem-worker-executor-base/src/services/mod.rs b/golem-worker-executor-base/src/services/mod.rs index 6b1fbd57d..47f92145f 100644 --- a/golem-worker-executor-base/src/services/mod.rs +++ b/golem-worker-executor-base/src/services/mod.rs @@ -16,7 +16,6 @@ use std::sync::Arc; #[cfg(any(feature = "mocks", test))] use std::time::Duration; -use crate::services::worker_enumeration::RunningWorkerEnumerationServiceDefault; use tokio::runtime::Handle; use crate::workerctx::WorkerCtx; diff --git a/golem-worker-executor-base/src/services/recovery.rs b/golem-worker-executor-base/src/services/recovery.rs index 3bc3d8147..80af9eb2e 100644 --- a/golem-worker-executor-base/src/services/recovery.rs +++ b/golem-worker-executor-base/src/services/recovery.rs @@ -603,8 +603,8 @@ mod tests { use crate::services::worker_event::WorkerEventService; use crate::services::{ All, HasAll, HasBlobStoreService, HasConfig, HasExtraDeps, HasInvocationKeyService, - HasKeyValueService, HasPromiseService, HasRpc, HasTemplateService, HasWasmtimeEngine, - HasWorkerService, + HasKeyValueService, HasPromiseService, HasRpc, HasRunningWorkerEnumerationService, + HasTemplateService, HasWasmtimeEngine, HasWorkerEnumerationService, HasWorkerService, }; use crate::workerctx::{ ExternalOperations, FuelManagement, InvocationHooks, InvocationManagement, IoCapturing, @@ -937,6 +937,8 @@ mod tests { runtime, deps.template_service(), deps.worker_service(), + deps.worker_enumeration_service(), + deps.running_worker_enumeration_service(), oplog, deps.promise_service(), scheduler, diff --git a/golem-worker-executor-base/src/services/worker.rs b/golem-worker-executor-base/src/services/worker.rs index 7452fcb45..10f781bb8 100644 --- a/golem-worker-executor-base/src/services/worker.rs +++ b/golem-worker-executor-base/src/services/worker.rs @@ -53,19 +53,6 @@ pub trait WorkerService { ); } -pub fn configured( - config: &WorkersServiceConfig, - redis_pool: RedisPool, - shard_service: Arc, -) -> Arc { - match config { - WorkersServiceConfig::InMemory => Arc::new(WorkerServiceInMemory::new()), - WorkersServiceConfig::Redis => { - Arc::new(WorkerServiceRedis::new(redis_pool.clone(), shard_service)) - } - } -} - #[derive(Clone)] pub struct WorkerServiceRedis { redis: RedisPool, diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 4994b9860..8936d5667 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -56,28 +56,26 @@ impl } #[cfg(any(feature = "mocks", test))] -pub struct RunningWorkerEnumerationServiceMock {} +pub struct RunningWorkerEnumerationServiceMock {} #[cfg(any(feature = "mocks", test))] -impl Default - for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock -{ +impl Default for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock { fn default() -> Self { Self::new() } } #[cfg(any(feature = "mocks", test))] -impl crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock { - pub fn new() -> Self { +impl crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock { + pub fn new() -> Self { Self {} } } #[cfg(any(feature = "mocks", test))] #[async_trait] -impl RunningWorkerEnumerationService - for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock +impl RunningWorkerEnumerationService + for crate::services::worker_enumeration::RunningWorkerEnumerationServiceMock { async fn get( &self, @@ -132,7 +130,7 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { let mut new_cursor: Option = None; let mut template_workers: Vec = vec![]; - let template_worker_redis_key = get_template_worker_redis_key(&template_id); + let template_worker_redis_key = get_template_worker_redis_key(template_id); let (new_redis_cursor, worker_redis_keys) = self .redis @@ -142,7 +140,7 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { .map_err(|e| GolemError::unknown(e.details()))?; for worker_redis_key in worker_redis_keys { - let worker_id = get_worker_id_from_redis_key(&worker_redis_key, &template_id)?; + let worker_id = get_worker_id_from_redis_key(&worker_redis_key, template_id)?; let worker_metadata = self.worker_service.get(&worker_id).await; @@ -181,8 +179,8 @@ impl WorkerEnumerationService let (next_cursor, workers) = self .get_internal( - &template_id, - &filter, + template_id, + filter, new_cursor.unwrap_or(0), new_count, precise, @@ -242,16 +240,14 @@ impl WorkerEnumerationService filter: &WorkerFilter, cursor: usize, count: usize, - precise: bool, + _precise: bool, ) -> Result<(Option, Vec), GolemError> { - // TODO implement precise let workers = self.worker_service.enumerate().await; let all_workers_count = workers.len(); - let mut template_workers: Vec = vec![]; - - let new_cursor = if all_workers_count > cursor { + if all_workers_count > cursor { + let mut template_workers: Vec = vec![]; let mut index = 0; for worker in workers { if index >= cursor @@ -261,21 +257,20 @@ impl WorkerEnumerationService template_workers.push(worker); } - index = index + 1; + index += 1; if template_workers.len() == count { break; } } if index >= all_workers_count { - None + Ok((None, template_workers)) } else { - Some(index) + Ok((Some(index), template_workers)) } } else { - None - }; - Ok((new_cursor, template_workers)) + Ok((None, vec![])) + } } } From 8dc8cdacb0ae63dc1246bc215f64da51bce5474b Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Tue, 26 Mar 2024 18:18:18 +0100 Subject: [PATCH 13/25] Grpc api - initial impl --- .../workerexecutor/worker_executor.proto | 38 ++++++ golem-common/src/model/mod.rs | 36 ++++-- golem-worker-executor-base/src/grpc.rs | 122 +++++++++++++++++- .../src/services/worker.rs | 1 - 4 files changed, 181 insertions(+), 16 deletions(-) diff --git a/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto b/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto index dfd3de712..b647b4046 100644 --- a/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto +++ b/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto @@ -30,6 +30,8 @@ service WorkerExecutor { rpc AssignShards(AssignShardsRequest) returns (AssignShardsResponse); rpc GetWorkerMetadata(golem.worker.WorkerId) returns (GetWorkerMetadataResponse); rpc ResumeWorker(ResumeWorkerRequest) returns (ResumeWorkerResponse); + rpc GetRunningWorkerMetadatas(GetRunningWorkerMetadatasRequest) returns (GetRunningWorkerMetadatasResponse); + rpc GetWorkerMetadatas(GetWorkerMetadatasRequest) returns (GetWorkerMetadatasResponse); } message InvokeWorkerResponse { @@ -179,3 +181,39 @@ message ResumeWorkerResponse { golem.worker.WorkerExecutionError failure = 2; } } + +message GetRunningWorkerMetadatasRequest { + golem.template.TemplateId template_id = 1; +} + + +message GetRunningWorkerMetadatasResponse { + oneof result { + GetRunningWorkerMetadatasSuccessResponse success = 1; + golem.worker.WorkerExecutionError failure = 2; + } +} + +message GetRunningWorkerMetadatasSuccessResponse { + repeated golem.worker.WorkerMetadata workers = 1; +} + +message GetWorkerMetadatasRequest { + golem.template.TemplateId template_id = 1; + uint64 cursor = 2; + uint64 count = 3; + bool precise = 4; +} + + +message GetWorkerMetadatasResponse { + oneof result { + GetWorkerMetadatasSuccessResponse success = 1; + golem.worker.WorkerExecutionError failure = 2; + } +} + +message GetWorkerMetadatasSuccessResponse { + repeated golem.worker.WorkerMetadata workers = 1; + uint64 cursor = 2; +} \ No newline at end of file diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index 762ba72fe..0a7db56b4 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::borrow::Cow; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; use std::ops::Add; use std::str::FromStr; @@ -27,6 +27,7 @@ use bincode::enc::Encoder; use bincode::error::{DecodeError, EncodeError}; use bincode::{BorrowDecode, Decode, Encode}; use derive_more::FromStr; +use golem_api_grpc::proto::golem; use poem_openapi::registry::{MetaSchema, MetaSchemaRef}; use poem_openapi::types::{ParseFromJSON, ParseFromParameter, ParseResult, ToJSON}; use poem_openapi::{Enum, Object}; @@ -520,6 +521,21 @@ impl WorkerMetadata { } } +impl From for golem_api_grpc::proto::golem::worker::WorkerMetadata { + fn from(value: WorkerMetadata) -> Self { + golem_api_grpc::proto::golem::worker::WorkerMetadata { + worker_id: Some(value.worker_id.worker_id.into_proto()), + account_id: Some(value.account_id.into()), + args: value.args, + env: HashMap::from_iter(value.env.iter().cloned()), + template_version: value.worker_id.template_version, + status: Into::::into(value.last_known_status.status) + .into(), + retry_count: 0, // FIXME + } + } +} + /// Contains status information about a worker according to a given oplog index. /// This status is just cached information, all fields must be computable by the oplog alone. /// By having an associated oplog_idx, the cached information can be used together with the @@ -948,18 +964,12 @@ pub enum FilterComparator { impl FilterComparator { pub fn matches(&self, value1: &T, value2: &T) -> bool { match self { - FilterComparator::Equal => value1.cmp(value2) == std::cmp::Ordering::Equal, - FilterComparator::NotEqual => value1.cmp(value2) != std::cmp::Ordering::Equal, - FilterComparator::Less => value1.cmp(value2) == std::cmp::Ordering::Less, - FilterComparator::LessEqual => { - let r = value1.cmp(value2); - r == std::cmp::Ordering::Equal || r == std::cmp::Ordering::Less - } - FilterComparator::Greater => value1.cmp(value2) == std::cmp::Ordering::Greater, - FilterComparator::GreaterEqual => { - let r = value1.cmp(value2); - r == std::cmp::Ordering::Equal || r == std::cmp::Ordering::Greater - } + FilterComparator::Equal => value1 == value2, + FilterComparator::NotEqual => value1 != value2, + FilterComparator::Less => value1 < value2, + FilterComparator::LessEqual => value1 <= value2, + FilterComparator::Greater => value1 > value2, + FilterComparator::GreaterEqual => value1 >= value2, } } } diff --git a/golem-worker-executor-base/src/grpc.rs b/golem-worker-executor-base/src/grpc.rs index c649c284c..63f67771e 100644 --- a/golem-worker-executor-base/src/grpc.rs +++ b/golem-worker-executor-base/src/grpc.rs @@ -22,10 +22,15 @@ use std::sync::Arc; use golem_api_grpc::proto::golem; use golem_api_grpc::proto::golem::common::ResourceLimits as GrpcResourceLimits; use golem_api_grpc::proto::golem::workerexecutor::worker_executor_server::WorkerExecutor; +use golem_api_grpc::proto::golem::workerexecutor::{ + GetRunningWorkerMetadatasRequest, GetRunningWorkerMetadatasResponse, GetWorkerMetadatasRequest, + GetWorkerMetadatasResponse, +}; use golem_common::cache::PendingOrFinal; use golem_common::model as common_model; use golem_common::model::{ - AccountId, CallingConvention, InvocationKey, ShardId, WorkerMetadata, WorkerStatus, + AccountId, CallingConvention, InvocationKey, ShardId, WorkerFilter, WorkerMetadata, + WorkerStatus, }; use golem_wasm_rpc::protobuf::Val; use tokio::sync::mpsc; @@ -44,7 +49,8 @@ use crate::services::worker_activator::{DefaultWorkerActivator, LazyWorkerActiva use crate::services::worker_event::LogLevel; use crate::services::{ worker_event, All, HasActiveWorkers, HasAll, HasInvocationKeyService, HasPromiseService, - HasShardManagerService, HasShardService, HasWorkerService, UsesAllDeps, + HasRunningWorkerEnumerationService, HasShardManagerService, HasShardService, + HasWorkerEnumerationService, HasWorkerService, UsesAllDeps, }; use crate::worker::{invoke, invoke_and_await, Worker}; use crate::workerctx::{PublicWorkerIo, WorkerCtx}; @@ -668,6 +674,43 @@ impl + UsesAllDeps + Send + Sync + // TODO: add error details }) } + + async fn get_running_worker_metadatas_internal( + &self, + request: golem::workerexecutor::GetRunningWorkerMetadatasRequest, + ) -> Result, GolemError> { + let template_id: common_model::TemplateId = + request.template_id.unwrap().try_into().unwrap(); + let workers = self + .running_worker_enumeration_service() + .get(&template_id, &WorkerFilter::Empty) // FIXME filter + .await?; + let result: Vec = + workers.iter().map(|worker| worker.clone().into()).collect(); + Ok(result) + } + + async fn get_worker_metadatas_internal( + &self, + request: golem::workerexecutor::GetWorkerMetadatasRequest, + ) -> Result<(Option, Vec), GolemError> { + let template_id: common_model::TemplateId = + request.template_id.unwrap().try_into().unwrap(); + let (new_cursor, workers) = self + .worker_enumeration_service() + .get( + &template_id, + &WorkerFilter::Empty, // FIXME filter + request.cursor as usize, + request.count as usize, + request.precise, + ) + .await?; + + let result: Vec = + workers.iter().map(|worker| worker.clone().into()).collect(); + Ok((new_cursor, result)) + } } impl + UsesAllDeps + Send + Sync + 'static> UsesAllDeps @@ -1208,6 +1251,81 @@ impl + UsesAllDeps + Send + Sync + ), } } + + async fn get_running_worker_metadatas( + &self, + request: Request, + ) -> Result, Status> { + let request = request.into_inner(); + let record = RecordedGrpcRequest::new( + "get_running_worker_metadatas", + format!("template_id={:?}", request.template_id), + ); + let result = self.get_running_worker_metadatas_internal(request).await; + match result { + Ok(workers) => record.succeed(Ok(Response::new( + golem::workerexecutor::GetRunningWorkerMetadatasResponse { + result: Some( + golem::workerexecutor::get_running_worker_metadatas_response::Result::Success( + golem::workerexecutor::GetRunningWorkerMetadatasSuccessResponse { + workers + } + ), + ), + }, + ))), + Err(err) => record.fail( + Ok(Response::new( + golem::workerexecutor::GetRunningWorkerMetadatasResponse { + result: Some( + golem::workerexecutor::get_running_worker_metadatas_response::Result::Failure( + err.clone().into(), + ), + ), + }, + )), + &err, + ), + } + } + + async fn get_worker_metadatas( + &self, + request: Request, + ) -> Result, Status> { + let request = request.into_inner(); + let record = RecordedGrpcRequest::new( + "get_worker_metadatas", + format!("template_id={:?}", request.template_id), + ); + let result = self.get_worker_metadatas_internal(request).await; + match result { + Ok((cursor, workers)) => record.succeed(Ok(Response::new( + golem::workerexecutor::GetWorkerMetadatasResponse { + result: Some( + golem::workerexecutor::get_worker_metadatas_response::Result::Success( + golem::workerexecutor::GetWorkerMetadatasSuccessResponse { + workers, + cursor: cursor.unwrap_or(0) as u64, // FIXME can we do option in proto? + }, + ), + ), + }, + ))), + Err(err) => record.fail( + Ok(Response::new( + golem::workerexecutor::GetWorkerMetadatasResponse { + result: Some( + golem::workerexecutor::get_worker_metadatas_response::Result::Failure( + err.clone().into(), + ), + ), + }, + )), + &err, + ), + } + } } trait GrpcInvokeRequest { diff --git a/golem-worker-executor-base/src/services/worker.rs b/golem-worker-executor-base/src/services/worker.rs index 10f781bb8..c6845152c 100644 --- a/golem-worker-executor-base/src/services/worker.rs +++ b/golem-worker-executor-base/src/services/worker.rs @@ -27,7 +27,6 @@ use tracing::debug; use crate::error::GolemError; use crate::metrics::workers::record_worker_call; -use crate::services::golem_config::WorkersServiceConfig; use crate::services::shard::ShardService; /// Service for persisting the current set of Golem workers represented by their metadata From c3882d112d240ef7eb114fba8ae765661c1b9ff0 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Tue, 26 Mar 2024 19:42:03 +0100 Subject: [PATCH 14/25] Grpc api - WorkerFilter --- golem-api-grpc/build.rs | 1 + .../golem/common/filter_comparator.proto | 12 ++++ .../common/string_filter_comparator.proto | 9 +++ .../proto/golem/worker/worker_filter.proto | 53 ++++++++++++++++++ .../workerexecutor/worker_executor.proto | 9 ++- golem-common/src/model/mod.rs | 55 ++++++++++++++----- 6 files changed, 121 insertions(+), 18 deletions(-) create mode 100644 golem-api-grpc/proto/golem/common/filter_comparator.proto create mode 100644 golem-api-grpc/proto/golem/common/string_filter_comparator.proto create mode 100644 golem-api-grpc/proto/golem/worker/worker_filter.proto diff --git a/golem-api-grpc/build.rs b/golem-api-grpc/build.rs index 5b2dc2fe1..886c66ca5 100644 --- a/golem-api-grpc/build.rs +++ b/golem-api-grpc/build.rs @@ -52,6 +52,7 @@ fn main() -> Result<(), Box> { "proto/golem/worker/worker_error.proto", "proto/golem/worker/worker_id.proto", "proto/golem/worker/worker_metadata.proto", + "proto/golem/worker/worker_filter.proto", "proto/golem/worker/worker_status.proto", "proto/golem/worker/worker_service.proto", "proto/golem/workerexecutor/worker_executor.proto", diff --git a/golem-api-grpc/proto/golem/common/filter_comparator.proto b/golem-api-grpc/proto/golem/common/filter_comparator.proto new file mode 100644 index 000000000..8e4346103 --- /dev/null +++ b/golem-api-grpc/proto/golem/common/filter_comparator.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package golem.common; + +enum FilterComparator { + EQUAL = 0; + NOT_EQUAL = 1; + LESS = 2; + LESS_EQUAL = 3; + GREATER = 4; + GREATER_EQUAL = 5; +} \ No newline at end of file diff --git a/golem-api-grpc/proto/golem/common/string_filter_comparator.proto b/golem-api-grpc/proto/golem/common/string_filter_comparator.proto new file mode 100644 index 000000000..59b2f1a6a --- /dev/null +++ b/golem-api-grpc/proto/golem/common/string_filter_comparator.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package golem.common; + + +enum StringFilterComparator { + STRING_EQUAL = 0; + STRING_LIKE = 1; +} \ No newline at end of file diff --git a/golem-api-grpc/proto/golem/worker/worker_filter.proto b/golem-api-grpc/proto/golem/worker/worker_filter.proto new file mode 100644 index 000000000..dcf06fa82 --- /dev/null +++ b/golem-api-grpc/proto/golem/worker/worker_filter.proto @@ -0,0 +1,53 @@ +syntax = "proto3"; + +package golem.worker; + +import "golem/common/account_id.proto"; +import "golem/common/string_filter_comparator.proto"; +import "golem/common/filter_comparator.proto"; +import "golem/worker/worker_id.proto"; +import "golem/worker/worker_status.proto"; + +message WorkerFilter { + oneof filter { + WorkerNameFilter name = 1; + WorkerVersionFilter version = 2; + WorkerStatusFilter status = 3; + WorkerEnvFilter env = 4; + WorkerAndFilter and = 5; + WorkerOrFilter or = 6; + WorkerNotFilter not = 7; + } +} + +message WorkerNotFilter { + WorkerFilter filter = 1; +} + +message WorkerAndFilter { + repeated WorkerFilter filters = 1; +} + +message WorkerOrFilter { + repeated WorkerFilter filters = 1; +} + +message WorkerNameFilter { + golem.common.StringFilterComparator comparator = 1; + string value = 2; +} + +message WorkerVersionFilter { + golem.common.FilterComparator comparator = 1; + int32 value = 2; +} + +message WorkerStatusFilter { + WorkerStatus value = 1; +} + +message WorkerEnvFilter { + string name = 1; + golem.common.StringFilterComparator comparator = 2; + string value = 3; +} \ No newline at end of file diff --git a/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto b/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto index b647b4046..ea4f40c98 100644 --- a/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto +++ b/golem-api-grpc/proto/golem/workerexecutor/worker_executor.proto @@ -12,6 +12,7 @@ import public "golem/template/template_id.proto"; import public "golem/worker/worker_id.proto"; import public "golem/worker/worker_metadata.proto"; import public "golem/worker/worker_status.proto"; +import public "golem/worker/worker_filter.proto"; import public "golem/worker/worker_execution_error.proto"; import public "wasm/rpc/val.proto"; @@ -184,6 +185,7 @@ message ResumeWorkerResponse { message GetRunningWorkerMetadatasRequest { golem.template.TemplateId template_id = 1; + golem.worker.WorkerFilter filter = 2; } @@ -200,9 +202,10 @@ message GetRunningWorkerMetadatasSuccessResponse { message GetWorkerMetadatasRequest { golem.template.TemplateId template_id = 1; - uint64 cursor = 2; - uint64 count = 3; - bool precise = 4; + golem.worker.WorkerFilter filter = 2; + uint64 cursor = 3; + uint64 count = 4; + bool precise = 5; } diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index 0a7db56b4..5bdc7586e 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -793,11 +793,36 @@ pub fn parse_function_name(name: &str) -> ParsedFunctionName { } } +// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +// pub struct WorkerNameFilter { +// pub comparator: StringFilterComparator, +// pub value: String, +// } +// +// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +// pub struct WorkerStatusFilter { +// pub value: WorkerStatus +// } +// +// +// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +// pub struct WorkerVersionFilter { +// pub comparator: FilterComparator, +// pub value: i32, +// } +// +// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +// pub struct WorkerVersionEnvFilter { +// pub name: String, +// pub comparator: StringFilterComparator, +// pub value: String +// } + #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] pub enum WorkerFilter { Empty, Name { - comparator: FilterStringComparator, + comparator: StringFilterComparator, value: String, }, Status { @@ -813,7 +838,7 @@ pub enum WorkerFilter { // }, Env { name: String, - comparator: FilterStringComparator, + comparator: StringFilterComparator, value: String, }, And(Vec), @@ -913,11 +938,11 @@ impl WorkerFilter { WorkerFilter::Not(Box::new(filter)) } - pub fn new_name(comparator: FilterStringComparator, value: String) -> Self { + pub fn new_name(comparator: StringFilterComparator, value: String) -> Self { WorkerFilter::Name { comparator, value } } - pub fn new_env(name: String, comparator: FilterStringComparator, value: String) -> Self { + pub fn new_env(name: String, comparator: StringFilterComparator, value: String) -> Self { WorkerFilter::Env { name, comparator, @@ -935,16 +960,16 @@ impl WorkerFilter { } #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -pub enum FilterStringComparator { +pub enum StringFilterComparator { Equal, Like, } -impl FilterStringComparator { +impl StringFilterComparator { pub fn matches(&self, value1: &T, value2: &T) -> bool { match self { - FilterStringComparator::Equal => value1.to_string() == value2.to_string(), - FilterStringComparator::Like => { + StringFilterComparator::Equal => value1.to_string() == value2.to_string(), + StringFilterComparator::Like => { value1.to_string().contains(value2.to_string().as_str()) } // FIXME } @@ -980,7 +1005,7 @@ mod tests { use serde::{Deserialize, Serialize}; use crate::model::{ - parse_function_name, AccountId, FilterComparator, FilterStringComparator, TemplateId, + parse_function_name, AccountId, FilterComparator, StringFilterComparator, TemplateId, VersionedWorkerId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus, WorkerStatusRecord, }; @@ -1132,14 +1157,14 @@ mod tests { assert!(WorkerFilter::Empty.matches(&worker_metadata)); assert!( - WorkerFilter::new_name(FilterStringComparator::Equal, "worker-1".to_string()) + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) .and(WorkerFilter::new_status(WorkerStatus::Idle)) .matches(&worker_metadata) ); assert!(WorkerFilter::new_env( "env1".to_string(), - FilterStringComparator::Equal, + StringFilterComparator::Equal, "value1".to_string(), ) .and(WorkerFilter::new_status(WorkerStatus::Idle)) @@ -1147,7 +1172,7 @@ mod tests { assert!(WorkerFilter::new_env( "env1".to_string(), - FilterStringComparator::Equal, + StringFilterComparator::Equal, "value2".to_string(), ) .not() @@ -1158,13 +1183,13 @@ mod tests { .matches(&worker_metadata)); assert!( - WorkerFilter::new_name(FilterStringComparator::Equal, "worker-1".to_string()) + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) .and(WorkerFilter::new_version(FilterComparator::Equal, 1)) .matches(&worker_metadata) ); assert!( - WorkerFilter::new_name(FilterStringComparator::Equal, "worker-2".to_string()) + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-2".to_string()) .or(WorkerFilter::new_version(FilterComparator::Equal, 1)) .matches(&worker_metadata) ); @@ -1172,7 +1197,7 @@ mod tests { assert!(WorkerFilter::new_version(FilterComparator::GreaterEqual, 1) .and(WorkerFilter::new_version(FilterComparator::Less, 2)) .or(WorkerFilter::new_name( - FilterStringComparator::Equal, + StringFilterComparator::Equal, "worker-2".to_string(), )) .matches(&worker_metadata)); From 6661c46d60adab1bf08a8bc708aae4d353b81b99 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Wed, 27 Mar 2024 13:03:16 +0100 Subject: [PATCH 15/25] Grpc api - WorkerFilter --- golem-common/src/model/mod.rs | 256 ++++++++++++++++-- golem-common/src/redis.rs | 1 - golem-worker-executor-base/src/grpc.rs | 37 ++- .../src/services/worker_enumeration.rs | 26 +- 4 files changed, 274 insertions(+), 46 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index 5bdc7586e..df9c2a7e8 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -818,9 +818,8 @@ pub fn parse_function_name(name: &str) -> ParsedFunctionName { // pub value: String // } -#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Encode, Decode)] pub enum WorkerFilter { - Empty, Name { comparator: StringFilterComparator, value: String, @@ -848,35 +847,25 @@ pub enum WorkerFilter { impl WorkerFilter { pub fn and(&self, filter: WorkerFilter) -> Self { - match self { - WorkerFilter::Empty => filter, - _ => match filter { - WorkerFilter::And(filters) => Self::new_and([vec![self.clone()], filters].concat()), - _ => Self::new_and(vec![self.clone(), filter]), - }, + match self.clone() { + WorkerFilter::And(filters) => Self::new_and([filters, vec![filter]].concat()), + f => Self::new_and(vec![f, filter]), } } pub fn or(&self, filter: WorkerFilter) -> Self { - match self { - WorkerFilter::Empty => filter, - _ => match filter { - WorkerFilter::Or(filters) => Self::new_or([vec![self.clone()], filters].concat()), - _ => Self::new_or(vec![self.clone(), filter]), - }, + match self.clone() { + WorkerFilter::Or(filters) => Self::new_or([filters, vec![filter]].concat()), + f => Self::new_or(vec![f, filter]), } } pub fn not(&self) -> Self { - match self { - WorkerFilter::Empty => self.clone(), - _ => Self::new_not(self.clone()), - } + Self::new_not(self.clone()) } pub fn matches(&self, metadata: &WorkerMetadata) -> bool { match self.clone() { - WorkerFilter::Empty => true, WorkerFilter::Name { comparator, value } => { comparator.matches(&metadata.worker_id.worker_id.worker_name, &value) } @@ -903,8 +892,8 @@ impl WorkerFilter { WorkerFilter::And(filters) => { let mut result = true; for filter in filters { - result = filter.matches(metadata); - if !result { + if !filter.matches(metadata) { + result = false; break; } } @@ -915,8 +904,8 @@ impl WorkerFilter { if !filters.is_empty() { result = false; for filter in filters { - result = filter.matches(metadata); - if result { + if filter.matches(metadata) { + result = true; break; } } @@ -959,7 +948,58 @@ impl WorkerFilter { } } -#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +impl TryFrom for WorkerFilter { + type Error = String; + + fn try_from( + value: golem_api_grpc::proto::golem::worker::WorkerFilter, + ) -> Result { + match value.filter { + Some(filter) => match filter { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Name(filter) => Ok( + WorkerFilter::new_name(filter.comparator.try_into()?, filter.value), + ), + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Version(filter) => Ok( + WorkerFilter::new_version(filter.comparator.try_into()?, filter.value), + ), + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Status(filter) => { + Ok(WorkerFilter::new_status(filter.value.try_into()?)) + } + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Env(filter) => Ok( + WorkerFilter::new_env(filter.name, filter.comparator.try_into()?, filter.value), + ), + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Not(filter) => { + let filter = *filter.filter.ok_or_else(|| "Missing filter".to_string())?; + Ok(WorkerFilter::new_not(filter.try_into()?)) + } + golem_api_grpc::proto::golem::worker::worker_filter::Filter::And( + golem_api_grpc::proto::golem::worker::WorkerAndFilter { filters }, + ) => { + let filters = filters.into_iter().map(|f| f.try_into()).collect::, + String, + >>( + )?; + + Ok(WorkerFilter::new_and(filters)) + } + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Or( + golem_api_grpc::proto::golem::worker::WorkerOrFilter { filters }, + ) => { + let filters = filters.into_iter().map(|f| f.try_into()).collect::, + String, + >>( + )?; + Ok(WorkerFilter::new_or(filters)) + } + }, + None => Err("Missing filter".to_string()), + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Encode, Decode, Enum)] pub enum StringFilterComparator { Equal, Like, @@ -976,7 +1016,41 @@ impl StringFilterComparator { } } -#[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] +impl From for golem_api_grpc::proto::golem::common::StringFilterComparator { + fn from(value: StringFilterComparator) -> Self { + match value { + StringFilterComparator::Equal => { + golem_api_grpc::proto::golem::common::StringFilterComparator::StringEqual + } + StringFilterComparator::Like => { + golem_api_grpc::proto::golem::common::StringFilterComparator::StringLike + } + } + } +} + +impl TryFrom for StringFilterComparator { + type Error = String; + + fn try_from(value: i32) -> Result { + match value { + 0 => Ok(StringFilterComparator::Equal), + 1 => Ok(StringFilterComparator::Like), + _ => Err(format!("Unknown String Filter Comparator: {}", value)), + } + } +} + +impl From for i32 { + fn from(value: StringFilterComparator) -> Self { + match value { + StringFilterComparator::Equal => 0, + StringFilterComparator::Like => 1, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Encode, Decode, Enum)] pub enum FilterComparator { Equal, NotEqual, @@ -999,10 +1073,63 @@ impl FilterComparator { } } +impl From for golem_api_grpc::proto::golem::common::FilterComparator { + fn from(value: FilterComparator) -> Self { + match value { + FilterComparator::Equal => { + golem_api_grpc::proto::golem::common::FilterComparator::Equal + } + FilterComparator::NotEqual => { + golem_api_grpc::proto::golem::common::FilterComparator::NotEqual + } + FilterComparator::Less => golem_api_grpc::proto::golem::common::FilterComparator::Less, + FilterComparator::LessEqual => { + golem_api_grpc::proto::golem::common::FilterComparator::LessEqual + } + FilterComparator::Greater => { + golem_api_grpc::proto::golem::common::FilterComparator::Greater + } + FilterComparator::GreaterEqual => { + golem_api_grpc::proto::golem::common::FilterComparator::GreaterEqual + } + } + } +} + +impl TryFrom for FilterComparator { + type Error = String; + + fn try_from(value: i32) -> Result { + match value { + 0 => Ok(FilterComparator::Equal), + 1 => Ok(FilterComparator::NotEqual), + 2 => Ok(FilterComparator::Less), + 3 => Ok(FilterComparator::LessEqual), + 4 => Ok(FilterComparator::Greater), + 5 => Ok(FilterComparator::GreaterEqual), + _ => Err(format!("Unknown Filter Comparator: {}", value)), + } + } +} + +impl From for i32 { + fn from(value: FilterComparator) -> Self { + match value { + FilterComparator::Equal => 0, + FilterComparator::NotEqual => 1, + FilterComparator::Less => 2, + FilterComparator::LessEqual => 3, + FilterComparator::Greater => 4, + FilterComparator::GreaterEqual => 5, + } + } +} + #[cfg(test)] mod tests { use bincode::{Decode, Encode}; use serde::{Deserialize, Serialize}; + use std::vec; use crate::model::{ parse_function_name, AccountId, FilterComparator, StringFilterComparator, TemplateId, @@ -1132,6 +1259,83 @@ mod tests { assert_eq!(json, "{\"account_id\":\"account-1\"}"); } + #[test] + fn worker_filter_combination() { + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()).not(), + WorkerFilter::new_not(WorkerFilter::new_name( + StringFilterComparator::Equal, + "worker-1".to_string() + )) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .and(WorkerFilter::new_status(WorkerStatus::Running)), + WorkerFilter::new_and(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running) + ]) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .and(WorkerFilter::new_status(WorkerStatus::Running)) + .and(WorkerFilter::new_version(FilterComparator::Equal, 1)), + WorkerFilter::new_and(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running), + WorkerFilter::new_version(FilterComparator::Equal, 1) + ]) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .or(WorkerFilter::new_status(WorkerStatus::Running)), + WorkerFilter::new_or(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running) + ]) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .or(WorkerFilter::new_status(WorkerStatus::Running)) + .or(WorkerFilter::new_version(FilterComparator::Equal, 1)), + WorkerFilter::new_or(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running), + WorkerFilter::new_version(FilterComparator::Equal, 1) + ]) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .and(WorkerFilter::new_status(WorkerStatus::Running)) + .or(WorkerFilter::new_version(FilterComparator::Equal, 1)), + WorkerFilter::new_or(vec![ + WorkerFilter::new_and(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running) + ]), + WorkerFilter::new_version(FilterComparator::Equal, 1) + ]) + ); + + assert_eq!( + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) + .or(WorkerFilter::new_status(WorkerStatus::Running)) + .and(WorkerFilter::new_version(FilterComparator::Equal, 1)), + WorkerFilter::new_and(vec![ + WorkerFilter::new_or(vec![ + WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()), + WorkerFilter::new_status(WorkerStatus::Running) + ]), + WorkerFilter::new_version(FilterComparator::Equal, 1) + ]) + ); + } + #[test] fn worker_filter_matches() { let template_id = TemplateId::new_v4(); @@ -1154,8 +1358,6 @@ mod tests { last_known_status: WorkerStatusRecord::default(), }; - assert!(WorkerFilter::Empty.matches(&worker_metadata)); - assert!( WorkerFilter::new_name(StringFilterComparator::Equal, "worker-1".to_string()) .and(WorkerFilter::new_status(WorkerStatus::Idle)) diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index f3ffc97e2..cd81cebbf 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -636,7 +636,6 @@ impl<'a> RedisLabelledApi<'a> { Ok(connected_slaves) } - pub async fn scan( &self, pattern: K, diff --git a/golem-worker-executor-base/src/grpc.rs b/golem-worker-executor-base/src/grpc.rs index 63f67771e..1671c33a8 100644 --- a/golem-worker-executor-base/src/grpc.rs +++ b/golem-worker-executor-base/src/grpc.rs @@ -679,14 +679,27 @@ impl + UsesAllDeps + Send + Sync + &self, request: golem::workerexecutor::GetRunningWorkerMetadatasRequest, ) -> Result, GolemError> { - let template_id: common_model::TemplateId = - request.template_id.unwrap().try_into().unwrap(); + let template_id: common_model::TemplateId = request + .template_id + .and_then(|t| t.try_into().ok()) + .ok_or(GolemError::invalid_request("Invalid template id"))?; + + let filter: Option = match request.filter { + Some(f) => { + let f = f.try_into().map_err(|e| GolemError::invalid_request(e))?; + Some(f) + } + None => None, + }; + let workers = self .running_worker_enumeration_service() - .get(&template_id, &WorkerFilter::Empty) // FIXME filter + .get(&template_id, filter) .await?; + let result: Vec = workers.iter().map(|worker| worker.clone().into()).collect(); + Ok(result) } @@ -694,13 +707,24 @@ impl + UsesAllDeps + Send + Sync + &self, request: golem::workerexecutor::GetWorkerMetadatasRequest, ) -> Result<(Option, Vec), GolemError> { - let template_id: common_model::TemplateId = - request.template_id.unwrap().try_into().unwrap(); + let template_id: common_model::TemplateId = request + .template_id + .and_then(|t| t.try_into().ok()) + .ok_or(GolemError::invalid_request("Invalid template id"))?; + + let filter: Option = match request.filter { + Some(f) => { + let f = f.try_into().map_err(|e| GolemError::invalid_request(e))?; + Some(f) + } + None => None, + }; + let (new_cursor, workers) = self .worker_enumeration_service() .get( &template_id, - &WorkerFilter::Empty, // FIXME filter + filter, request.cursor as usize, request.count as usize, request.precise, @@ -709,6 +733,7 @@ impl + UsesAllDeps + Send + Sync + let result: Vec = workers.iter().map(|worker| worker.clone().into()).collect(); + Ok((new_cursor, result)) } } diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 8936d5667..e57560af8 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -13,7 +13,7 @@ pub trait RunningWorkerEnumerationService { async fn get( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, ) -> Result, GolemError>; } @@ -29,7 +29,7 @@ impl RunningWorkerEnumerationService async fn get( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, ) -> Result, GolemError> { let active_workers = self.active_workers.enum_workers(); @@ -37,7 +37,9 @@ impl RunningWorkerEnumerationService for worker in active_workers { if worker.0.template_id == *template_id && worker.1.metadata.last_known_status.status == WorkerStatus::Running - && filter.matches(&worker.1.metadata) + && filter + .clone() + .map_or(true, |f| f.matches(&worker.1.metadata)) { template_workers.push(worker.1.metadata.clone()); } @@ -80,7 +82,7 @@ impl RunningWorkerEnumerationService async fn get( &self, _template_id: &TemplateId, - _filter: &WorkerFilter, + _filter: Option, ) -> Result, GolemError> { unimplemented!() } @@ -91,7 +93,7 @@ pub trait WorkerEnumerationService { async fn get( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, cursor: usize, count: usize, precise: bool, @@ -121,7 +123,7 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { async fn get_internal( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, cursor: usize, count: usize, precise: bool, @@ -145,7 +147,7 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { let worker_metadata = self.worker_service.get(&worker_id).await; if let Some(worker_metadata) = worker_metadata { - if filter.matches(&worker_metadata) { + if filter.clone().map_or(true, |f| f.matches(&worker_metadata)) { template_workers.push(worker_metadata); } } @@ -166,7 +168,7 @@ impl WorkerEnumerationService async fn get( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, cursor: usize, count: usize, precise: bool, @@ -180,7 +182,7 @@ impl WorkerEnumerationService let (next_cursor, workers) = self .get_internal( template_id, - filter, + filter.clone(), new_cursor.unwrap_or(0), new_count, precise, @@ -237,7 +239,7 @@ impl WorkerEnumerationService async fn get( &self, template_id: &TemplateId, - filter: &WorkerFilter, + filter: Option, cursor: usize, count: usize, _precise: bool, @@ -252,7 +254,7 @@ impl WorkerEnumerationService for worker in workers { if index >= cursor && worker.worker_id.worker_id.template_id == *template_id - && filter.matches(&worker) + && filter.clone().map_or(true, |f| f.matches(&worker)) { template_workers.push(worker); } @@ -299,7 +301,7 @@ impl WorkerEnumerationService async fn get( &self, _template_id: &TemplateId, - _filter: &WorkerFilter, + _filter: Option, _cursor: usize, _count: usize, _precise: bool, From 878002a0f3bdc9bbd5a81bea996612b322e8c6db Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Wed, 27 Mar 2024 20:24:22 +0100 Subject: [PATCH 16/25] basic tests, fixes --- .../src/services/worker_enumeration.rs | 10 +- golem-worker-executor-base/tests/api.rs | 9 ++ .../tests/common/mod.rs | 140 ++++++++++++++---- 3 files changed, 126 insertions(+), 33 deletions(-) diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index e57560af8..095daa53e 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -36,7 +36,8 @@ impl RunningWorkerEnumerationService let mut template_workers: Vec = vec![]; for worker in active_workers { if worker.0.template_id == *template_id - && worker.1.metadata.last_known_status.status == WorkerStatus::Running + && (worker.1.metadata.last_known_status.status == WorkerStatus::Running + || worker.1.metadata.last_known_status.status == WorkerStatus::Idle) && filter .clone() .map_or(true, |f| f.matches(&worker.1.metadata)) @@ -177,7 +178,7 @@ impl WorkerEnumerationService let mut template_workers: Vec = vec![]; while new_cursor.is_some() && template_workers.len() < count { - let new_count = template_workers.len() - count; + let new_count = count - template_workers.len(); let (next_cursor, workers) = self .get_internal( @@ -208,8 +209,9 @@ fn get_worker_id_from_redis_key( ) -> Result { let template_prefix = format!("instance:instance:{}:", template_id.0); - if worker_redis_key.starts_with(&template_prefix) { - let worker_name = &worker_redis_key[template_prefix.len()..]; + // find because of redis key_prefix - figure out better solution + if let Some(i) = worker_redis_key.find(&template_prefix) { + let worker_name = &worker_redis_key[(i + template_prefix.len())..]; Ok(WorkerId { worker_name: worker_name.to_string(), template_id: template_id.clone(), diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index a238169d2..096b74156 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -635,9 +635,18 @@ async fn delete_instance() { .unwrap(); let metadata1 = executor.get_worker_metadata(&worker_id).await; + let metadatas1r = executor + .get_running_worker_metadatas(&worker_id.template_id) + .await; + let (cursor1, metadatas1) = executor.get_worker_metadatas(&worker_id.template_id).await; + executor.delete_worker(&worker_id).await; + let metadata2 = executor.get_worker_metadata(&worker_id).await; + check!(metadatas1r.len() == 1); + check!(metadatas1.len() == 1); + check!(cursor1.is_none()); check!(metadata1.is_some()); check!(metadata2.is_none()); } diff --git a/golem-worker-executor-base/tests/common/mod.rs b/golem-worker-executor-base/tests/common/mod.rs index 0dcf05bad..a28c11e4e 100644 --- a/golem-worker-executor-base/tests/common/mod.rs +++ b/golem-worker-executor-base/tests/common/mod.rs @@ -24,11 +24,13 @@ use golem_api_grpc::proto::golem::worker::{ }; use golem_api_grpc::proto::golem::workerexecutor::worker_executor_client::WorkerExecutorClient; use golem_api_grpc::proto::golem::workerexecutor::{ - create_worker_response, get_invocation_key_response, get_worker_metadata_response, - interrupt_worker_response, invoke_and_await_worker_response, invoke_worker_response, - resume_worker_response, ConnectWorkerRequest, CreateWorkerRequest, GetInvocationKeyRequest, - InterruptWorkerRequest, InterruptWorkerResponse, InvokeAndAwaitWorkerRequest, - InvokeWorkerRequest, ResumeWorkerRequest, + create_worker_response, get_invocation_key_response, get_running_worker_metadatas_response, + get_worker_metadata_response, get_worker_metadatas_response, interrupt_worker_response, + invoke_and_await_worker_response, invoke_worker_response, resume_worker_response, + ConnectWorkerRequest, CreateWorkerRequest, GetInvocationKeyRequest, + GetRunningWorkerMetadatasRequest, GetRunningWorkerMetadatasSuccessResponse, + GetWorkerMetadatasRequest, GetWorkerMetadatasSuccessResponse, InterruptWorkerRequest, + InterruptWorkerResponse, InvokeAndAwaitWorkerRequest, InvokeWorkerRequest, ResumeWorkerRequest, }; use golem_common::model::{ AccountId, InvocationKey, TemplateId, VersionedWorkerId, WorkerId, WorkerMetadata, @@ -249,30 +251,9 @@ impl TestWorkerExecutor { match response.result { None => panic!("No response from connect_worker"), - Some(get_worker_metadata_response::Result::Success(metadata)) => Some(WorkerMetadata { - worker_id: VersionedWorkerId { - worker_id: metadata - .worker_id - .expect("no worker_id") - .clone() - .try_into() - .expect("invalid worker_id"), - template_version: metadata.template_version, - }, - args: metadata.args.clone(), - env: metadata - .env - .iter() - .map(|(k, v)| (k.clone(), v.clone())) - .collect::>(), - account_id: metadata.account_id.expect("no account_id").clone().into(), - last_known_status: WorkerStatusRecord { - oplog_idx: 0, - status: metadata.status.try_into().expect("invalid status"), - overridden_retry_config: None, // not passed through gRPC - deleted_regions: DeletedRegions::new(), - }, - }), + Some(get_worker_metadata_response::Result::Success(metadata)) => { + Some(to_worker_metadata(&metadata)) + } Some(get_worker_metadata_response::Result::Failure(WorkerExecutionError { error: Some(worker_execution_error::Error::WorkerNotFound(_)), })) => None, @@ -282,6 +263,72 @@ impl TestWorkerExecutor { } } + pub async fn get_running_worker_metadatas( + &mut self, + template_id: &TemplateId, + ) -> Vec { + let template_id: golem_api_grpc::proto::golem::template::TemplateId = + template_id.clone().into(); + let response = self + .client + .get_running_worker_metadatas(GetRunningWorkerMetadatasRequest { + template_id: Some(template_id), + filter: None, + }) + .await + .expect("Failed to get running worker metadatas") + .into_inner(); + + match response.result { + None => panic!("No response from get_running_worker_metadatas"), + Some(get_running_worker_metadatas_response::Result::Success( + GetRunningWorkerMetadatasSuccessResponse { workers }, + )) => workers.iter().map(to_worker_metadata).collect(), + + Some(get_running_worker_metadatas_response::Result::Failure(error)) => { + panic!("Failed to get worker metadata: {error:?}") + } + } + } + + pub async fn get_worker_metadatas( + &mut self, + template_id: &TemplateId, + ) -> (Option, Vec) { + let template_id: golem_api_grpc::proto::golem::template::TemplateId = + template_id.clone().into(); + let response = self + .client + .get_worker_metadatas(GetWorkerMetadatasRequest { + template_id: Some(template_id), + filter: None, + cursor: 0, + count: 10, + precise: true, + }) + .await + .expect("Failed to get worker metadatas") + .into_inner(); + + match response.result { + None => panic!("No response from get_worker_metadatas"), + Some(get_worker_metadatas_response::Result::Success( + GetWorkerMetadatasSuccessResponse { workers, cursor }, + )) => { + let cursor: Option = if cursor == 0 { + None + } else { + Some(cursor as usize) + }; + + (cursor, workers.iter().map(to_worker_metadata).collect()) + } + Some(get_worker_metadatas_response::Result::Failure(error)) => { + panic!("Failed to get worker metadatas: {error:?}") + } + } + } + pub async fn delete_worker(&mut self, worker_id: &WorkerId) { let worker_id: golem_api_grpc::proto::golem::worker::WorkerId = worker_id.clone().into(); self.client.delete_worker(worker_id).await.unwrap(); @@ -733,6 +780,41 @@ impl TestWorkerExecutor { } } +fn to_worker_metadata( + metadata: &golem_api_grpc::proto::golem::worker::WorkerMetadata, +) -> WorkerMetadata { + WorkerMetadata { + worker_id: VersionedWorkerId { + worker_id: metadata + .worker_id + .clone() + .expect("no worker_id") + .clone() + .try_into() + .expect("invalid worker_id"), + template_version: metadata.template_version, + }, + args: metadata.args.clone(), + env: metadata + .env + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect::>(), + account_id: metadata + .account_id + .clone() + .expect("no account_id") + .clone() + .into(), + last_known_status: WorkerStatusRecord { + oplog_idx: 0, + status: metadata.status.try_into().expect("invalid status"), + overridden_retry_config: None, // not passed through gRPC + deleted_regions: DeletedRegions::new(), + }, + } +} + impl Drop for TestWorkerExecutor { fn drop(&mut self) { if let Some(handle) = &self.handle { From 03223f8717ac9d927047823a5fd70ad3312d39a4 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Wed, 27 Mar 2024 22:42:24 +0100 Subject: [PATCH 17/25] tests with filter, fixes --- golem-common/src/model/mod.rs | 67 +++++++++ golem-common/src/redis.rs | 9 +- .../src/services/worker_enumeration.rs | 6 +- golem-worker-executor-base/tests/api.rs | 127 +++++++++++++++++- .../tests/common/mod.rs | 19 ++- 5 files changed, 212 insertions(+), 16 deletions(-) diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index df9c2a7e8..d7789be2c 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -999,6 +999,73 @@ impl TryFrom for WorkerFilte } } +impl From for golem_api_grpc::proto::golem::worker::WorkerFilter { + fn from(value: WorkerFilter) -> Self { + let filter = match value { + WorkerFilter::Name { comparator, value } => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Name( + golem_api_grpc::proto::golem::worker::WorkerNameFilter { + comparator: comparator.into(), + value, + }, + ) + } + WorkerFilter::Version { comparator, value } => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Version( + golem_api_grpc::proto::golem::worker::WorkerVersionFilter { + comparator: comparator.into(), + value, + }, + ) + } + WorkerFilter::Env { + name, + comparator, + value, + } => golem_api_grpc::proto::golem::worker::worker_filter::Filter::Env( + golem_api_grpc::proto::golem::worker::WorkerEnvFilter { + name, + comparator: comparator.into(), + value, + }, + ), + WorkerFilter::Status { value } => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Status( + golem_api_grpc::proto::golem::worker::WorkerStatusFilter { + value: value.into(), + }, + ) + } + WorkerFilter::Not(filter) => { + let f: golem_api_grpc::proto::golem::worker::WorkerFilter = (*filter).into(); + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Not(Box::new( + golem_api_grpc::proto::golem::worker::WorkerNotFilter { + filter: Some(Box::new(f)), + }, + )) + } + WorkerFilter::And(filters) => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::And( + golem_api_grpc::proto::golem::worker::WorkerAndFilter { + filters: filters.into_iter().map(|f| f.into()).collect(), + }, + ) + } + WorkerFilter::Or(filters) => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::Or( + golem_api_grpc::proto::golem::worker::WorkerOrFilter { + filters: filters.into_iter().map(|f| f.into()).collect(), + }, + ) + } + }; + + golem_api_grpc::proto::golem::worker::WorkerFilter { + filter: Some(filter), + } + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Encode, Decode, Enum)] pub enum StringFilterComparator { Equal, diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index cd81cebbf..c225a2582 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -682,11 +682,18 @@ impl<'a> RedisLabelledApi<'a> { if let Some(Resp3Frame::Array { data, .. }) = data.pop() { let mut keys = Vec::with_capacity(data.len()); + let key_prefix_len = self.key_prefix.len(); + for frame in data.into_iter() { let key: String = frame .try_into() .and_then(|value: RedisValue| value.convert())?; - keys.push(key); + + if key_prefix_len > 0 { + keys.push(key[key_prefix_len..].to_string()); + } else { + keys.push(key); + } } Ok((cursor, keys)) diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 095daa53e..80344b852 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -208,10 +208,8 @@ fn get_worker_id_from_redis_key( template_id: &TemplateId, ) -> Result { let template_prefix = format!("instance:instance:{}:", template_id.0); - - // find because of redis key_prefix - figure out better solution - if let Some(i) = worker_redis_key.find(&template_prefix) { - let worker_name = &worker_redis_key[(i + template_prefix.len())..]; + if worker_redis_key.starts_with(&template_prefix) { + let worker_name = &worker_redis_key[template_prefix.len()..]; Ok(WorkerId { worker_name: worker_name.to_string(), template_id: template_id.clone(), diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index 096b74156..9696ce414 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -14,11 +14,14 @@ use redis::Commands; use golem_api_grpc::proto::golem::worker::{worker_execution_error, LogEvent, TemplateParseFailed}; use golem_api_grpc::proto::golem::workerexecutor::CompletePromiseRequest; -use golem_common::model::{AccountId, InvocationKey, PromiseId, WorkerId, WorkerStatus}; +use golem_common::model::{ + AccountId, InvocationKey, PromiseId, StringFilterComparator, TemplateId, WorkerFilter, + WorkerId, WorkerStatus, +}; use golem_worker_executor_base::error::GolemError; use serde_json::Value; -use crate::common::val_pair; +use crate::common::{val_pair, TestWorkerExecutor}; use tokio::time::sleep; use tonic::transport::Body; use tracing::debug; @@ -636,9 +639,26 @@ async fn delete_instance() { let metadata1 = executor.get_worker_metadata(&worker_id).await; let metadatas1r = executor - .get_running_worker_metadatas(&worker_id.template_id) + .get_running_worker_metadatas( + &worker_id.template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id.worker_name.clone(), + )), + ) + .await; + let (cursor1, metadatas1) = executor + .get_worker_metadatas( + &worker_id.template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id.worker_name.clone(), + )), + 0, + 10, + true, + ) .await; - let (cursor1, metadatas1) = executor.get_worker_metadatas(&worker_id.template_id).await; executor.delete_worker(&worker_id).await; @@ -651,6 +671,105 @@ async fn delete_instance() { check!(metadata2.is_none()); } +#[tokio::test] +#[tracing::instrument] +async fn get_workers() { + let context = common::TestContext::new(); + let mut executor = common::start(&context).await.unwrap(); + + let template_id = executor.store_template(Path::new("../test-templates/option-service.wasm")); + let worker_id1 = executor.start_worker(&template_id, "test-instance-1").await; + + let _ = executor + .invoke_and_await( + &worker_id1, + "golem:it/api/echo", + vec![common::val_option(Some(common::val_string("Hello")))], + ) + .await + .unwrap(); + + let worker_id2 = executor.start_worker(&template_id, "test-instance-2").await; + + let _ = executor + .invoke_and_await( + &worker_id2, + "golem:it/api/echo", + vec![common::val_option(Some(common::val_string("Hello")))], + ) + .await + .unwrap(); + + async fn get_check( + template_id: &TemplateId, + filter: Option, + expected: usize, + executor: &mut TestWorkerExecutor, + ) { + let metadatas1r = executor + .get_running_worker_metadatas(&template_id, filter.clone()) + .await; + + let (cursor1, metadatas1) = executor + .get_worker_metadatas(&template_id, filter, 0, 10, true) + .await; + + check!(metadatas1r.len() == expected); + check!(metadatas1.len() == expected); + check!(cursor1.is_none()); + } + + get_check( + &template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id1.worker_name.clone(), + )), + 1, + &mut executor, + ) + .await; + + get_check( + &template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id2.worker_name.clone(), + )), + 1, + &mut executor, + ) + .await; + + get_check( + &template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Like, + "test".to_string(), + )), + 2, + &mut executor, + ) + .await; + + get_check( + &template_id, + Some(WorkerFilter::new_name(StringFilterComparator::Like, "test".to_string()).not()), + 0, + &mut executor, + ) + .await; + + get_check(&template_id, None, 2, &mut executor).await; + + get_check(&template_id, None, 2, &mut executor).await; + + executor.delete_worker(&worker_id1).await; + executor.delete_worker(&worker_id2).await; + + get_check(&template_id, None, 0, &mut executor).await; +} + #[tokio::test] #[tracing::instrument] async fn error_handling_when_worker_is_invoked_with_fewer_than_expected_parameters() { diff --git a/golem-worker-executor-base/tests/common/mod.rs b/golem-worker-executor-base/tests/common/mod.rs index a28c11e4e..b3d7e8173 100644 --- a/golem-worker-executor-base/tests/common/mod.rs +++ b/golem-worker-executor-base/tests/common/mod.rs @@ -33,8 +33,8 @@ use golem_api_grpc::proto::golem::workerexecutor::{ InterruptWorkerResponse, InvokeAndAwaitWorkerRequest, InvokeWorkerRequest, ResumeWorkerRequest, }; use golem_common::model::{ - AccountId, InvocationKey, TemplateId, VersionedWorkerId, WorkerId, WorkerMetadata, - WorkerStatus, WorkerStatusRecord, + AccountId, InvocationKey, TemplateId, VersionedWorkerId, WorkerFilter, WorkerId, + WorkerMetadata, WorkerStatus, WorkerStatusRecord, }; use golem_worker_executor_base::error::GolemError; use golem_worker_executor_base::services::golem_config::{ @@ -266,6 +266,7 @@ impl TestWorkerExecutor { pub async fn get_running_worker_metadatas( &mut self, template_id: &TemplateId, + filter: Option, ) -> Vec { let template_id: golem_api_grpc::proto::golem::template::TemplateId = template_id.clone().into(); @@ -273,7 +274,7 @@ impl TestWorkerExecutor { .client .get_running_worker_metadatas(GetRunningWorkerMetadatasRequest { template_id: Some(template_id), - filter: None, + filter: filter.map(|f| f.into()), }) .await .expect("Failed to get running worker metadatas") @@ -294,6 +295,10 @@ impl TestWorkerExecutor { pub async fn get_worker_metadatas( &mut self, template_id: &TemplateId, + filter: Option, + cursor: usize, + count: usize, + precise: bool, ) -> (Option, Vec) { let template_id: golem_api_grpc::proto::golem::template::TemplateId = template_id.clone().into(); @@ -301,10 +306,10 @@ impl TestWorkerExecutor { .client .get_worker_metadatas(GetWorkerMetadatasRequest { template_id: Some(template_id), - filter: None, - cursor: 0, - count: 10, - precise: true, + filter: filter.map(|f| f.into()), + cursor: cursor as u64, + count: count as u64, + precise, }) .await .expect("Failed to get worker metadatas") From e1f5d31f2ab8b7b76f4281f315f181a0a0ae2c0f Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Thu, 28 Mar 2024 18:19:45 +0100 Subject: [PATCH 18/25] WorkerEnumeration precise --- .../src/durable_host/mod.rs | 1 - golem-worker-executor-base/src/grpc.rs | 14 +- golem-worker-executor-base/src/lib.rs | 1 + .../src/services/recovery.rs | 47 +++--- .../src/services/worker_enumeration.rs | 62 +++++++- golem-worker-executor-base/src/worker.rs | 14 +- golem-worker-executor-base/tests/api.rs | 138 +++++++++++------- 7 files changed, 166 insertions(+), 111 deletions(-) diff --git a/golem-worker-executor-base/src/durable_host/mod.rs b/golem-worker-executor-base/src/durable_host/mod.rs index 6bfa57a52..6ac05bd02 100644 --- a/golem-worker-executor-base/src/durable_host/mod.rs +++ b/golem-worker-executor-base/src/durable_host/mod.rs @@ -733,7 +733,6 @@ impl InvocationHooks for DurableWorkerCtx { ); Ok(calculate_worker_status( - self.state.recovery_management.clone(), &retry_config, error, previous_tries, diff --git a/golem-worker-executor-base/src/grpc.rs b/golem-worker-executor-base/src/grpc.rs index 1671c33a8..04b82f841 100644 --- a/golem-worker-executor-base/src/grpc.rs +++ b/golem-worker-executor-base/src/grpc.rs @@ -685,11 +685,8 @@ impl + UsesAllDeps + Send + Sync + .ok_or(GolemError::invalid_request("Invalid template id"))?; let filter: Option = match request.filter { - Some(f) => { - let f = f.try_into().map_err(|e| GolemError::invalid_request(e))?; - Some(f) - } - None => None, + Some(f) => Some(f.try_into().map_err(GolemError::invalid_request)?), + _ => None, }; let workers = self @@ -713,11 +710,8 @@ impl + UsesAllDeps + Send + Sync + .ok_or(GolemError::invalid_request("Invalid template id"))?; let filter: Option = match request.filter { - Some(f) => { - let f = f.try_into().map_err(|e| GolemError::invalid_request(e))?; - Some(f) - } - None => None, + Some(f) => Some(f.try_into().map_err(GolemError::invalid_request)?), + _ => None, }; let (new_cursor, workers) = self diff --git a/golem-worker-executor-base/src/lib.rs b/golem-worker-executor-base/src/lib.rs index 8ad2dd027..b10a2f0c1 100644 --- a/golem-worker-executor-base/src/lib.rs +++ b/golem-worker-executor-base/src/lib.rs @@ -170,6 +170,7 @@ pub trait Bootstrap { pool.clone(), worker_service.clone(), oplog_service.clone(), + golem_config.clone(), )); ( diff --git a/golem-worker-executor-base/src/services/recovery.rs b/golem-worker-executor-base/src/services/recovery.rs index 80af9eb2e..84cebb001 100644 --- a/golem-worker-executor-base/src/services/recovery.rs +++ b/golem-worker-executor-base/src/services/recovery.rs @@ -66,13 +66,6 @@ pub trait RecoveryManagement { retry_config: &RetryConfig, last_error: &Option, ) -> RecoveryDecision; - - fn is_retriable( - &self, - retry_config: &RetryConfig, - error: &WorkerError, - retry_count: u64, - ) -> bool; } pub struct RecoveryManagementDefault { @@ -345,7 +338,7 @@ impl RecoveryManagementDefault { TrapType::Interrupt(InterruptKind::Jump) => RecoveryDecision::Immediate, TrapType::Exit => RecoveryDecision::None, TrapType::Error(error) => { - if self.is_retriable(retry_config, error, previous_tries) { + if is_worker_error_retriable(retry_config, error, previous_tries) { match get_delay(retry_config, previous_tries) { Some(delay) => RecoveryDecision::Delayed(delay), None => RecoveryDecision::None, @@ -364,7 +357,11 @@ impl RecoveryManagementDefault { ) -> RecoveryDecision { match last_error { Some(last_error) => { - if self.is_retriable(retry_config, &last_error.error, last_error.retry_count) { + if is_worker_error_retriable( + retry_config, + &last_error.error, + last_error.retry_count, + ) { RecoveryDecision::Immediate } else { RecoveryDecision::None @@ -491,18 +488,6 @@ impl RecoveryManagement for RecoveryManagementDefault { ) .await } - - fn is_retriable( - &self, - retry_config: &RetryConfig, - error: &WorkerError, - retry_count: u64, - ) -> bool { - match error { - WorkerError::Unknown(_) => retry_count < (retry_config.max_attempts as u64), - WorkerError::StackOverflow => false, - } - } } async fn recover_worker(this: &T, worker_id: &VersionedWorkerId) @@ -533,6 +518,17 @@ where } } +pub fn is_worker_error_retriable( + retry_config: &RetryConfig, + error: &WorkerError, + retry_count: u64, +) -> bool { + match error { + WorkerError::Unknown(_) => retry_count < (retry_config.max_attempts as u64), + WorkerError::StackOverflow => false, + } +} + #[cfg(any(feature = "mocks", test))] pub struct RecoveryManagementMock; @@ -572,15 +568,6 @@ impl RecoveryManagement for RecoveryManagementMock { ) -> RecoveryDecision { todo!() } - - fn is_retriable( - &self, - _retry_config: &RetryConfig, - _error: &WorkerError, - _previous_tries: u64, - ) -> bool { - todo!() - } } #[cfg(test)] diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 80344b852..a1e024e4c 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -1,12 +1,16 @@ use crate::error::GolemError; use crate::services::active_workers::ActiveWorkers; -use crate::services::oplog::OplogServiceDefault; +use crate::services::golem_config::GolemConfig; +use crate::services::oplog::{OplogService, OplogServiceDefault}; use crate::services::worker::{WorkerService, WorkerServiceInMemory, WorkerServiceRedis}; +use crate::services::{golem_config, HasConfig, HasOplogService, HasWorkerService}; +use crate::worker::calculate_last_known_status; use crate::workerctx::WorkerCtx; use async_trait::async_trait; use golem_common::model::{TemplateId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus}; use golem_common::redis::RedisPool; use std::sync::Arc; +use tracing::info; #[async_trait] pub trait RunningWorkerEnumerationService { @@ -31,6 +35,12 @@ impl RunningWorkerEnumerationService template_id: &TemplateId, filter: Option, ) -> Result, GolemError> { + info!( + "Get workers for template: {}, filter: {}", + template_id, + filter.is_some() + ); + let active_workers = self.active_workers.enum_workers(); let mut template_workers: Vec = vec![]; @@ -106,6 +116,7 @@ pub struct WorkerEnumerationServiceRedis { redis: RedisPool, worker_service: Arc, oplog_service: Arc, + golem_config: Arc, } impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { @@ -113,11 +124,13 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { redis: RedisPool, worker_service: Arc, oplog_service: Arc, + golem_config: Arc, ) -> Self { Self { redis, worker_service, oplog_service, + golem_config, } } @@ -129,7 +142,6 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { count: usize, precise: bool, ) -> Result<(Option, Vec), GolemError> { - // TODO implement precise let mut new_cursor: Option = None; let mut template_workers: Vec = vec![]; @@ -144,12 +156,26 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { for worker_redis_key in worker_redis_keys { let worker_id = get_worker_id_from_redis_key(&worker_redis_key, template_id)?; - let worker_metadata = self.worker_service.get(&worker_id).await; if let Some(worker_metadata) = worker_metadata { - if filter.clone().map_or(true, |f| f.matches(&worker_metadata)) { - template_workers.push(worker_metadata); + let metadata = if precise { + let last_known_status = calculate_last_known_status( + self, + &worker_id, + &Some(worker_metadata.clone()), + ) + .await?; + WorkerMetadata { + last_known_status, + ..worker_metadata + } + } else { + worker_metadata + }; + + if filter.clone().map_or(true, |f| f.matches(&metadata)) { + template_workers.push(metadata); } } } @@ -162,6 +188,24 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { } } +impl HasOplogService for WorkerEnumerationServiceRedis { + fn oplog_service(&self) -> Arc { + self.oplog_service.clone() + } +} + +impl HasWorkerService for WorkerEnumerationServiceRedis { + fn worker_service(&self) -> Arc { + self.worker_service.clone() + } +} + +impl HasConfig for WorkerEnumerationServiceRedis { + fn config(&self) -> Arc { + self.golem_config.clone() + } +} + #[async_trait] impl WorkerEnumerationService for crate::services::worker_enumeration::WorkerEnumerationServiceRedis @@ -174,6 +218,14 @@ impl WorkerEnumerationService count: usize, precise: bool, ) -> Result<(Option, Vec), GolemError> { + info!( + "Get workers for template: {}, filter: {}, cursor: {}, count: {}, precise: {}", + template_id, + filter.is_some(), + cursor, + count, + precise + ); let mut new_cursor: Option = Some(cursor); let mut template_workers: Vec = vec![]; diff --git a/golem-worker-executor-base/src/worker.rs b/golem-worker-executor-base/src/worker.rs index e18731282..631c29408 100644 --- a/golem-worker-executor-base/src/worker.rs +++ b/golem-worker-executor-base/src/worker.rs @@ -38,11 +38,10 @@ use crate::metrics::wasm::{record_create_worker, record_create_worker_failure}; use crate::model::{ExecutionStatus, InterruptKind, TrapType, WorkerConfig}; use crate::services::golem_config::GolemConfig; use crate::services::invocation_key::LookupResult; -use crate::services::recovery::RecoveryManagement; +use crate::services::recovery::is_worker_error_retriable; use crate::services::worker_event::{WorkerEventService, WorkerEventServiceDefault}; use crate::services::{ - HasAll, HasConfig, HasInvocationKeyService, HasOplogService, HasRecoveryManagement, - HasWorkerService, + HasAll, HasConfig, HasInvocationKeyService, HasOplogService, HasWorkerService, }; use crate::workerctx::{PublicWorkerIo, WorkerCtx}; @@ -666,7 +665,7 @@ pub async fn calculate_last_known_status( metadata: &Option, ) -> Result where - T: HasOplogService + HasWorkerService + HasRecoveryManagement + HasConfig, + T: HasOplogService + HasWorkerService + HasConfig, { let last_known = metadata .as_ref() @@ -691,7 +690,6 @@ where &new_entries, ); let status = calculate_latest_worker_status( - this.recovery_management().clone(), &last_known.status, &this.config().retry, last_known.overridden_retry_config.clone(), @@ -709,7 +707,6 @@ where } fn calculate_latest_worker_status( - recovery_manager: Arc, initial: &WorkerStatus, default_retry_policy: &RetryConfig, initial_retry_policy: Option, @@ -745,7 +742,7 @@ fn calculate_latest_worker_status( OplogEntry::Error { error, .. } => { last_error_count += 1; - if recovery_manager.is_retriable( + if is_worker_error_retriable( current_retry_policy .as_ref() .unwrap_or(default_retry_policy), @@ -795,7 +792,6 @@ fn calculate_deleted_regions(initial: DeletedRegions, entries: &[OplogEntry]) -> } pub fn calculate_worker_status( - recovery_management: Arc, retry_config: &RetryConfig, trap_type: &TrapType, previous_tries: u64, @@ -807,7 +803,7 @@ pub fn calculate_worker_status( TrapType::Interrupt(InterruptKind::Restart) => WorkerStatus::Running, TrapType::Exit => WorkerStatus::Exited, TrapType::Error(error) => { - if recovery_management.is_retriable(retry_config, error, previous_tries) { + if is_worker_error_retriable(retry_config, error, previous_tries) { WorkerStatus::Retrying } else { WorkerStatus::Failed diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index 9696ce414..da9d9adf6 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -15,8 +15,8 @@ use redis::Commands; use golem_api_grpc::proto::golem::worker::{worker_execution_error, LogEvent, TemplateParseFailed}; use golem_api_grpc::proto::golem::workerexecutor::CompletePromiseRequest; use golem_common::model::{ - AccountId, InvocationKey, PromiseId, StringFilterComparator, TemplateId, WorkerFilter, - WorkerId, WorkerStatus, + AccountId, FilterComparator, InvocationKey, PromiseId, StringFilterComparator, TemplateId, + WorkerFilter, WorkerId, WorkerStatus, }; use golem_worker_executor_base::error::GolemError; use serde_json::Value; @@ -674,36 +674,10 @@ async fn delete_instance() { #[tokio::test] #[tracing::instrument] async fn get_workers() { - let context = common::TestContext::new(); - let mut executor = common::start(&context).await.unwrap(); - - let template_id = executor.store_template(Path::new("../test-templates/option-service.wasm")); - let worker_id1 = executor.start_worker(&template_id, "test-instance-1").await; - - let _ = executor - .invoke_and_await( - &worker_id1, - "golem:it/api/echo", - vec![common::val_option(Some(common::val_string("Hello")))], - ) - .await - .unwrap(); - - let worker_id2 = executor.start_worker(&template_id, "test-instance-2").await; - - let _ = executor - .invoke_and_await( - &worker_id2, - "golem:it/api/echo", - vec![common::val_option(Some(common::val_string("Hello")))], - ) - .await - .unwrap(); - async fn get_check( template_id: &TemplateId, filter: Option, - expected: usize, + expected_worker_ids: Vec, executor: &mut TestWorkerExecutor, ) { let metadatas1r = executor @@ -711,43 +685,76 @@ async fn get_workers() { .await; let (cursor1, metadatas1) = executor - .get_worker_metadatas(&template_id, filter, 0, 10, true) + .get_worker_metadatas(&template_id, filter, 0, 20, true) .await; - check!(metadatas1r.len() == expected); - check!(metadatas1.len() == expected); + let expected_count = expected_worker_ids.len(); + + check!(metadatas1r.len() == expected_count); + check!(metadatas1.len() == expected_count); check!(cursor1.is_none()); } - get_check( - &template_id, - Some(WorkerFilter::new_name( - StringFilterComparator::Equal, - worker_id1.worker_name.clone(), - )), - 1, - &mut executor, - ) - .await; + let context = common::TestContext::new(); + let mut executor = common::start(&context).await.unwrap(); + + let template_id = executor.store_template(Path::new("../test-templates/option-service.wasm")); + + let workers_count = 10; + let mut worker_ids = vec![]; + + for i in 0..workers_count { + let worker_id = executor + .start_worker(&template_id, &format!("test-instance-{}", i)) + .await; + + worker_ids.push(worker_id); + } + + for worker_id in worker_ids.clone() { + let _ = executor + .invoke_and_await( + &worker_id, + "golem:it/api/echo", + vec![common::val_option(Some(common::val_string("Hello")))], + ) + .await + .unwrap(); + + get_check( + &template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id.worker_name.clone(), + )), + vec![worker_id], + &mut executor, + ) + .await; + } get_check( &template_id, Some(WorkerFilter::new_name( - StringFilterComparator::Equal, - worker_id2.worker_name.clone(), + StringFilterComparator::Like, + "test".to_string(), )), - 1, + worker_ids.clone(), &mut executor, ) .await; get_check( &template_id, - Some(WorkerFilter::new_name( - StringFilterComparator::Like, - "test".to_string(), - )), - 2, + Some( + WorkerFilter::new_name(StringFilterComparator::Like, "test".to_string()) + .and( + WorkerFilter::new_status(WorkerStatus::Idle) + .or(WorkerFilter::new_status(WorkerStatus::Running)), + ) + .and(WorkerFilter::new_version(FilterComparator::Equal, 0)), + ), + worker_ids.clone(), &mut executor, ) .await; @@ -755,19 +762,38 @@ async fn get_workers() { get_check( &template_id, Some(WorkerFilter::new_name(StringFilterComparator::Like, "test".to_string()).not()), - 0, + vec![], &mut executor, ) .await; - get_check(&template_id, None, 2, &mut executor).await; + get_check(&template_id, None, worker_ids.clone(), &mut executor).await; + + let (cursor1, metadatas1) = executor + .get_worker_metadatas(&template_id, None, 0, workers_count / 2, true) + .await; - get_check(&template_id, None, 2, &mut executor).await; + check!(cursor1.is_some()); + check!(metadatas1.len() == workers_count / 2); - executor.delete_worker(&worker_id1).await; - executor.delete_worker(&worker_id2).await; + let (cursor2, metadatas2) = executor + .get_worker_metadatas( + &template_id, + None, + cursor1.unwrap(), + workers_count - metadatas1.len(), + true, + ) + .await; + + check!(cursor2.is_none()); + check!(metadatas2.len() == workers_count - metadatas1.len()); + + for worker_id in worker_ids { + executor.delete_worker(&worker_id).await; + } - get_check(&template_id, None, 0, &mut executor).await; + get_check(&template_id, None, vec![], &mut executor).await; } #[tokio::test] From 8e27f4ac4cb0cb876bc411c65a17f0c8a429936c Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Thu, 28 Mar 2024 18:50:48 +0100 Subject: [PATCH 19/25] clippy --- golem-worker-executor-base/tests/api.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index da9d9adf6..f99bdb691 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -681,11 +681,11 @@ async fn get_workers() { executor: &mut TestWorkerExecutor, ) { let metadatas1r = executor - .get_running_worker_metadatas(&template_id, filter.clone()) + .get_running_worker_metadatas(template_id, filter.clone()) .await; let (cursor1, metadatas1) = executor - .get_worker_metadatas(&template_id, filter, 0, 20, true) + .get_worker_metadatas(template_id, filter, 0, 20, true) .await; let expected_count = expected_worker_ids.len(); From 8fb68b749317acd61ec1d97d049b2e8c95ef4f72 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Thu, 28 Mar 2024 23:47:33 +0100 Subject: [PATCH 20/25] test fix --- golem-worker-executor-base/tests/api.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index f99bdb691..95e8fb94a 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -786,9 +786,15 @@ async fn get_workers() { ) .await; - check!(cursor2.is_none()); check!(metadatas2.len() == workers_count - metadatas1.len()); + if let Some(cursor2) = cursor2 { + let (_, metadatas3) = executor + .get_worker_metadatas(&template_id, None, cursor2, workers_count, true) + .await; + check!(metadatas3.len() == 0); + } + for worker_id in worker_ids { executor.delete_worker(&worker_id).await; } From 7b7104e2cc87538ad028951bd8473d71ed1b19d7 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 29 Mar 2024 13:42:56 +0100 Subject: [PATCH 21/25] CreatedAt filter, runnig worker filtering --- Cargo.lock | 2 + Cargo.toml | 1 + golem-api-grpc/Cargo.toml | 1 + golem-api-grpc/build.rs | 22 +++- .../proto/golem/worker/worker_filter.proto | 15 ++- golem-common/Cargo.toml | 1 + golem-common/src/model/mod.rs | 117 +++++++++++------- golem-worker-executor-base/src/grpc.rs | 25 +++- .../src/services/worker_enumeration.rs | 3 +- golem-worker-executor-base/tests/api.rs | 42 +++---- 10 files changed, 142 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 69d8a7ff2..191d1c998 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2746,6 +2746,7 @@ dependencies = [ "futures-core", "golem-wasm-rpc", "prost 0.12.3", + "prost-types 0.12.3", "serde", "tokio", "tonic", @@ -2841,6 +2842,7 @@ dependencies = [ "poem-openapi", "prometheus", "prost 0.12.3", + "prost-types 0.12.3", "rand 0.8.5", "range-set-blaze", "serde", diff --git a/Cargo.toml b/Cargo.toml index cb1810b24..dd94b92c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,6 +87,7 @@ poem-openapi = { version = "4.0.0", features = [ prometheus = { version = "0.13.3", features = ["process"] } proptest = "1.4.0" prost = "0.12.3" +prost-types = "0.12.3" rustls = { version = "0.22.2" } serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0" } diff --git a/golem-api-grpc/Cargo.toml b/golem-api-grpc/Cargo.toml index 34bccb0b0..8a6da7885 100644 --- a/golem-api-grpc/Cargo.toml +++ b/golem-api-grpc/Cargo.toml @@ -16,6 +16,7 @@ bytes = { workspace = true } futures-core = { workspace = true } golem-wasm-rpc = { workspace = true } prost = { workspace = true } +prost-types = { workspace = true, features = [] } serde = { workspace = true } tokio = { workspace = true } tonic = { workspace = true } diff --git a/golem-api-grpc/build.rs b/golem-api-grpc/build.rs index 886c66ca5..497403bce 100644 --- a/golem-api-grpc/build.rs +++ b/golem-api-grpc/build.rs @@ -11,7 +11,27 @@ fn main() -> Result<(), Box> { .file_descriptor_set_path(out_dir.join("services.bin")) .extern_path(".wasm.rpc", "::golem_wasm_rpc::protobuf") .type_attribute( - ".", + "golem.worker.LogEvent", + "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", + ) + .type_attribute( + "golem.worker.LogEvent.event", + "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", + ) + .type_attribute( + "golem.worker.StdOutLog", + "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", + ) + .type_attribute( + "golem.worker.StdErrLog", + "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", + ) + .type_attribute( + "golem.worker.Level", + "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", + ) + .type_attribute( + "golem.worker.Log", "#[derive(bincode::Encode, bincode::Decode, serde::Serialize, serde::Deserialize)]", ) .include_file("mod.rs") diff --git a/golem-api-grpc/proto/golem/worker/worker_filter.proto b/golem-api-grpc/proto/golem/worker/worker_filter.proto index dcf06fa82..f562c9a12 100644 --- a/golem-api-grpc/proto/golem/worker/worker_filter.proto +++ b/golem-api-grpc/proto/golem/worker/worker_filter.proto @@ -7,16 +7,18 @@ import "golem/common/string_filter_comparator.proto"; import "golem/common/filter_comparator.proto"; import "golem/worker/worker_id.proto"; import "golem/worker/worker_status.proto"; +import "google/protobuf/timestamp.proto"; message WorkerFilter { oneof filter { WorkerNameFilter name = 1; WorkerVersionFilter version = 2; WorkerStatusFilter status = 3; - WorkerEnvFilter env = 4; - WorkerAndFilter and = 5; - WorkerOrFilter or = 6; - WorkerNotFilter not = 7; + WorkerCreatedAtFilter created_at = 4; + WorkerEnvFilter env = 5; + WorkerAndFilter and = 6; + WorkerOrFilter or = 7; + WorkerNotFilter not = 8; } } @@ -42,6 +44,11 @@ message WorkerVersionFilter { int32 value = 2; } +message WorkerCreatedAtFilter { + golem.common.FilterComparator comparator = 1; + google.protobuf.Timestamp value = 2; +} + message WorkerStatusFilter { WorkerStatus value = 1; } diff --git a/golem-common/Cargo.toml b/golem-common/Cargo.toml index 974d1023e..efb120d33 100644 --- a/golem-common/Cargo.toml +++ b/golem-common/Cargo.toml @@ -28,6 +28,7 @@ poem = { workspace = true } poem-openapi = { workspace = true } prometheus = { workspace = true } prost = { workspace = true } +prost-types = { workspace = true } rand = "0.8.5" range-set-blaze = "0.1.16" serde = { workspace = true } diff --git a/golem-common/src/model/mod.rs b/golem-common/src/model/mod.rs index d7789be2c..ad949aea1 100644 --- a/golem-common/src/model/mod.rs +++ b/golem-common/src/model/mod.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::borrow::Cow; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::fmt::{Display, Formatter}; use std::ops::Add; use std::str::FromStr; @@ -27,7 +27,6 @@ use bincode::enc::Encoder; use bincode::error::{DecodeError, EncodeError}; use bincode::{BorrowDecode, Decode, Encode}; use derive_more::FromStr; -use golem_api_grpc::proto::golem; use poem_openapi::registry::{MetaSchema, MetaSchemaRef}; use poem_openapi::types::{ParseFromJSON, ParseFromParameter, ParseResult, ToJSON}; use poem_openapi::{Enum, Object}; @@ -119,6 +118,27 @@ impl<'de> bincode::BorrowDecode<'de> for Timestamp { } } +impl From for prost_types::Timestamp { + fn from(value: Timestamp) -> Self { + let d = value + .0 + .duration_since(iso8601_timestamp::Timestamp::UNIX_EPOCH); + Self { + seconds: d.whole_seconds(), + nanos: d.subsec_nanoseconds(), + } + } +} + +impl From for Timestamp { + fn from(value: prost_types::Timestamp) -> Self { + Timestamp( + iso8601_timestamp::Timestamp::UNIX_EPOCH + .add(Duration::new(value.seconds as u64, value.nanos as u32)), + ) + } +} + #[derive(Clone, Debug, Eq, PartialEq, Hash, Serialize, Deserialize, Encode, Decode)] pub struct VersionedWorkerId { #[serde(rename = "instance_id")] @@ -521,21 +541,6 @@ impl WorkerMetadata { } } -impl From for golem_api_grpc::proto::golem::worker::WorkerMetadata { - fn from(value: WorkerMetadata) -> Self { - golem_api_grpc::proto::golem::worker::WorkerMetadata { - worker_id: Some(value.worker_id.worker_id.into_proto()), - account_id: Some(value.account_id.into()), - args: value.args, - env: HashMap::from_iter(value.env.iter().cloned()), - template_version: value.worker_id.template_version, - status: Into::::into(value.last_known_status.status) - .into(), - retry_count: 0, // FIXME - } - } -} - /// Contains status information about a worker according to a given oplog index. /// This status is just cached information, all fields must be computable by the oplog alone. /// By having an associated oplog_idx, the cached information can be used together with the @@ -793,31 +798,6 @@ pub fn parse_function_name(name: &str) -> ParsedFunctionName { } } -// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -// pub struct WorkerNameFilter { -// pub comparator: StringFilterComparator, -// pub value: String, -// } -// -// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -// pub struct WorkerStatusFilter { -// pub value: WorkerStatus -// } -// -// -// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -// pub struct WorkerVersionFilter { -// pub comparator: FilterComparator, -// pub value: i32, -// } -// -// #[derive(Clone, Debug, Serialize, Deserialize, Encode, Decode)] -// pub struct WorkerVersionEnvFilter { -// pub name: String, -// pub comparator: StringFilterComparator, -// pub value: String -// } - #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Encode, Decode)] pub enum WorkerFilter { Name { @@ -831,10 +811,10 @@ pub enum WorkerFilter { comparator: FilterComparator, value: i32, }, - // CreatedAt { - // comparator: FilterComparator, - // value: String, - // }, + CreatedAt { + comparator: FilterComparator, + value: Timestamp, + }, Env { name: String, comparator: StringFilterComparator, @@ -878,8 +858,9 @@ impl WorkerFilter { value, } => { let mut result = false; + let name = name.to_lowercase(); for env_value in metadata.env.clone() { - if env_value.0 == name { + if env_value.0.to_lowercase() == name { result = comparator.matches(&env_value.1, &value); break; @@ -887,6 +868,12 @@ impl WorkerFilter { } result } + WorkerFilter::CreatedAt { + comparator: _, + value: _, + } => { + true // TODO implement when we will have timestamp in metadata + } WorkerFilter::Status { value } => metadata.last_known_status.status == value, WorkerFilter::Not(filter) => !filter.matches(metadata), WorkerFilter::And(filters) => { @@ -946,6 +933,10 @@ impl WorkerFilter { pub fn new_status(value: WorkerStatus) -> Self { WorkerFilter::Status { value } } + + pub fn new_created_at(comparator: FilterComparator, value: Timestamp) -> Self { + WorkerFilter::CreatedAt { comparator, value } + } } impl TryFrom for WorkerFilter { @@ -965,6 +956,16 @@ impl TryFrom for WorkerFilte golem_api_grpc::proto::golem::worker::worker_filter::Filter::Status(filter) => { Ok(WorkerFilter::new_status(filter.value.try_into()?)) } + golem_api_grpc::proto::golem::worker::worker_filter::Filter::CreatedAt(filter) => { + let value = filter + .value + .map(|t| t.into()) + .ok_or_else(|| "Missing value".to_string())?; + Ok(WorkerFilter::new_created_at( + filter.comparator.try_into()?, + value, + )) + } golem_api_grpc::proto::golem::worker::worker_filter::Filter::Env(filter) => Ok( WorkerFilter::new_env(filter.name, filter.comparator.try_into()?, filter.value), ), @@ -991,6 +992,7 @@ impl TryFrom for WorkerFilte String, >>( )?; + Ok(WorkerFilter::new_or(filters)) } }, @@ -1036,6 +1038,14 @@ impl From for golem_api_grpc::proto::golem::worker::WorkerFilter { }, ) } + WorkerFilter::CreatedAt { comparator, value } => { + golem_api_grpc::proto::golem::worker::worker_filter::Filter::CreatedAt( + golem_api_grpc::proto::golem::worker::WorkerCreatedAtFilter { + value: Some(value.into()), + comparator: comparator.into(), + }, + ) + } WorkerFilter::Not(filter) => { let f: golem_api_grpc::proto::golem::worker::WorkerFilter = (*filter).into(); golem_api_grpc::proto::golem::worker::worker_filter::Filter::Not(Box::new( @@ -1200,10 +1210,21 @@ mod tests { use crate::model::{ parse_function_name, AccountId, FilterComparator, StringFilterComparator, TemplateId, - VersionedWorkerId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus, + Timestamp, VersionedWorkerId, WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus, WorkerStatusRecord, }; + #[test] + fn timestamp_conversion() { + let ts: Timestamp = Timestamp::now_utc(); + + let prost_ts: prost_types::Timestamp = ts.into(); + + let ts2: Timestamp = prost_ts.into(); + + assert_eq!(ts2, ts); + } + #[test] fn parse_function_name_global() { let parsed = parse_function_name("run-example"); diff --git a/golem-worker-executor-base/src/grpc.rs b/golem-worker-executor-base/src/grpc.rs index 04b82f841..12848dfbf 100644 --- a/golem-worker-executor-base/src/grpc.rs +++ b/golem-worker-executor-base/src/grpc.rs @@ -694,8 +694,10 @@ impl + UsesAllDeps + Send + Sync + .get(&template_id, filter) .await?; - let result: Vec = - workers.iter().map(|worker| worker.clone().into()).collect(); + let result: Vec = workers + .iter() + .map(|worker| self.to_proto_metadata(worker.clone())) + .collect(); Ok(result) } @@ -725,11 +727,26 @@ impl + UsesAllDeps + Send + Sync + ) .await?; - let result: Vec = - workers.iter().map(|worker| worker.clone().into()).collect(); + let result: Vec = workers + .iter() + .map(|worker| self.to_proto_metadata(worker.clone())) + .collect(); Ok((new_cursor, result)) } + + fn to_proto_metadata(&self, value: WorkerMetadata) -> golem::worker::WorkerMetadata { + golem::worker::WorkerMetadata { + worker_id: Some(value.worker_id.worker_id.into_proto()), + account_id: Some(value.account_id.into()), + args: value.args, + env: HashMap::from_iter(value.env.iter().cloned()), + template_version: value.worker_id.template_version, + status: Into::::into(value.last_known_status.status) + .into(), + retry_count: 0, + } + } } impl + UsesAllDeps + Send + Sync + 'static> UsesAllDeps diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index a1e024e4c..4edb84529 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -46,8 +46,7 @@ impl RunningWorkerEnumerationService let mut template_workers: Vec = vec![]; for worker in active_workers { if worker.0.template_id == *template_id - && (worker.1.metadata.last_known_status.status == WorkerStatus::Running - || worker.1.metadata.last_known_status.status == WorkerStatus::Idle) + && worker.1.metadata.last_known_status.status == WorkerStatus::Running && filter .clone() .map_or(true, |f| f.matches(&worker.1.metadata)) diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index 95e8fb94a..1ff97f26f 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -16,7 +16,7 @@ use golem_api_grpc::proto::golem::worker::{worker_execution_error, LogEvent, Tem use golem_api_grpc::proto::golem::workerexecutor::CompletePromiseRequest; use golem_common::model::{ AccountId, FilterComparator, InvocationKey, PromiseId, StringFilterComparator, TemplateId, - WorkerFilter, WorkerId, WorkerStatus, + WorkerFilter, WorkerId, WorkerMetadata, WorkerStatus, }; use golem_worker_executor_base::error::GolemError; use serde_json::Value; @@ -638,15 +638,7 @@ async fn delete_instance() { .unwrap(); let metadata1 = executor.get_worker_metadata(&worker_id).await; - let metadatas1r = executor - .get_running_worker_metadatas( - &worker_id.template_id, - Some(WorkerFilter::new_name( - StringFilterComparator::Equal, - worker_id.worker_name.clone(), - )), - ) - .await; + let (cursor1, metadatas1) = executor .get_worker_metadatas( &worker_id.template_id, @@ -664,7 +656,6 @@ async fn delete_instance() { let metadata2 = executor.get_worker_metadata(&worker_id).await; - check!(metadatas1r.len() == 1); check!(metadatas1.len() == 1); check!(cursor1.is_none()); check!(metadata1.is_some()); @@ -677,22 +668,17 @@ async fn get_workers() { async fn get_check( template_id: &TemplateId, filter: Option, - expected_worker_ids: Vec, + expected_count: usize, executor: &mut TestWorkerExecutor, - ) { - let metadatas1r = executor - .get_running_worker_metadatas(template_id, filter.clone()) - .await; - - let (cursor1, metadatas1) = executor + ) -> Vec { + let (cursor, metadatas) = executor .get_worker_metadatas(template_id, filter, 0, 20, true) .await; - let expected_count = expected_worker_ids.len(); + check!(metadatas.len() == expected_count); + check!(cursor.is_none()); - check!(metadatas1r.len() == expected_count); - check!(metadatas1.len() == expected_count); - check!(cursor1.is_none()); + metadatas } let context = common::TestContext::new(); @@ -727,7 +713,7 @@ async fn get_workers() { StringFilterComparator::Equal, worker_id.worker_name.clone(), )), - vec![worker_id], + 1, &mut executor, ) .await; @@ -739,7 +725,7 @@ async fn get_workers() { StringFilterComparator::Like, "test".to_string(), )), - worker_ids.clone(), + workers_count, &mut executor, ) .await; @@ -754,7 +740,7 @@ async fn get_workers() { ) .and(WorkerFilter::new_version(FilterComparator::Equal, 0)), ), - worker_ids.clone(), + workers_count, &mut executor, ) .await; @@ -762,12 +748,12 @@ async fn get_workers() { get_check( &template_id, Some(WorkerFilter::new_name(StringFilterComparator::Like, "test".to_string()).not()), - vec![], + 0, &mut executor, ) .await; - get_check(&template_id, None, worker_ids.clone(), &mut executor).await; + get_check(&template_id, None, workers_count, &mut executor).await; let (cursor1, metadatas1) = executor .get_worker_metadatas(&template_id, None, 0, workers_count / 2, true) @@ -799,7 +785,7 @@ async fn get_workers() { executor.delete_worker(&worker_id).await; } - get_check(&template_id, None, vec![], &mut executor).await; + get_check(&template_id, None, 0, &mut executor).await; } #[tokio::test] From 4082a1e6b35c4819fa55f7a74b81ac55cc320601 Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 29 Mar 2024 14:46:34 +0100 Subject: [PATCH 22/25] u64 for count and cursor --- golem-common/src/redis.rs | 10 ++--- golem-worker-executor-base/src/grpc.rs | 8 ++-- .../src/services/worker_enumeration.rs | 42 +++++++++---------- golem-worker-executor-base/tests/api.rs | 6 +-- .../tests/common/mod.rs | 16 +++---- 5 files changed, 39 insertions(+), 43 deletions(-) diff --git a/golem-common/src/redis.rs b/golem-common/src/redis.rs index c225a2582..de23ca22c 100644 --- a/golem-common/src/redis.rs +++ b/golem-common/src/redis.rs @@ -639,9 +639,9 @@ impl<'a> RedisLabelledApi<'a> { pub async fn scan( &self, pattern: K, - cursor: usize, - count: usize, - ) -> RedisResult<(usize, Vec)> + cursor: u64, + count: u64, + ) -> RedisResult<(u64, Vec)> where K: AsRef, { @@ -670,11 +670,11 @@ impl<'a> RedisLabelledApi<'a> { ) } - fn parse_key_scan_frame(&self, frame: Resp3Frame) -> RedisResult<(usize, Vec)> { + fn parse_key_scan_frame(&self, frame: Resp3Frame) -> RedisResult<(u64, Vec)> { use fred::prelude::*; if let Resp3Frame::Array { mut data, .. } = frame { if data.len() == 2 { - let cursor: usize = data[0] + let cursor: u64 = data[0] .clone() .try_into() .and_then(|value: RedisValue| value.convert())?; diff --git a/golem-worker-executor-base/src/grpc.rs b/golem-worker-executor-base/src/grpc.rs index 12848dfbf..5ae69e07a 100644 --- a/golem-worker-executor-base/src/grpc.rs +++ b/golem-worker-executor-base/src/grpc.rs @@ -705,7 +705,7 @@ impl + UsesAllDeps + Send + Sync + async fn get_worker_metadatas_internal( &self, request: golem::workerexecutor::GetWorkerMetadatasRequest, - ) -> Result<(Option, Vec), GolemError> { + ) -> Result<(Option, Vec), GolemError> { let template_id: common_model::TemplateId = request .template_id .and_then(|t| t.try_into().ok()) @@ -721,8 +721,8 @@ impl + UsesAllDeps + Send + Sync + .get( &template_id, filter, - request.cursor as usize, - request.count as usize, + request.cursor, + request.count, request.precise, ) .await?; @@ -1342,7 +1342,7 @@ impl + UsesAllDeps + Send + Sync + golem::workerexecutor::get_worker_metadatas_response::Result::Success( golem::workerexecutor::GetWorkerMetadatasSuccessResponse { workers, - cursor: cursor.unwrap_or(0) as u64, // FIXME can we do option in proto? + cursor: cursor.unwrap_or(0), // FIXME can we do option in proto? }, ), ), diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 4edb84529..66a80a657 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -104,10 +104,10 @@ pub trait WorkerEnumerationService { &self, template_id: &TemplateId, filter: Option, - cursor: usize, - count: usize, + cursor: u64, + count: u64, precise: bool, - ) -> Result<(Option, Vec), GolemError>; + ) -> Result<(Option, Vec), GolemError>; } #[derive(Clone)] @@ -137,11 +137,11 @@ impl crate::services::worker_enumeration::WorkerEnumerationServiceRedis { &self, template_id: &TemplateId, filter: Option, - cursor: usize, - count: usize, + cursor: u64, + count: u64, precise: bool, - ) -> Result<(Option, Vec), GolemError> { - let mut new_cursor: Option = None; + ) -> Result<(Option, Vec), GolemError> { + let mut new_cursor: Option = None; let mut template_workers: Vec = vec![]; let template_worker_redis_key = get_template_worker_redis_key(template_id); @@ -213,10 +213,10 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: Option, - cursor: usize, - count: usize, + cursor: u64, + count: u64, precise: bool, - ) -> Result<(Option, Vec), GolemError> { + ) -> Result<(Option, Vec), GolemError> { info!( "Get workers for template: {}, filter: {}, cursor: {}, count: {}, precise: {}", template_id, @@ -225,11 +225,11 @@ impl WorkerEnumerationService count, precise ); - let mut new_cursor: Option = Some(cursor); + let mut new_cursor: Option = Some(cursor); let mut template_workers: Vec = vec![]; - while new_cursor.is_some() && template_workers.len() < count { - let new_count = count - template_workers.len(); + while new_cursor.is_some() && (template_workers.len() as u64) < count { + let new_count = count - (template_workers.len() as u64); let (next_cursor, workers) = self .get_internal( @@ -291,13 +291,13 @@ impl WorkerEnumerationService &self, template_id: &TemplateId, filter: Option, - cursor: usize, - count: usize, + cursor: u64, + count: u64, _precise: bool, - ) -> Result<(Option, Vec), GolemError> { + ) -> Result<(Option, Vec), GolemError> { let workers = self.worker_service.enumerate().await; - let all_workers_count = workers.len(); + let all_workers_count = workers.len() as u64; if all_workers_count > cursor { let mut template_workers: Vec = vec![]; @@ -312,7 +312,7 @@ impl WorkerEnumerationService index += 1; - if template_workers.len() == count { + if (template_workers.len() as u64) == count { break; } } @@ -353,10 +353,10 @@ impl WorkerEnumerationService &self, _template_id: &TemplateId, _filter: Option, - _cursor: usize, - _count: usize, + _cursor: u64, + _count: u64, _precise: bool, - ) -> Result<(Option, Vec), GolemError> { + ) -> Result<(Option, Vec), GolemError> { unimplemented!() } } diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index 1ff97f26f..d87201b99 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -756,7 +756,7 @@ async fn get_workers() { get_check(&template_id, None, workers_count, &mut executor).await; let (cursor1, metadatas1) = executor - .get_worker_metadatas(&template_id, None, 0, workers_count / 2, true) + .get_worker_metadatas(&template_id, None, 0, (workers_count / 2) as u64, true) .await; check!(cursor1.is_some()); @@ -767,7 +767,7 @@ async fn get_workers() { &template_id, None, cursor1.unwrap(), - workers_count - metadatas1.len(), + (workers_count - metadatas1.len()) as u64, true, ) .await; @@ -776,7 +776,7 @@ async fn get_workers() { if let Some(cursor2) = cursor2 { let (_, metadatas3) = executor - .get_worker_metadatas(&template_id, None, cursor2, workers_count, true) + .get_worker_metadatas(&template_id, None, cursor2, workers_count as u64, true) .await; check!(metadatas3.len() == 0); } diff --git a/golem-worker-executor-base/tests/common/mod.rs b/golem-worker-executor-base/tests/common/mod.rs index b3d7e8173..4f6d9afa8 100644 --- a/golem-worker-executor-base/tests/common/mod.rs +++ b/golem-worker-executor-base/tests/common/mod.rs @@ -296,10 +296,10 @@ impl TestWorkerExecutor { &mut self, template_id: &TemplateId, filter: Option, - cursor: usize, - count: usize, + cursor: u64, + count: u64, precise: bool, - ) -> (Option, Vec) { + ) -> (Option, Vec) { let template_id: golem_api_grpc::proto::golem::template::TemplateId = template_id.clone().into(); let response = self @@ -307,8 +307,8 @@ impl TestWorkerExecutor { .get_worker_metadatas(GetWorkerMetadatasRequest { template_id: Some(template_id), filter: filter.map(|f| f.into()), - cursor: cursor as u64, - count: count as u64, + cursor, + count, precise, }) .await @@ -320,11 +320,7 @@ impl TestWorkerExecutor { Some(get_worker_metadatas_response::Result::Success( GetWorkerMetadatasSuccessResponse { workers, cursor }, )) => { - let cursor: Option = if cursor == 0 { - None - } else { - Some(cursor as usize) - }; + let cursor: Option = if cursor == 0 { None } else { Some(cursor) }; (cursor, workers.iter().map(to_worker_metadata).collect()) } From 83c5170b5fbde3968dd6199106b5868a133f8bf2 Mon Sep 17 00:00:00 2001 From: Daniel Vigovszky Date: Fri, 29 Mar 2024 19:07:09 +0100 Subject: [PATCH 23/25] Expose last known worker status through public worker state --- .../src/durable_host/mod.rs | 30 +++++++++---- golem-worker-executor-base/src/model.rs | 42 +++++++++++++++++-- .../src/services/worker_enumeration.rs | 5 ++- golem-worker-executor-base/src/worker.rs | 22 ++++++++-- 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/golem-worker-executor-base/src/durable_host/mod.rs b/golem-worker-executor-base/src/durable_host/mod.rs index 6ac05bd02..85a0c2b01 100644 --- a/golem-worker-executor-base/src/durable_host/mod.rs +++ b/golem-worker-executor-base/src/durable_host/mod.rs @@ -246,7 +246,7 @@ impl DurableWorkerCtx { let execution_status = self.execution_status.read().unwrap().clone(); match execution_status { ExecutionStatus::Interrupting { interrupt_kind, .. } => Some(interrupt_kind), - ExecutionStatus::Interrupted { interrupt_kind } => Some(interrupt_kind), + ExecutionStatus::Interrupted { interrupt_kind, .. } => Some(interrupt_kind), _ => None, } } @@ -255,15 +255,19 @@ impl DurableWorkerCtx { let mut execution_status = self.execution_status.write().unwrap(); let current_execution_status = execution_status.clone(); match current_execution_status { - ExecutionStatus::Running => { - *execution_status = ExecutionStatus::Suspended; + ExecutionStatus::Running { last_known_status } => { + *execution_status = ExecutionStatus::Suspended { last_known_status }; } - ExecutionStatus::Suspended => {} + ExecutionStatus::Suspended { .. } => {} ExecutionStatus::Interrupting { interrupt_kind, await_interruption, + last_known_status, } => { - *execution_status = ExecutionStatus::Interrupted { interrupt_kind }; + *execution_status = ExecutionStatus::Interrupted { + interrupt_kind, + last_known_status, + }; await_interruption.send(()).ok(); } ExecutionStatus::Interrupted { .. } => {} @@ -274,9 +278,9 @@ impl DurableWorkerCtx { let mut execution_status = self.execution_status.write().unwrap(); let current_execution_status = execution_status.clone(); match current_execution_status { - ExecutionStatus::Running => {} - ExecutionStatus::Suspended => { - *execution_status = ExecutionStatus::Running; + ExecutionStatus::Running { .. } => {} + ExecutionStatus::Suspended { last_known_status } => { + *execution_status = ExecutionStatus::Running { last_known_status }; } ExecutionStatus::Interrupting { .. } => {} ExecutionStatus::Interrupted { .. } => {} @@ -312,7 +316,15 @@ impl DurableWorkerCtx { self.state.overridden_retry_policy.clone(), oplog_idx, ) - .await + .await; + + let mut execution_status = self.execution_status.write().unwrap(); + execution_status.set_last_known_status(WorkerStatusRecord { + status, + deleted_regions: self.private_state.deleted_regions.clone(), + overridden_retry_config: self.private_state.overridden_retry_policy.clone(), + oplog_idx, + }); } pub fn get_stdio(&self) -> ManagedStandardIo { diff --git a/golem-worker-executor-base/src/model.rs b/golem-worker-executor-base/src/model.rs index 1ea170128..9c3991682 100644 --- a/golem-worker-executor-base/src/model.rs +++ b/golem-worker-executor-base/src/model.rs @@ -19,7 +19,9 @@ use std::sync::Arc; use golem_common::model::oplog::WorkerError; use golem_common::model::regions::DeletedRegions; -use golem_common::model::{ShardAssignment, ShardId, VersionedWorkerId, WorkerId}; +use golem_common::model::{ + ShardAssignment, ShardId, VersionedWorkerId, WorkerId, WorkerStatusRecord, +}; use serde::{Deserialize, Serialize}; use wasmtime::Trap; @@ -117,20 +119,52 @@ impl From for CurrentResou #[derive(Clone, Debug)] pub enum ExecutionStatus { - Running, - Suspended, + Running { + last_known_status: WorkerStatusRecord, + }, + Suspended { + last_known_status: WorkerStatusRecord, + }, Interrupting { interrupt_kind: InterruptKind, await_interruption: Arc>, + last_known_status: WorkerStatusRecord, }, Interrupted { interrupt_kind: InterruptKind, + last_known_status: WorkerStatusRecord, }, } impl ExecutionStatus { pub fn is_running(&self) -> bool { - matches!(self, ExecutionStatus::Running) + matches!(self, ExecutionStatus::Running { .. }) + } + + pub fn last_known_status(&self) -> &WorkerStatusRecord { + match self { + ExecutionStatus::Running { last_known_status } => last_known_status, + ExecutionStatus::Suspended { last_known_status } => last_known_status, + ExecutionStatus::Interrupting { + last_known_status, .. + } => last_known_status, + ExecutionStatus::Interrupted { + last_known_status, .. + } => last_known_status, + } + } + + pub fn set_last_known_status(&mut self, status: WorkerStatusRecord) { + match self { + ExecutionStatus::Running { last_known_status } => *last_known_status = status, + ExecutionStatus::Suspended { last_known_status } => *last_known_status = status, + ExecutionStatus::Interrupting { + last_known_status, .. + } => *last_known_status = status, + ExecutionStatus::Interrupted { + last_known_status, .. + } => *last_known_status = status, + } } } diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index 66a80a657..adb40c1db 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -46,12 +46,13 @@ impl RunningWorkerEnumerationService let mut template_workers: Vec = vec![]; for worker in active_workers { if worker.0.template_id == *template_id - && worker.1.metadata.last_known_status.status == WorkerStatus::Running + && (worker.1.metadata.last_known_status.status == WorkerStatus::Running + || worker.1.metadata.last_known_status.status == WorkerStatus::Idle) && filter .clone() .map_or(true, |f| f.matches(&worker.1.metadata)) { - template_workers.push(worker.1.metadata.clone()); + template_workers.push(metadata); } } diff --git a/golem-worker-executor-base/src/worker.rs b/golem-worker-executor-base/src/worker.rs index 631c29408..0ddf970dc 100644 --- a/golem-worker-executor-base/src/worker.rs +++ b/golem-worker-executor-base/src/worker.rs @@ -131,7 +131,9 @@ impl Worker { this.worker_service().add(&worker_metadata).await?; - let execution_status = Arc::new(RwLock::new(ExecutionStatus::Suspended)); + let execution_status = Arc::new(RwLock::new(ExecutionStatus::Suspended { + last_known_status: worker_metadata.last_known_status.clone(), + })); let context = Ctx::create( worker_metadata.worker_id.clone(), @@ -405,16 +407,17 @@ impl Worker { let mut execution_status = self.execution_status.write().unwrap(); let current_execution_status = execution_status.clone(); match current_execution_status { - ExecutionStatus::Running => { + ExecutionStatus::Running { last_known_status }=> { let (sender, receiver) = tokio::sync::broadcast::channel(1); *execution_status = ExecutionStatus::Interrupting { interrupt_kind, await_interruption: Arc::new(sender), + last_known_status }; Some(receiver) } - ExecutionStatus::Suspended => { - *execution_status = ExecutionStatus::Interrupted { interrupt_kind }; + ExecutionStatus::Suspended { last_known_status }=> { + *execution_status = ExecutionStatus::Interrupted { interrupt_kind, last_known_status }; None } ExecutionStatus::Interrupting { @@ -426,6 +429,17 @@ impl Worker { ExecutionStatus::Interrupted { .. } => None, } } + + pub fn get_metadata(&self) -> WorkerMetadata { + let mut result = self.metadata.clone(); + result.last_known_status = self + .execution_status + .read() + .unwrap() + .last_known_status() + .clone(); + result + } } impl Drop for Worker { From 53ef7ec9ae2a426948b0e58de18adc036bbf14ae Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 29 Mar 2024 19:21:01 +0100 Subject: [PATCH 24/25] get running workers fix and test --- golem-api-grpc/Cargo.toml | 2 +- .../src/services/worker_enumeration.rs | 12 +++++------ golem-worker-executor-base/src/worker.rs | 11 ++++++---- golem-worker-executor-base/tests/api.rs | 20 +++++++++++++++++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/golem-api-grpc/Cargo.toml b/golem-api-grpc/Cargo.toml index 8a6da7885..0de942213 100644 --- a/golem-api-grpc/Cargo.toml +++ b/golem-api-grpc/Cargo.toml @@ -16,7 +16,7 @@ bytes = { workspace = true } futures-core = { workspace = true } golem-wasm-rpc = { workspace = true } prost = { workspace = true } -prost-types = { workspace = true, features = [] } +prost-types = { workspace = true } serde = { workspace = true } tokio = { workspace = true } tonic = { workspace = true } diff --git a/golem-worker-executor-base/src/services/worker_enumeration.rs b/golem-worker-executor-base/src/services/worker_enumeration.rs index adb40c1db..24e418cf3 100644 --- a/golem-worker-executor-base/src/services/worker_enumeration.rs +++ b/golem-worker-executor-base/src/services/worker_enumeration.rs @@ -44,13 +44,11 @@ impl RunningWorkerEnumerationService let active_workers = self.active_workers.enum_workers(); let mut template_workers: Vec = vec![]; - for worker in active_workers { - if worker.0.template_id == *template_id - && (worker.1.metadata.last_known_status.status == WorkerStatus::Running - || worker.1.metadata.last_known_status.status == WorkerStatus::Idle) - && filter - .clone() - .map_or(true, |f| f.matches(&worker.1.metadata)) + for (worker_id, worker) in active_workers { + let metadata = worker.get_metadata(); + if worker_id.template_id == *template_id + && (metadata.last_known_status.status == WorkerStatus::Running) + && filter.clone().map_or(true, |f| f.matches(&metadata)) { template_workers.push(metadata); } diff --git a/golem-worker-executor-base/src/worker.rs b/golem-worker-executor-base/src/worker.rs index 0ddf970dc..2b3f59f9e 100644 --- a/golem-worker-executor-base/src/worker.rs +++ b/golem-worker-executor-base/src/worker.rs @@ -407,17 +407,20 @@ impl Worker { let mut execution_status = self.execution_status.write().unwrap(); let current_execution_status = execution_status.clone(); match current_execution_status { - ExecutionStatus::Running { last_known_status }=> { + ExecutionStatus::Running { last_known_status } => { let (sender, receiver) = tokio::sync::broadcast::channel(1); *execution_status = ExecutionStatus::Interrupting { interrupt_kind, await_interruption: Arc::new(sender), - last_known_status + last_known_status, }; Some(receiver) } - ExecutionStatus::Suspended { last_known_status }=> { - *execution_status = ExecutionStatus::Interrupted { interrupt_kind, last_known_status }; + ExecutionStatus::Suspended { last_known_status } => { + *execution_status = ExecutionStatus::Interrupted { + interrupt_kind, + last_known_status, + }; None } ExecutionStatus::Interrupting { diff --git a/golem-worker-executor-base/tests/api.rs b/golem-worker-executor-base/tests/api.rs index d87201b99..8234f33b6 100644 --- a/golem-worker-executor-base/tests/api.rs +++ b/golem-worker-executor-base/tests/api.rs @@ -1298,12 +1298,30 @@ async fn long_running_poll_loop_interrupting_and_resuming_by_second_invocation() sleep(Duration::from_secs(2)).await; let status1 = executor.get_worker_metadata(&worker_id).await.unwrap(); + let metadatas1 = executor + .get_running_worker_metadatas( + &worker_id.template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id.worker_name.clone(), + )), + ) + .await; sleep(Duration::from_secs(4)).await; executor.interrupt(&worker_id).await; sleep(Duration::from_secs(2)).await; let status2 = executor.get_worker_metadata(&worker_id).await.unwrap(); + let metadatas2 = executor + .get_running_worker_metadatas( + &worker_id.template_id, + Some(WorkerFilter::new_name( + StringFilterComparator::Equal, + worker_id.worker_name.clone(), + )), + ) + .await; let mut executor_clone = executor.async_clone().await; let worker_id_clone = worker_id.clone(); @@ -1344,8 +1362,10 @@ async fn long_running_poll_loop_interrupting_and_resuming_by_second_invocation() http_server.abort(); check!(status1.last_known_status.status == WorkerStatus::Running); + check!(!metadatas1.is_empty()); // first running check!(status2.last_known_status.status == WorkerStatus::Interrupted); + check!(metadatas2.is_empty()); // first interrupted check!(status3.last_known_status.status == WorkerStatus::Running); // first resumed From 8dc06701f1fae465e364bf9d69e0601dcf7fa35a Mon Sep 17 00:00:00 2001 From: Peter Kotula Date: Fri, 29 Mar 2024 20:05:02 +0100 Subject: [PATCH 25/25] merge fixes --- golem-worker-executor-base/src/durable_host/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/golem-worker-executor-base/src/durable_host/mod.rs b/golem-worker-executor-base/src/durable_host/mod.rs index 85a0c2b01..289366d97 100644 --- a/golem-worker-executor-base/src/durable_host/mod.rs +++ b/golem-worker-executor-base/src/durable_host/mod.rs @@ -311,7 +311,7 @@ impl DurableWorkerCtx { .worker_service .update_status( &self.worker_id.worker_id, - status, + status.clone(), self.state.deleted_regions.clone(), self.state.overridden_retry_policy.clone(), oplog_idx, @@ -321,8 +321,8 @@ impl DurableWorkerCtx { let mut execution_status = self.execution_status.write().unwrap(); execution_status.set_last_known_status(WorkerStatusRecord { status, - deleted_regions: self.private_state.deleted_regions.clone(), - overridden_retry_config: self.private_state.overridden_retry_policy.clone(), + deleted_regions: self.state.deleted_regions.clone(), + overridden_retry_config: self.state.overridden_retry_policy.clone(), oplog_idx, }); }