mirror of
https://github.com/fawney19/Aether.git
synced 2026-09-13 22:50:19 +08:00
Fix provider deletion cleanup
This commit is contained in:
@@ -426,18 +426,24 @@ impl GatewayDataState {
|
||||
pub(crate) async fn cleanup_deleted_provider_catalog_refs(
|
||||
&self,
|
||||
provider_id: &str,
|
||||
provider_deleted: bool,
|
||||
endpoint_ids: &[String],
|
||||
key_ids: &[String],
|
||||
) -> Result<(), DataLayerError> {
|
||||
let cleaned = match &self.provider_catalog_writer {
|
||||
Some(repository) => {
|
||||
repository
|
||||
.cleanup_deleted_provider_refs(provider_id, endpoint_ids, key_ids)
|
||||
.cleanup_deleted_provider_refs(
|
||||
provider_id,
|
||||
provider_deleted,
|
||||
endpoint_ids,
|
||||
key_ids,
|
||||
)
|
||||
.await
|
||||
}
|
||||
None => Ok(()),
|
||||
};
|
||||
if !endpoint_ids.is_empty() || !key_ids.is_empty() {
|
||||
if provider_deleted || !endpoint_ids.is_empty() || !key_ids.is_empty() {
|
||||
self.clear_provider_catalog_cache();
|
||||
}
|
||||
cleaned
|
||||
|
||||
@@ -94,7 +94,7 @@ pub(crate) async fn run_admin_provider_delete_task(
|
||||
.map(|item| item.id.clone())
|
||||
.collect::<Vec<_>>();
|
||||
let key_ids = keys.iter().map(|item| item.id.clone()).collect::<Vec<_>>();
|
||||
app.cleanup_deleted_provider_catalog_refs(&provider.id, &endpoint_ids, &key_ids)
|
||||
app.cleanup_deleted_provider_catalog_refs(&provider.id, true, &endpoint_ids, &key_ids)
|
||||
.await?;
|
||||
|
||||
task.stage = "deleting_models".to_string();
|
||||
|
||||
@@ -220,11 +220,17 @@ impl<'a> AdminAppState<'a> {
|
||||
pub(crate) async fn cleanup_deleted_provider_catalog_refs(
|
||||
&self,
|
||||
provider_id: &str,
|
||||
provider_deleted: bool,
|
||||
endpoint_ids: &[String],
|
||||
key_ids: &[String],
|
||||
) -> Result<(), GatewayError> {
|
||||
self.app
|
||||
.cleanup_deleted_provider_catalog_refs(provider_id, endpoint_ids, key_ids)
|
||||
.cleanup_deleted_provider_catalog_refs(
|
||||
provider_id,
|
||||
provider_deleted,
|
||||
endpoint_ids,
|
||||
key_ids,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
@@ -260,7 +260,7 @@ impl<'a> AdminAppState<'a> {
|
||||
affected += 1;
|
||||
}
|
||||
}
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, &[], &deleted_key_ids)
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, false, &[], &deleted_key_ids)
|
||||
.await?;
|
||||
|
||||
Ok(affected)
|
||||
@@ -295,7 +295,7 @@ impl<'a> AdminAppState<'a> {
|
||||
let deleted = self.delete_provider_catalog_key(&key.id).await?;
|
||||
if deleted {
|
||||
let deleted_key_ids = [key.id.clone()];
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, &[], &deleted_key_ids)
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, false, &[], &deleted_key_ids)
|
||||
.await?;
|
||||
}
|
||||
Ok(deleted)
|
||||
@@ -356,7 +356,7 @@ impl<'a> AdminAppState<'a> {
|
||||
affected = affected.saturating_add(1);
|
||||
}
|
||||
}
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, &[], &deleted_key_ids)
|
||||
self.cleanup_deleted_provider_catalog_refs(&provider.id, false, &[], &deleted_key_ids)
|
||||
.await?;
|
||||
|
||||
return Ok(Json(
|
||||
|
||||
@@ -564,11 +564,17 @@ impl AppState {
|
||||
pub(crate) async fn cleanup_deleted_provider_catalog_refs(
|
||||
&self,
|
||||
provider_id: &str,
|
||||
provider_deleted: bool,
|
||||
endpoint_ids: &[String],
|
||||
key_ids: &[String],
|
||||
) -> Result<(), GatewayError> {
|
||||
self.data
|
||||
.cleanup_deleted_provider_catalog_refs(provider_id, endpoint_ids, key_ids)
|
||||
.cleanup_deleted_provider_catalog_refs(
|
||||
provider_id,
|
||||
provider_deleted,
|
||||
endpoint_ids,
|
||||
key_ids,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| GatewayError::Internal(err.to_string()))?;
|
||||
for key_id in key_ids {
|
||||
@@ -770,6 +776,25 @@ impl AppState {
|
||||
tasks.insert(task.task_id.clone(), task);
|
||||
}
|
||||
|
||||
pub(crate) fn reserve_provider_delete_task(
|
||||
&self,
|
||||
task: LocalProviderDeleteTaskState,
|
||||
) -> LocalProviderDeleteTaskState {
|
||||
let mut tasks = self
|
||||
.provider_delete_tasks
|
||||
.lock()
|
||||
.expect("provider delete tasks cache should lock");
|
||||
if let Some(existing) = tasks
|
||||
.values()
|
||||
.find(|existing| existing.provider_id == task.provider_id && existing.is_active())
|
||||
.cloned()
|
||||
{
|
||||
return existing;
|
||||
}
|
||||
tasks.insert(task.task_id.clone(), task.clone());
|
||||
task
|
||||
}
|
||||
|
||||
pub(crate) fn get_provider_delete_task(
|
||||
&self,
|
||||
task_id: &str,
|
||||
|
||||
@@ -1291,6 +1291,7 @@ impl AppState {
|
||||
let deleted_key_ids = [key_id.to_string()];
|
||||
self.cleanup_deleted_provider_catalog_refs(
|
||||
&transport.provider.id,
|
||||
false,
|
||||
&[],
|
||||
&deleted_key_ids,
|
||||
)
|
||||
|
||||
@@ -11,6 +11,12 @@ pub(crate) struct LocalProviderDeleteTaskState {
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
impl LocalProviderDeleteTaskState {
|
||||
pub(crate) fn is_active(&self) -> bool {
|
||||
matches!(self.status.as_str(), "pending" | "running")
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) enum LocalMutationOutcome<T> {
|
||||
Applied(T),
|
||||
|
||||
@@ -46,6 +46,7 @@ pub(crate) const TASK_KEY_STATS_HOURLY_AGG: &str = "maintenance.stats.hourly.agg
|
||||
pub(crate) const TASK_KEY_USAGE_SYNC_REPORT: &str = "usage.sync.report";
|
||||
pub(crate) const TASK_KEY_PROVIDER_OAUTH_ACCOUNT_REFRESH: &str = "provider.oauth.account.refresh";
|
||||
pub(crate) const TASK_KEY_PROVIDER_BALANCE_REFRESH: &str = "provider.ops.balance.refresh";
|
||||
const PROVIDER_DELETE_LOCK_TTL_SECS: u64 = 60 * 60 * 6;
|
||||
|
||||
const RETRY_ONCE: RetryPolicy = RetryPolicy { max_attempts: 1 };
|
||||
|
||||
@@ -501,17 +502,23 @@ pub(crate) async fn submit_provider_delete_task(
|
||||
};
|
||||
|
||||
let task_id = Uuid::new_v4().simple().to_string()[..16].to_string();
|
||||
state.put_provider_delete_task(crate::LocalProviderDeleteTaskState {
|
||||
task_id: task_id.clone(),
|
||||
provider_id: provider.id.clone(),
|
||||
status: "pending".to_string(),
|
||||
stage: "queued".to_string(),
|
||||
total_keys: 0,
|
||||
deleted_keys: 0,
|
||||
total_endpoints: 0,
|
||||
deleted_endpoints: 0,
|
||||
message: "delete task submitted".to_string(),
|
||||
});
|
||||
let reserved =
|
||||
state
|
||||
.as_ref()
|
||||
.reserve_provider_delete_task(crate::LocalProviderDeleteTaskState {
|
||||
task_id: task_id.clone(),
|
||||
provider_id: provider.id.clone(),
|
||||
status: "pending".to_string(),
|
||||
stage: "queued".to_string(),
|
||||
total_keys: 0,
|
||||
deleted_keys: 0,
|
||||
total_endpoints: 0,
|
||||
deleted_endpoints: 0,
|
||||
message: "delete task submitted".to_string(),
|
||||
});
|
||||
if reserved.task_id != task_id {
|
||||
return Ok(Some(reserved.task_id));
|
||||
}
|
||||
|
||||
let app = state.cloned_app();
|
||||
let provider_id = provider.id.clone();
|
||||
@@ -555,7 +562,7 @@ pub(crate) async fn submit_provider_delete_task(
|
||||
|
||||
spawn_named("task-runtime-provider-delete", async move {
|
||||
let lock_key = format!("task_runtime:lock:{TASK_KEY_PROVIDER_DELETE}:{provider_id}");
|
||||
let lock_ttl = std::time::Duration::from_secs(60 * 15);
|
||||
let lock_ttl = std::time::Duration::from_secs(PROVIDER_DELETE_LOCK_TTL_SECS);
|
||||
let lock = app
|
||||
.runtime_state
|
||||
.lock_try_acquire(&lock_key, app.tunnel.local_instance_id(), lock_ttl)
|
||||
@@ -563,6 +570,17 @@ pub(crate) async fn submit_provider_delete_task(
|
||||
.ok()
|
||||
.flatten();
|
||||
if lock.is_none() {
|
||||
app.put_provider_delete_task(crate::LocalProviderDeleteTaskState {
|
||||
task_id: run_id.clone(),
|
||||
provider_id: provider_id.clone(),
|
||||
status: "failed".to_string(),
|
||||
stage: "skipped".to_string(),
|
||||
total_keys: 0,
|
||||
deleted_keys: 0,
|
||||
total_endpoints: 0,
|
||||
deleted_endpoints: 0,
|
||||
message: "provider delete skipped: another node is running this task".to_string(),
|
||||
});
|
||||
let _ = update_run_status(
|
||||
&app,
|
||||
&run_id,
|
||||
|
||||
@@ -117,6 +117,48 @@ fn admin_provider_oauth_complete_dispatch_remains_thin() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_provider_cleanup_preserves_usage_history() {
|
||||
let postgres_provider_catalog =
|
||||
read_workspace_file("crates/aether-data/src/repository/provider_catalog/postgres.rs");
|
||||
|
||||
for forbidden in [
|
||||
"UPDATE usage SET provider_id = NULL",
|
||||
"UPDATE usage SET provider_endpoint_id = NULL",
|
||||
"UPDATE usage SET provider_api_key_id = NULL",
|
||||
] {
|
||||
assert!(
|
||||
!postgres_provider_catalog.contains(forbidden),
|
||||
"provider cleanup must not rewrite usage history with {forbidden}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_cleanup_keeps_common_backends_in_sync() {
|
||||
for path in [
|
||||
"crates/aether-data/src/repository/provider_catalog/postgres.rs",
|
||||
"crates/aether-data/src/repository/provider_catalog/mysql.rs",
|
||||
"crates/aether-data/src/repository/provider_catalog/sqlite.rs",
|
||||
] {
|
||||
let source = read_workspace_file(path);
|
||||
for required in [
|
||||
"UPDATE user_preferences SET default_provider_id = NULL WHERE default_provider_id =",
|
||||
"UPDATE video_tasks SET provider_id = NULL WHERE provider_id =",
|
||||
"DELETE FROM request_candidates WHERE provider_id =",
|
||||
"UPDATE video_tasks SET endpoint_id = NULL WHERE endpoint_id =",
|
||||
"DELETE FROM request_candidates WHERE endpoint_id =",
|
||||
"DELETE FROM gemini_file_mappings WHERE key_id =",
|
||||
"UPDATE video_tasks SET key_id = NULL WHERE key_id =",
|
||||
] {
|
||||
assert!(
|
||||
source.contains(required),
|
||||
"{path} should keep provider cleanup behavior in sync with {required}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn admin_provider_oauth_complete_helpers_are_split() {
|
||||
let complete_mod = read_workspace_file(
|
||||
|
||||
@@ -1703,6 +1703,52 @@ async fn gateway_submits_admin_provider_delete_task_locally_with_trusted_admin_p
|
||||
upstream_handle.abort();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_delete_task_reservation_reuses_active_provider_task() {
|
||||
let state = AppState::new().expect("gateway should build");
|
||||
let first = crate::LocalProviderDeleteTaskState {
|
||||
task_id: "task-first".to_string(),
|
||||
provider_id: "provider-openai".to_string(),
|
||||
status: "pending".to_string(),
|
||||
stage: "queued".to_string(),
|
||||
total_keys: 0,
|
||||
deleted_keys: 0,
|
||||
total_endpoints: 0,
|
||||
deleted_endpoints: 0,
|
||||
message: "delete task submitted".to_string(),
|
||||
};
|
||||
let second = crate::LocalProviderDeleteTaskState {
|
||||
task_id: "task-second".to_string(),
|
||||
provider_id: "provider-openai".to_string(),
|
||||
status: "pending".to_string(),
|
||||
stage: "queued".to_string(),
|
||||
total_keys: 0,
|
||||
deleted_keys: 0,
|
||||
total_endpoints: 0,
|
||||
deleted_endpoints: 0,
|
||||
message: "delete task submitted".to_string(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
state.reserve_provider_delete_task(first.clone()).task_id,
|
||||
"task-first"
|
||||
);
|
||||
assert_eq!(
|
||||
state.reserve_provider_delete_task(second.clone()).task_id,
|
||||
"task-first"
|
||||
);
|
||||
|
||||
state.put_provider_delete_task(crate::LocalProviderDeleteTaskState {
|
||||
status: "completed".to_string(),
|
||||
stage: "completed".to_string(),
|
||||
..first
|
||||
});
|
||||
assert_eq!(
|
||||
state.reserve_provider_delete_task(second).task_id,
|
||||
"task-second"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_admin_provider_delete_task_status_attaches_audit_only_for_terminal_states() {
|
||||
let mut completed_state = AppState::new().expect("gateway should build");
|
||||
|
||||
Reference in New Issue
Block a user