fix: bound copy concurrency process-wide; unify run exit codes - #248
Merged
Conversation
- share one semaphore(COPY_CONCURRENCY) across all jobs in run_jobs so multi-job directory runs cannot exhaust file descriptors: the per-job buffer_unordered bounds stacked up (measured 111 in flight with 3 jobs before the fix) and EMFILE'd on low-ulimit systems - bk run -i failures now exit 1 like every other run mode instead of EX_IOERR (74), with the same per-job id reporting as bk run - ignore an empty HBACKUP_CONFIG instead of resolving the config to a relative ./config.toml in the current directory - fix the config_dir error message naming the wrong directory - drop the unused criterion dev-dependency
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the #247 review: fixes the one Important finding plus the cheap Minors.
Changes
run_jobsnow shares oneSemaphore(COPY_CONCURRENCY)across all jobs;execute_item_asyncacquires one permit per in-flight item (copies and deletes). Before this, the per-jobbuffer_unordered(64)bounds stacked across concurrent jobs: measured 111 in-flight copies with 3 directory jobs, enough to hitEMFILEon macOS's default 256-fd soft limit. After: saturates at exactly 64.run_jobstandalone keeps its own fresh pool; compression jobs bypass the semaphore by design (fd usage O(1) per job).bk run -ifailures now exit 1 (was 74 /EX_IOERR) like every other run mode, routed throughrun_jobsfor identical per-job id reporting.HBACKUP_CONFIGis now ignored instead of resolving the config to a relative./config.tomlin the CWD; also fixed the non-macOSconfig_dir()error message naming the wrong directory.criteriondev-dependency (no bench targets; lockfile pruned ~290 lines).Tests
run_jobs_bounds_concurrent_copies_process_wideunit test measures real concurrency viacfg(test)instrumentation incopy_async(compiled out of release/integration builds): red at 111 pre-fix, green at ≤ 64 post-fix, with a lower-bound assert so it can't rot into a vacuous pass.run_single_failing_job_exits_non_zeronow pins exit code 1 + theFailed to run job with id Nmessage.empty_hbackup_config_env_falls_back_to_absolute_defaultintegration test.cargo fmt/clippy -D warningsclean; all 65 tests pass.Follow-up candidates (out of scope here)
bk run -i 1 99with a nonexistent id exits 0 today (stderr warning only) — for a data-preservation tool, a requested-but-not-run id arguably warrants a non-zero exit.