Safekeeper: avoid holding mutex when deleting a tenant (#1746)

Following discussion with @arssher after #1653
This commit is contained in:
Egor Suvorov
2022-05-18 22:02:17 +02:00
committed by GitHub
parent 98da0aa159
commit 432907ff5f

View File

@@ -679,29 +679,32 @@ impl GlobalTimelines {
/// Deactivates and deletes all timelines for the tenant, see `delete()`.
/// Returns map of all timelines which the tenant had, `true` if a timeline was active.
/// There may be a race if new timelines are created simultaneously.
pub fn delete_force_all_for_tenant(
conf: &SafeKeeperConf,
tenant_id: &ZTenantId,
) -> Result<HashMap<ZTenantTimelineId, TimelineDeleteForceResult>> {
info!("deleting all timelines for tenant {}", tenant_id);
let mut state = TIMELINES_STATE.lock().unwrap();
let mut deleted = HashMap::new();
for (zttid, tli) in &state.timelines {
if zttid.tenant_id == *tenant_id {
deleted.insert(
*zttid,
GlobalTimelines::delete_force_internal(
conf,
zttid,
tli.deactivate_for_delete()?,
)?,
);
let mut to_delete = HashMap::new();
{
// Keep mutex in this scope.
let timelines = &mut TIMELINES_STATE.lock().unwrap().timelines;
for (&zttid, tli) in timelines.iter() {
if zttid.tenant_id == *tenant_id {
to_delete.insert(zttid, tli.deactivate_for_delete()?);
}
}
// TODO: test that the correct subset of timelines is removed. It's complicated because they are implicitly created currently.
timelines.retain(|zttid, _| !to_delete.contains_key(zttid));
}
// TODO: test that the exact subset of timelines is removed.
state
.timelines
.retain(|zttid, _| !deleted.contains_key(zttid));
let mut deleted = HashMap::new();
for (zttid, was_active) in to_delete {
deleted.insert(
zttid,
GlobalTimelines::delete_force_internal(conf, &zttid, was_active)?,
);
}
// There may be inactive timelines, so delete the whole tenant dir as well.
match std::fs::remove_dir_all(conf.tenant_dir(tenant_id)) {
Ok(_) => (),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => (),