db_purge: remove --retired_wus option#6932
Conversation
Never purge WUs/results of a non-retired batch.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Removes the --retired_wus option from db_purge and changes purge eligibility so that workunits/results are purged only when they’re either in a retired batch or not in any batch.
Changes:
- Updated
db_purgeto always restrict purging to retired batches plus non-batch WUs/results (batch=0), and removed the--retired_wusCLI option. - Clarified/upgraded module comments across scheduler/tools, including an “NOT SUPPORTED” note in the legacy Python assimilator.
- Minor HTML/text touch-ups around batch retirement UI and utilities.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/process_input_template.cpp | Adds clarifying comments about staging and md5 handling. |
| sched/file_deleter.cpp | Improves header comment to more accurately describe behavior. |
| sched/db_purge.cpp | Removes --retired_wus and enforces “retired batch or no batch” purge restriction. |
| sched/assimilator.py | Adds a notice that this assimilator is not supported. |
| html/user/submit.php | Adjusts markup in the retire-batch confirmation message. |
| html/inc/submit_util.inc | Clarifies retire-batch comments and reorders/clarifies related inline comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo " | ||
| <p> | ||
| Retiring a batch will remove all of its output files. | ||
| Are you sure you want to do this? | ||
| <p> |
There was a problem hiding this comment.
This emits two opening <p> tags and no explicit closing </p>, producing invalid HTML (nested/implicit paragraph termination). Prefer a single well-formed paragraph, e.g. wrap the message in <p>...</p> (or remove the newly added <p> if the trailing one is the intended separator).
| string retired_batch_ids; | ||
| retval = get_retired_batch_ids(retired_batch_ids); | ||
| if (retval) { | ||
| log_messages.printf(MSG_CRITICAL, "Can't get retired batch IDs"); | ||
| exit(1); | ||
| } | ||
|
|
||
| bool did_something = do_pass(); | ||
| bool did_something = do_pass(retired_batch_ids); |
There was a problem hiding this comment.
The main loop now queries retired batch IDs on every iteration, even when there may be nothing to purge. This adds avoidable DB load and can become costly if the loop runs frequently. Consider replacing the “ID list building + IN (...)” approach with a single SQL predicate (e.g., batch=0 OR batch IN (SELECT id FROM batch WHERE state=RETIRED)) or caching the retired batch IDs and refreshing them on a timer (e.g., hourly) to reduce repeated queries.
| string retired_batch_ids; | ||
| retval = get_retired_batch_ids(retired_batch_ids); | ||
| if (retval) { | ||
| log_messages.printf(MSG_CRITICAL, "Can't get retired batch IDs"); |
There was a problem hiding this comment.
This error message loses useful diagnostics (no newline and no DB/MySQL error details). Include the underlying error code/message (e.g., mysql_error(boinc_db.mysql) / mysql_errno(...)) and a newline so operators can identify the root cause from logs.
| log_messages.printf(MSG_CRITICAL, "Can't get retired batch IDs"); | |
| log_messages.printf(MSG_CRITICAL, | |
| "Can't get retired batch IDs: %d: %s\n", | |
| mysql_errno(boinc_db.mysql), | |
| mysql_error(boinc_db.mysql) | |
| ); |
| } | ||
|
|
||
| // get list of IDs of retired batches | ||
| // get list of IDs of retired batches (and 0, = no batch) |
There was a problem hiding this comment.
The parenthetical is hard to parse (“and 0, = no batch”). Reword to something clearer like “including 0 (meaning ‘no batch’)” to avoid ambiguity for future maintainers.
| // get list of IDs of retired batches (and 0, = no batch) | |
| // get list of IDs of retired batches, including 0 (meaning "no batch") |
Never purge WUs/results of a non-retired batch.
Summary by cubic
Removed the
--retired_wusoption fromdb_purgeand made purging always limited to WUs withFILE_DELETE_DONEin retired batches or with no batch. This prevents purging from active batches.Refactors
db_purge: removed--retired_wus; queries now restrict tobatch in (0,<retired_ids>); updated usage text and flow.file_deleter,process_input_template,submit_util.inc; small copy tweak insubmit.php; markedassimilator.pyas not supported.Migration
--retired_wusfrom any scripts.Written for commit 1423143. Summary will update on new commits.