Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ Version control uses Jujutsu (`jj`) with Git.
- Do **NOT** add `Co-Authored-By` lines to commit messages
- Do **NOT** add `🤖 Generated with [Claude Code](https://claude.com/claude-code)` to PR descriptions

## Public Disclosure

This is a public repository. **NEVER** disclose sensitive information about the production setup, data, or operations in any public-facing artifact: PR descriptions, PR/issue comments, commit messages, code comments, or docs. This includes production database sizes, row counts, table statistics, customer or project identifiers, incident details and timelines, and measured production performance characteristics. Describe changes in relative terms (e.g., "a large report" instead of actual production numbers) and keep concrete production figures in private channels.

## Writing Style

- **NEVER** use emdashes in any writing output: code comments, commit messages, PR descriptions, docs, issue/PR replies, and chat responses. Use a colon, semicolon, comma, parentheses, or two sentences instead.
Expand Down
2 changes: 2 additions & 0 deletions lib/api_projects/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ mod thresholds;

mod macros;

pub use reports::DELETE_CHUNK_SIZE;

pub struct Api;

impl bencher_endpoint::Registrar for Api {
Expand Down
52 changes: 48 additions & 4 deletions lib/api_projects/src/reports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use bencher_schema::{
head::HeadId,
version::{QueryVersion, VersionId},
},
report::{NewRunReport, QueryReport, ReportId},
report::{NewRunReport, QueryReport, ReportId, report_benchmark::ReportBenchmarkId},
},
user::{
actor::{ApiActor, PubProjectBearerToken},
Expand Down Expand Up @@ -446,6 +446,12 @@ pub async fn proj_report_delete(
Ok(Delete::auth_response_deleted())
}

/// Number of `report_benchmark` rows deleted per write statement when
/// deleting a report's results. Bounds how long each delete holds the single
/// writer connection (validated at ~1s per chunk against a production-scale
/// report).
pub const DELETE_CHUNK_SIZE: i64 = 1024;

async fn delete_inner(
context: &ApiContext,
path_params: ProjReportParams,
Expand All @@ -470,10 +476,15 @@ async fn delete_inner(
Report,
(&query_project, path_params.report)
))?;
delete_report_results(context, &query_project, report_id).await?;

diesel::delete(schema::report::table.filter(schema::report::id.eq(report_id)))
.execute(write_conn!(context))
.map_err(resource_conflict_err!(Report, report_id))?;

#[cfg(feature = "otel")]
bencher_otel::ApiMeter::increment(bencher_otel::ApiCounter::ReportDelete);

// If there are no more reports for this version, delete the version
// This is necessary because multiple reports can use the same version via a git hash
// This will cascade and delete all head versions for this version
Expand Down Expand Up @@ -549,8 +560,41 @@ async fn delete_inner(
(&query_project, report_id, &query_version)
))?;

#[cfg(feature = "otel")]
bencher_otel::ApiMeter::increment(bencher_otel::ApiCounter::ReportDelete);

Ok(())
}

/// Delete a report's results in bounded chunks, each in its own write
/// statement, so a large report does not hold the single writer connection
/// for its entire cascade (`report_benchmark` -> metric -> boundary -> alert).
/// Other writers interleave between chunks. Any rows that land between the
/// final chunk and the report row delete are cleaned up by the report's own
/// cascade.
///
/// The deletion is deliberately not atomic: if the server fails mid-loop, the
/// report remains visible with a partial set of results (and a stale
/// `metric_count_by_report` rollup) until the delete is retried. This is
/// acceptable because reports are immutable after creation, the delete is
/// idempotent, and the report was already condemned by the caller.
async fn delete_report_results(
context: &ApiContext,
query_project: &QueryProject,
report_id: ReportId,
) -> Result<(), HttpError> {
loop {
let report_benchmark_ids = schema::report_benchmark::table
.filter(schema::report_benchmark::report_id.eq(report_id))
.select(schema::report_benchmark::id)
.limit(DELETE_CHUNK_SIZE)
.load::<ReportBenchmarkId>(auth_conn!(context))
.map_err(resource_not_found_err!(Report, (query_project, report_id)))?;
if report_benchmark_ids.is_empty() {
return Ok(());
}
diesel::delete(
schema::report_benchmark::table
.filter(schema::report_benchmark::id.eq_any(report_benchmark_ids)),
)
.execute(write_conn!(context))
.map_err(resource_conflict_err!(Report, report_id))?;
}
}
Loading
Loading