Skip to content

Revert "fix: Adding lock to init relayer instances" #692

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.

Sign up for GitHub

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

Merged
collins-w merged 1 commit into from
Mar 13, 2026
Merged

Revert "fix: Adding lock to init relayer instances" #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
424 changes: 17 additions & 407 deletions src/bootstrap/config_processor.rs
View file
Open in desktop

Large diffs are not rendered by default.

922 changes: 13 additions & 909 deletions src/bootstrap/initialize_relayers.rs
View file
Open in desktop

Large diffs are not rendered by default.

32 changes: 16 additions & 16 deletions src/repositories/api_key/api_key_redis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,54 +330,54 @@ impl ApiKeyRepositoryTrait for RedisApiKeyRepository {
.await?;
let plugin_list_key = self.api_key_list_key();

debug!("Checking if API key entries exist");
debug!("Checking if plugin entries exist");

let exists: bool = conn
.exists(&plugin_list_key)
.await
.map_err(|e| self.map_redis_error(e, "has_entries_check"))?;

debug!("API key entries exist: {}", exists);
debug!("Plugin entries exist: {}", exists);
Comment on lines +333 to +340

@coderabbitai coderabbitai Bot Mar 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Restore API-key terminology in this repository's logging.

These branches still read and delete API key keys, but the new messages/comments now say "plugin". That will mislead storage-reset and bootstrap debugging inside RedisApiKeyRepository.

Also applies to: 350-380

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/repositories/api_key/api_key_redis.rs` around lines 333 - 340, Logging in
RedisApiKeyRepository incorrectly refers to "plugin" instead of "API key";
update log messages and any comments near the shown block (and similarly in the
subsequent blocks around lines 350-380) to use API-key terminology.
Specifically, change the debug strings around the check using plugin_list_key
(the conn.exists call and its surrounding debug!("Checking if plugin entries
exist") / debug!("Plugin entries exist: {}")) to something like "Checking if API
key entries exist" and "API key entries exist: {}", and apply the same rename to
any deletes/reads in the methods that call self.map_redis_error(...) (preserve
the existing error mapping call and variable names like plugin_list_key but
ensure logs/comments say API key).

Ok(exists)
}

async fn drop_all_entries(&self) -> Result<(), RepositoryError> {
let mut conn = self
.get_connection(self.connections.primary(), "drop_all_entries")
.await?;
let api_key_list_key = self.api_key_list_key();
let plugin_list_key = self.api_key_list_key();

debug!("Dropping all API key entries");
debug!("Dropping all plugin entries");

// Get all API key IDs first
let api_key_ids: Vec<String> = conn
.smembers(&api_key_list_key)
// Get all plugin IDs first
let plugin_ids: Vec<String> = conn
.smembers(&plugin_list_key)
.await
.map_err(|e| self.map_redis_error(e, "drop_all_entries_get_ids"))?;

if api_key_ids.is_empty() {
debug!("No API key entries to drop");
if plugin_ids.is_empty() {
debug!("No plugin entries to drop");
return Ok(());
}

// Use pipeline for atomic operations
let mut pipe = redis::pipe();
pipe.atomic();

// Delete all individual API key entries
for api_key_id in &api_key_ids {
let api_key_key = self.api_key_key(api_key_id);
pipe.del(&api_key_key);
// Delete all individual plugin entries
for plugin_id in &plugin_ids {
let plugin_key = self.api_key_key(plugin_id);
pipe.del(&plugin_key);
}

// Delete the API key list key
pipe.del(&api_key_list_key);
// Delete the plugin list key
pipe.del(&plugin_list_key);

pipe.exec_async(&mut conn)
.await
.map_err(|e| self.map_redis_error(e, "drop_all_entries_pipeline"))?;

debug!("Dropped {} API key entries", api_key_ids.len());
debug!("Dropped {} plugin entries", plugin_ids.len());
Ok(())
}
}
Expand Down
76 changes: 0 additions & 76 deletions src/repositories/relayer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use crate::{
utils::RedisConnections,
};
use async_trait::async_trait;
use deadpool_redis::Pool;
use std::sync::Arc;

#[async_trait]
Expand Down Expand Up @@ -68,19 +67,6 @@ pub trait RelayerRepository: Repository<RelayerRepoModel, String> + Send + Sync
/// Returns true if this repository uses persistent storage (e.g., Redis).
/// Returns false for in-memory storage.
fn is_persistent_storage(&self) -> bool;

/// Returns connection info for distributed operations.
///
/// This method provides access to the underlying connection and key prefix
/// when using persistent storage. This is useful for distributed locking and
/// other coordination operations that need direct storage access.
///
/// # Returns
/// * `Some((pool, prefix))` - If using persistent storage (e.g., Redis)
/// * `None` - If using in-memory storage (default)
fn connection_info(&self) -> Option<(Arc<Pool>, String)> {
None
}
}

#[cfg(test)]
Expand Down Expand Up @@ -110,7 +96,6 @@ mockall::mock! {
async fn disable_relayer(&self, relayer_id: String, reason: DisabledReason) -> Result<RelayerRepoModel, RepositoryError>;
async fn update_policy(&self, id: String, policy: RelayerNetworkPolicy) -> Result<RelayerRepoModel, RepositoryError>;
fn is_persistent_storage(&self) -> bool;
fn connection_info(&self) -> Option<(Arc<Pool>, String)>;
}
}

Expand All @@ -135,24 +120,6 @@ impl RelayerRepositoryStorage {
key_prefix,
)?))
}

/// Returns connection info for distributed operations.
///
/// This method provides access to the underlying Redis connection and key prefix
/// when using Redis-backed storage. This is useful for distributed locking and
/// other coordination operations that need direct Redis access.
///
/// # Returns
/// * `Some((pool, prefix))` - If using persistent storage (e.g., Redis)
/// * `None` - If using in-memory storage
pub fn connection_info(&self) -> Option<(Arc<Pool>, String)> {
match self {
RelayerRepositoryStorage::InMemory(_) => None,
RelayerRepositoryStorage::Redis(repo) => {
Some((repo.connections.primary().clone(), repo.key_prefix.clone()))
}
}
}
}

impl Default for RelayerRepositoryStorage {
Expand Down Expand Up @@ -318,15 +285,6 @@ impl RelayerRepository for RelayerRepositoryStorage {
RelayerRepositoryStorage::Redis(_) => true,
}
}

fn connection_info(&self) -> Option<(Arc<Pool>, String)> {
match self {
RelayerRepositoryStorage::InMemory(_) => None,
RelayerRepositoryStorage::Redis(repo) => {
Some((repo.connections.primary().clone(), repo.key_prefix.clone()))
}
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -541,38 +499,4 @@ mod tests {
repo.drop_all_entries().await.unwrap();
assert!(!repo.has_entries().await.unwrap());
}

#[tokio::test]
async fn test_connection_info_returns_none_for_in_memory() {
let storage = RelayerRepositoryStorage::new_in_memory();

// In-memory storage should return None for connection_info
assert!(storage.connection_info().is_none());
}

#[tokio::test]
async fn test_is_persistent_storage_returns_false_for_in_memory() {
let storage = RelayerRepositoryStorage::new_in_memory();

// In-memory storage should return false for is_persistent_storage
assert!(!storage.is_persistent_storage());
}

#[tokio::test]
async fn test_trait_connection_info_returns_none_for_in_memory() {
let storage = RelayerRepositoryStorage::new_in_memory();

// Test the RelayerRepository trait's connection_info method
let trait_ref: &dyn RelayerRepository = &storage;
assert!(trait_ref.connection_info().is_none());
}

#[tokio::test]
async fn test_struct_connection_info_returns_none_for_in_memory() {
let storage = RelayerRepositoryStorage::new_in_memory();

// Test the struct's own connection_info method
let result: Option<(Arc<Pool>, String)> = storage.connection_info();
assert!(result.is_none());
}
}
2 changes: 0 additions & 2 deletions src/repositories/relayer/relayer_in_memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ impl RelayerRepository for InMemoryRelayerRepository {
fn is_persistent_storage(&self) -> bool {
false
}

// Uses the trait default implementation which returns None
}

#[async_trait]
Expand Down
4 changes: 0 additions & 4 deletions src/repositories/relayer/relayer_redis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,10 +560,6 @@ impl RelayerRepository for RedisRelayerRepository {
fn is_persistent_storage(&self) -> bool {
true
}

fn connection_info(&self) -> Option<(Arc<deadpool_redis::Pool>, String)> {
Some((self.connections.primary().clone(), self.key_prefix.clone()))
}
}

#[cfg(test)]
Expand Down
3 changes: 0 additions & 3 deletions src/utils/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ pub use key::*;
mod auth;
pub use auth::*;

mod polling;
pub use polling::*;

mod time;
pub use time::*;

Expand Down
Loading
Loading