-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(conditional_configs): refactor conditional_configs to use Moka Cache instead of Static Cache #4814
Conversation
…he-decision-manager
…perswitch into cache-decision-manager
let value_to_cache = || async { | ||
VirInterpreterBackendCacheWrapper::try_from(record) | ||
.change_context(errors::StorageError::ValueNotFound("Program".to_string())) | ||
.attach_printable("Error initializing DSL interpreter backend") | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move the DB call to fetch the config (and thus parsing the record) within the closure, to avoid unnecessary DB calls.
let find_key_from_db = || async { | ||
backend::VirInterpreterBackend::with_program(rec.program) | ||
.change_context(ConfigError::DslBackendInitError) | ||
.attach_printable("Error initializing DSL interpreter backend")?; | ||
CONF_CACHE | ||
.save(key, interpreter, timestamp) | ||
.change_context(ConfigError::DslCachePoisoned) | ||
.attach_printable("Error saving DSL to cache")?; | ||
Ok(()) | ||
.change_context(errors::StorageError::ValueNotFound("Program".to_string())) | ||
.attach_printable("Error initializing DSL interpreter backend") | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar change can be done here.
@@ -517,7 +517,7 @@ async fn publish_and_redact_merchant_account_cache( | |||
let kgraph_key = merchant_account.default_profile.as_ref().map(|profile_id| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let kgraph_key = merchant_account.default_profile.as_ref().map(|profile_id| { | |
let cgraph_key = merchant_account.default_profile.as_ref().map(|profile_id| { |
@@ -527,7 +527,7 @@ async fn publish_and_redact_merchant_account_cache( | |||
|
|||
#[cfg(not(feature = "business_profile_routing"))] | |||
let kgraph_key = Some(CacheKind::CGraph( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let kgraph_key = Some(CacheKind::CGraph( | |
let cgraph_key = Some(CacheKind::CGraph( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add cache invalidation logic. When the 3ds decision manager config is updated or deleted, we should invalidate the cache of of all pods. Same for surcharge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surcharge Decision Configs Related changes LGTM
1604e92
…he-decision-manager
…perswitch into cache-decision-manager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type of Change
Description
refactor conditional_configs to use Moka Cache instead of Static Cache
Additional Changes
Motivation and Context
How did you test it?
Cannot be tested
Checklist
cargo +nightly fmt --all
cargo clippy