Skip to content

db_purge: batch a workunit's result deletions into one "where id in (...)" query - #7316

Closed
FraSanga wants to merge 2 commits into
BOINC:masterfrom
FraSanga:db_purge_batch_result_deletes
Closed

FraSanga wants to merge 2 commits into
BOINC:masterfrom
FraSanga:db_purge_batch_result_deletes

Conversation

@FraSanga

@FraSanga FraSanga commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What

db_purge deleted each of a workunit's results with its own delete from result where id = X query in purge_and_archive_results(). For a project with target_nresults = N that's N round-trips per workunit on top of the one for the workunit itself.

This collects the result IDs while archiving and issues a single delete from result where id in (id1, id2, ...) per workunit instead. The IN () list is chunked at 1000 IDs so the generated query stays well within MAX_QUERY_LEN (262144) — 1000 twenty-digit IDs plus separators is ~21 KB.

Partly addresses #6735 — workunit and assignment deletes are still one-per-row; batching those defers deletes across a whole do_pass() and widens the archived-but-not-deleted window much further, so I left it for a separate change.

Unchanged

  • Archiving — still per-result, same order, same archive_result() / archive_result_gz() paths.
  • --dont_delete — still logs Didn't purge result [...] per result and deletes nothing (the ID list stays empty).
  • Per-result loggingPurged result [id] batch N is still emitted for every purged result (now grouped after the batched delete rather than interleaved with individual deletes).
  • PR sched/db_purge: if a result fails to be deleted, don't purge the referencing workunit #5694's invariant — a workunit is still never deleted unless its results all deleted first: a failed batch delete returns the error, do_pass() does continue, and the workunit + results are retried next pass.
  • --max, --max_wu_per_file, enable_assignment, the workunit delete itself, number_results — untouched.

Behaviour change

Result deletes are issued once per workunit, after the workunit has been fully archived, instead of one immediately after each result is archived. If db_purge is killed (OOM / power loss) between archiving a workunit's results and the batched delete committing, the restart re-enumerates the workunit and re-archives its entire result set — previously this window was at most one result. No DB inconsistency (the workunit is still never deleted while any of its results survive) and archive entries are keyed by ID for de-duplication, but it's a real change to crash-recovery behaviour in the area #5694 touched.

Also: the two DB_RESULT deletes previously happened while a mysql_store_result-buffered enumeration over result was open; they now happen after that enumeration finishes, which is strictly safer.

Testing

  • g++ -fsyntax-only against the real headers is clean, no new -Wall -Wextra warnings.
  • The chunking of delete_ids_from_db() was unit-tested in isolation: 0 IDs -> 0 queries, 3 -> 1, exactly 1000 -> 1, 1001 -> 2, 2500 -> 3; no leading/trailing commas; full-width IDs format correctly.
  • I couldn't run a full server build in my environment; relying on CI for the complete compile/link.

Assisted by Claude (Sonnet 5) per the BOINC AI Assistants Usage Policy; the commit carries the Assisted-by: trailer. I've read the full diff and verified the behaviour described above.


Summary by cubic

Batches result deletions in db_purge from one query per result to a single WHERE id IN (...) query per workunit, reducing database round-trips. The query is chunked at 1000 IDs to stay within MAX_QUERY_LEN.

Behavior change

  • Result deletes now happen after the whole workunit is archived, so a crash in that window re-archives the entire result set on restart (previously at most one result). Archiving order, --dont_delete handling, and the per-result log format are unchanged.
  • Per-result purge logs are emitted after each chunk's delete commits, so a failure on a later chunk doesn't hide results already deleted.

Written for commit 41a4f11. Summary will update on new commits.

Review in cubic

…...)"

purge_and_archive_results() deleted each of a workunit's results with a
separate "delete from result where id=X" query. Collect the IDs while
archiving and delete them in one batched "where id in (...)" query
instead, split into chunks of 1000 to keep the query well within
MAX_QUERY_LEN.

Archiving, per-result logging, --dont_delete handling, number_results,
and the "don't purge a workunit whose results failed to delete"
invariant (BOINC#5694) are unchanged. One behaviour change: because the
result deletes are now issued after the whole workunit has been
archived rather than one-by-one, a hard kill between archiving and
deleting re-archives that workunit's entire result set on restart
(previously at most one result); archive entries are keyed by ID so
consumers can de-duplicate.

Partly addresses BOINC#6735 (workunit and assignment deletes are still
one-per-row).

Assisted-by: Claude:sonnet-5
Copilot AI lite review requested due to automatic review settings September 9, 2026 14:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread sched/db_purge.cpp Outdated
Move the batched result delete into a helper that logs each purged
result right after its chunk's DELETE commits, instead of after the
whole set. For a workunit with more than 1000 results, if a later
chunk's DELETE fails, the "Purged result" lines for the chunks that
already succeeded are no longer skipped by the early return.

Assisted-by: Claude:sonnet-5
Copilot AI review requested due to automatic review settings September 9, 2026 15:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@FraSanga

FraSanga commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the cubic P3 (per-result "Purged result" log skipped when a later DELETE chunk fails on a >1000-result workunit) in 41a4f11.

The batched delete now lives in a helper that logs each result immediately after its own chunk's DELETE commits, rather than after the whole set. On a later-chunk failure the earlier chunks' log lines are already emitted; the failure path then logs MSG_CRITICAL with the workunit id and the DB retval and returns.

Note: the failing source-code-check-trailing-whitespaces check is unrelated to this PR — it's client/hostinfo_unix.cpp:780 (a space-only line) from 1e842f7, on current master. This branch doesn't touch that file.

@davidpanderson

Copy link
Copy Markdown
Contributor

The number of results per workunit will rarely exceed 2, and never exceed 10.
So this is needlessly complex.

The performance of db_purge has never been an issue, so is this needed at all?

@FraSanga

FraSanga commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

As you wish. Actually, if a project doesn’t even exceed 2 workunits, there won’t usually be any significant improvements. Do we want to close the PR along with the issue, or did you have something else in mind?

@davidpanderson

Copy link
Copy Markdown
Contributor

I'll close it. We have a lot of stuff going on right now, so I'd prefer to stay away from changes that don't fix something concrete.

@github-project-automation github-project-automation Bot moved this from Backlog to Done in Server Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants