Skip to content

Consolidate manual-job resubmission into check-jobs; drop run --recreate-jobs - #541

Open
valsdav wants to merge 3 commits into
mainfrom
fix/check-jobs-robustness
Open

Consolidate manual-job resubmission into check-jobs; drop run --recreate-jobs#541
valsdav wants to merge 3 commits into
mainfrom
fix/check-jobs-robustness

Conversation

@valsdav

@valsdav valsdav commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The runner/executor --recreate-jobs path and check-jobs --resubmit duplicated a lot of logic (two find_other_file, two queue-bump helpers + two queues lists, two fileset-rewrite + condor-resubmit loops) while operating on the same jobs_dir on-disk contract. Fold all of it into check-jobs so there is a single consumer, and remove the runner path.

check-jobs gains a one-shot proactive recreate mode and the recreate-only knobs that used to live on pocket-coffea run:

  • --recreate auto|0,1,3 (+ --once) — recreate/resubmit a chosen set of jobs and exit; unlike --resubmit it can act on running/idle jobs too.
  • --use-redirector, --blocklist-sites (unioned with the auto blacklist), --recreate-queue, --skip-bad-files.

Shared/reused instead of duplicated:

  • new utils/htcondor_queue.py (QUEUES, bump_queue, set_queue) replaces check_jobs.bump_jobqueue and lxplus update_queue/set_queue + both queues.
  • site_rewrite.find_other_file extended (exclude_urls, blocklist-by-prefix, fallback_redirector=None) and now used by check-jobs in place of its private copy.
  • the submit-time inner_run_options helpers stay in executors_manual_jobs and are reused by check-jobs' --skip-bad-files.

Removed: recreate_jobs from executors_lxplus/rubin, the ABC abstractmethod, the submit() dispatch and init guard-skip, and the --blocklist-sites/--recreate-queue/--use-redirector options on the runner (--skip-bad-files stays; it also feeds the outer Coffea Runner). run now errors with a pointer to check-jobs if a moved flag is passed.

Also fixes two latent bugs: the babysitter's undefined log_file NameError on the "failed-again after resubmit" path, and the old explicit-list recreate crashing on an undefined runningjobs.

valsdav and others added 2 commits July 9, 2026 17:52
The babysitter loop only caught KeyboardInterrupt, so any unguarded parse killed a
session that may have run for days. Guard the known crash sites:

- bump_jobqueue: `queues.index(jf)` raised ValueError for a non-lxplus flavour, and
  `return next_jf` raised NameError when the .sub had no +JobFlavour line (e.g. rubin's
  +MaxRuntime). Fall back to the longest known queue for an unknown flavour and return
  None when there is no flavour line.
- The SYSTEM_PERIODIC_REMOVE check did `glob(...logs/job_*.log)[0]` -> IndexError before
  any log exists; skip when there are no logs yet.
- XRootD-error log parsing indexed `c[iln+1]`/`c[iln+3]` and `.split()[-1]` without
  bounds checks -> IndexError on a truncated log; guard both.
- Add a broad except around the loop that reports the error clearly instead of dumping
  a raw traceback.

Adds offline tests for bump_jobqueue (normal, capped, unknown flavour, no flavour line).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ate-jobs

The runner/executor `--recreate-jobs` path and `check-jobs --resubmit`
duplicated a lot of logic (two `find_other_file`, two queue-bump helpers +
two `queues` lists, two fileset-rewrite + condor-resubmit loops) while
operating on the same jobs_dir on-disk contract. Fold all of it into
`check-jobs` so there is a single consumer, and remove the runner path.

check-jobs gains a one-shot proactive recreate mode and the recreate-only
knobs that used to live on `pocket-coffea run`:
- `--recreate auto|0,1,3` (+ `--once`) — recreate/resubmit a chosen set of
  jobs and exit; unlike `--resubmit` it can act on running/idle jobs too.
- `--use-redirector`, `--blocklist-sites` (unioned with the auto blacklist),
  `--recreate-queue`, `--skip-bad-files`.

Shared/reused instead of duplicated:
- new `utils/htcondor_queue.py` (QUEUES, bump_queue, set_queue) replaces
  check_jobs.bump_jobqueue and lxplus update_queue/set_queue + both `queues`.
- `site_rewrite.find_other_file` extended (exclude_urls, blocklist-by-prefix,
  fallback_redirector=None) and now used by check-jobs in place of its private
  copy.
- the submit-time inner_run_options helpers stay in executors_manual_jobs and
  are reused by check-jobs' `--skip-bad-files`.

Removed: recreate_jobs from executors_lxplus/rubin, the ABC abstractmethod,
the submit() dispatch and __init__ guard-skip, and the
--blocklist-sites/--recreate-queue/--use-redirector options on the runner
(--skip-bad-files stays; it also feeds the outer Coffea Runner). `run` now
errors with a pointer to check-jobs if a moved flag is passed.

Also fixes two latent bugs: the babysitter's undefined `log_file` NameError on
the "failed-again after resubmit" path, and the old explicit-list recreate
crashing on an undefined `runningjobs`.

Docs updated; adds tests/test_check_jobs_recreate.py plus set_queue and
find_other_file-extension cases. 39 offline unit tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@valsdav
valsdav force-pushed the fix/check-jobs-robustness branch from b559550 to 5575af0 Compare July 9, 2026 15:54
@valsdav
valsdav marked this pull request as ready for review July 9, 2026 15:54
@valsdav

valsdav commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

What do you think about this interface change? Asking specifically to @mmarchegiani @mondalspandan as they introduced the two code paths

@mondalspandan

Copy link
Copy Markdown
Contributor

Hi Davide, did you already test these changes in an actual workflow? If not, I suggest one of us does it, and then we merge, since these are substantial changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants