Delete a batch of imports in one command, not one connection each
Replaces the connection throttle from the previous commit, which was a workaround: it made a batch slower without changing that N sessions meant N ssh connections, and the cap it picked was a guess at somebody else's sshd config. The batch now goes out as a single invocation. import::delete takes the whole list, the remote script loops over the ids and prints one `<id>\t<state>` line each, and the route settles every row from its own line. So a batch of any size is one connection and cannot exceed MaxStartups however many rows are selected -- and it is faster, since it stopped paying a handshake per session. Each id still reports on its own: deleted, missing, or failed, kept apart because only "failed" is worth retrying. Ids the machine never mentioned -- a connection that dropped part-way -- are reported as unknown rather than defaulting to either answer, and a malformed id fails only itself. Verified end-to-end against a fake ssh that counts connections: a 9-session batch used one, every row settled, both copies of a session recorded under two project directories went, and the id that was not there failed with a message saying so rather than a connection error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7a1a462caf
commit
dce1799802
4 files changed
+230
-79
No files matched your search
+64
-18
@@ -549,28 +549,68 @@ struct ImportableRow {
|
||||
/// id is registered as in flight before the 202 goes back, so the answer to
|
||||
/// "did my batch start" is one answer for the batch.
|
||||
///
|
||||
/// Registering is what has to be atomic; the work itself does not. Each id
|
||||
/// runs on its own task and settles on its own event, because six deletes
|
||||
/// that must all succeed or all roll back is not something a filesystem
|
||||
/// offers, and pretending otherwise would mean holding five sessions
|
||||
/// hostage to the one that failed.
|
||||
/// Registering is what has to be atomic; the work itself does not. The
|
||||
/// batch runs as one command on the machine -- see
|
||||
/// [`crate::session::import::delete`] for why it is not one per id -- but
|
||||
/// each row still settles on its own event from its own outcome, because
|
||||
/// six deletes that must all succeed or all roll back is not something a
|
||||
/// filesystem offers, and pretending otherwise would mean holding five
|
||||
/// sessions hostage to the one that failed.
|
||||
async fn delete_importable(
|
||||
State(manager): State<Arc<SessionManager>>,
|
||||
UrlPath(id): UrlPath<String>,
|
||||
axum::Json(body): axum::Json<DeleteBatch>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
let setup = setup_by_id(&manager, &id)?;
|
||||
for session in body.sessions {
|
||||
let transport = crate::session::transport::Transport::for_setup(&setup);
|
||||
let target = session.clone();
|
||||
in_background(
|
||||
&manager,
|
||||
id.clone(),
|
||||
session,
|
||||
Operation::Deleting,
|
||||
async move { crate::session::import::delete(&transport, &target).await },
|
||||
);
|
||||
}
|
||||
let transport = crate::session::transport::Transport::for_setup(&setup);
|
||||
// Registered before anything is spawned, so the 202 is only sent once
|
||||
// every row is already showing "deleting" -- a phone that refetches the
|
||||
// instant it gets the reply cannot catch a row that has not started.
|
||||
let running: Vec<(String, crate::session::pending::InFlight)> = body
|
||||
.sessions
|
||||
.iter()
|
||||
.map(|session| {
|
||||
(
|
||||
session.clone(),
|
||||
manager.pending().begin(&id, session, Operation::Deleting),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let sessions = body.sessions;
|
||||
tokio::spawn(async move {
|
||||
// One failure here is the machine being unreachable, which is true
|
||||
// of every row rather than of any one of them, so they all say so.
|
||||
let outcomes = match crate::session::import::delete(&transport, &sessions).await {
|
||||
Ok(outcomes) => outcomes,
|
||||
Err(err) => {
|
||||
let message = format!("{err:#}");
|
||||
tracing::warn!(
|
||||
"deleting {} sessions on {id} failed: {message}",
|
||||
running.len()
|
||||
);
|
||||
for (_, flight) in running {
|
||||
flight.failed(message.clone());
|
||||
}
|
||||
return;
|
||||
}
|
||||
};
|
||||
for (session, flight) in running {
|
||||
match outcomes.get(&session) {
|
||||
Some(Ok(())) => {
|
||||
tracing::info!("deleting {session} on {id}: done");
|
||||
flight.succeeded();
|
||||
}
|
||||
Some(Err(message)) => {
|
||||
tracing::warn!("deleting {session} on {id} failed: {message}");
|
||||
flight.failed(message.clone());
|
||||
}
|
||||
// `delete` promises an entry per id, so this is a bug
|
||||
// rather than a state -- but a row stuck on "deleting"
|
||||
// for ever is a worse answer than one that says so.
|
||||
None => flight.failed(format!("nothing was reported about {session}")),
|
||||
}
|
||||
}
|
||||
});
|
||||
Ok(StatusCode::ACCEPTED)
|
||||
}
|
||||
|
||||
@@ -872,9 +912,15 @@ async fn delete_session(
|
||||
if let Some((setup, session)) = &foreign {
|
||||
let setup = setup_by_id(&manager, setup)?;
|
||||
let transport = crate::session::transport::Transport::for_setup(&setup);
|
||||
crate::session::import::delete(&transport, session)
|
||||
// A batch of one: the same call, so there is one description of
|
||||
// what deleting a foreign transcript means. Its outcome is this
|
||||
// request's outcome, since there is only the one row.
|
||||
crate::session::import::delete(&transport, std::slice::from_ref(session))
|
||||
.await
|
||||
.map_err(bad_request)?;
|
||||
.map_err(bad_request)?
|
||||
.remove(session)
|
||||
.unwrap_or_else(|| Err(format!("nothing was reported about {session}")))
|
||||
.map_err(|message| bad_request(anyhow::anyhow!("{message}")))?;
|
||||
tracing::info!("deleted Claude Code session {session} with ai-app session {id}");
|
||||
}
|
||||
manager.delete_session(&id).map_err(bad_request)?;
|
||||
|
||||
Reference in new issue
Block a user