db_purge: ignore wu.batch unless --batch option#6947
Conversation
![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
Updates sched/db_purge.cpp to avoid treating workunit.batch as a BOINC batch ID by default (needed for NFS@home, which repurposes the field), while preserving the prior “purge only retired batches (and non-batch)” behavior behind an explicit flag.
Changes:
- Add a
--batchCLI option and associated help text. - Gate the “
batch in (retired_batch_ids)” SQL filter behind the--batchflag. - Only query retired batch IDs when
--batchis enabled.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Purge workunit and result records that are no longer needed.\n\n" | ||
| "Usage: db_purge [options]\n" | ||
| " -d N or --debug_level N Set verbosity level (1-4; 3=normal, 4=debug)\n" | ||
| " --batch purge batch jobs only if retired\n" |
There was a problem hiding this comment.
The new help text for --batch is misleading: the code enables filtering by retired batches (and batch=0), it doesn't mean “purge batch jobs only”. Consider rewording to something like “only purge WUs in retired batches (and non-batch WUs)” or otherwise clearly describe the actual behavior and the default when --batch is omitted.
| " --batch purge batch jobs only if retired\n" | |
| " --batch Only purge WUs in retired batches and non-batch WUs (default: all eligible WUs)\n" |
| // Purge WUs for which file_delete_state == FILE_DELETE_DONE | ||
| // and the WU is either | ||
| // and if --batch: | ||
| // the WU is either | ||
| // - in a retired batch | ||
| // - not in a batch |
There was a problem hiding this comment.
The updated header comment is grammatically awkward and unclear about the default behavior. Consider rewriting to a single sentence like: “If --batch is specified, only purge WUs that are either in a retired batch or not in a batch; otherwise ignore the batch field.”
| if (batch) { | ||
| 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 critical log message is missing a trailing newline, which can make logs harder to read and inconsistent with surrounding messages. Consider adding \n and including the returned error code for debugging.
| log_messages.printf(MSG_CRITICAL, "Can't get retired batch IDs"); | |
| log_messages.printf(MSG_CRITICAL, | |
| "Can't get retired batch IDs; retval=%d\n", | |
| retval | |
| ); |
fixes a problem for NFS@home, which uses the batch field for its own purposes
Summary by cubic
Make
db_purgeignore the workunit batch field by default; only filter by retired batches when--batchis used. This prevents unintended purges for projects like NFS@home that repurpose the batch field.--batchflag to purge only WUs in retired batches; otherwise the batch field is ignored.--batch; help text typo fixed and error message now includes the return code.Written for commit 2c971ac. Summary will update on new commits.