Commit badd9c9
feat(core): shape upload I/O with the io_priority setting (#176)
Follow-up to #170. Max: "i want the user-visible win on all uploads of
nontrivial duration, not just the narrow case of bundled uploads."
## The problem #170 left behind
#170 could only apply `io_priority` at one site - the `build_bundle`
`spawn_blocking` closure - because a `PriorityGuard` is per-thread and
only
sound where the thread cannot yield. The large-file upload path was
unreachable
by that lever: its reads go through `tokio::fs`, which hands every read
to an
anonymous thread in tokio's **shared blocking pool**. Driven neither
owns those
threads nor may demote them (they also serve UI/IPC work), so no amount
of
thread-priority plumbing reaches the bytes coming off the disk during an
upload.
## The fix: a second, per-HANDLE lever
On Windows, `SetFileInformationByHandle(FileIoPriorityHintInfo)`
attaches an I/O
priority hint to the **file handle**. Every read on that handle is then
shaped
regardless of which thread issues it - which sidesteps the
anonymous-pool
problem completely. It also needs no restore, unlike a pooled thread:
the handle
is opened for one upload and closed when it ends, so there is nothing
left
behind to leak a demotion into.
New `priority::apply_to_file_handle(&std::fs::File, WorkPriority)`:
| level | hint |
|---|---|
| `Low` | `IoPriorityHintLow` |
| `Idle` | `IoPriorityHintVeryLow` (what Windows itself uses for
background I/O) |
Best-effort like the rest of the module: a refused call logs at `debug`
and the
reads run at normal priority.
## One call site covers every upload read
`executor::open_shared` is the single choke point for every source-file
read in
the executor, so threading `WorkPriority` into it covers all seven
production
paths from one place:
- the plain live open (the normal upload path),
- the `vss_mode = always` probe open,
- the `FallbackDecision::OpenLive` re-open,
- the VSS **snapshot** open (locked-file reads),
- the resumable-session identity re-check,
- the reconcile re-hash (`rehash_local_plaintext`).
The hint is applied while the handle is still a plain `std::fs::File`,
before
`tokio::fs::File::from_std` wraps it.
## The fact this PR hinged on, verified empirically
The MS docs say the handle "must be opened with the appropriate
permissions" but
never state what those are for this class. If it needed
`FILE_WRITE_ATTRIBUTES`,
the call would fail `ERROR_ACCESS_DENIED` on the executor's `read(true)`
handle
and the whole feature would silently no-op - or force us to widen the
access
mask, changing the open's sharing/locking behaviour, which is
load-bearing for
the VSS locked-file logic.
So I probed it directly against a handle opened exactly the way
`open_shared`
opens one (`read(true)` + `FILE_SHARE_READ | WRITE | DELETE`): all three
hint
values succeed, at both `sizeof(struct)` and 8-byte buffer sizes. **A
read-only
handle is sufficient, and the access mask is unchanged.** That result is
now
locked in by a `cfg(windows)` test asserting the raw call succeeds.
The struct is `#[repr(C, align(8))]` per the documented LONGLONG
alignment
requirement.
## What this PR deliberately does NOT do
- **No restructure of the reader stage.** Routing reads through an owned
blocking thread would give cross-platform shaping, but `file` is
borrowed by
the caller across `fstat_identity`, `stream_upload`, and the post-upload
identity rechecks, and the pacer's `permit_bytes` is async. Moving
ownership
into a blocking task ripples straight through the bounded-memory
backpressure
(`mem_gauge`), the resumable-session persistence (P1-3), the
throughput-probe
accounting, and the `ChangedDuringUpload` defences. Not worth it for a
scheduling hint.
- **No per-chunk guards in `cpu_stage` / `read_hash_encrypt`.** These
interleave
`.await`s, so a guard would have to be taken and dropped around each
64-KiB
burst: two syscalls per chunk inside the AEAD framing loop, the most
data-safety-critical code in the repo. And for files at or above
`RAYON_HASH_THRESHOLD` (100 MiB) - exactly the "uploads of nontrivial
duration" this PR targets - `update_rayon` fans most of that CPU out to
rayon's global pool, which the calling thread's priority does not touch.
Poor
trade on both sides.
Nothing in the upload pipeline's control flow, error handling, or byte
handling
changed: the diff is the extra parameter, one hint call, and docs/tests.
## Platform reality - read before testing
**On Linux and macOS this PR changes nothing about upload I/O.** Neither
has a
per-descriptor I/O priority; both scope it to the thread, and the reads
land on
tokio's shared pool. Testing the setting on those platforms and
concluding it is
broken would be a false negative. Windows is where the win is.
**On Windows, test with large files, or at `idle`.** There is an
asymmetry at
the `low` level worth knowing before measuring. Large-file reads are
shaped by
the new handle hint at both levels. Bundled small-file reads still go
through
`build_bundle`'s own `std::fs` opens inside the *thread* guard from #170
- and
on Windows `low` maps to `THREAD_PRIORITY_BELOW_NORMAL`, which is CPU
only
(`idle` maps to `THREAD_MODE_BACKGROUND_BEGIN`, which does cover I/O).
So at
`low`, a backup of many small files still reads at normal I/O priority.
Pointing
Driven at a folder of tiny files and seeing no I/O change is expected,
not a
bug. Closing that gap means hinting the handles `build_bundle` opens,
which is a
separate change in `bundle.rs` - worth a follow-up, out of scope here.
## Tests
- `file_handle_hint_accepts_every_level_on_a_read_only_handle` - all
three
levels on an upload-style handle, every platform (off Windows the no-op
must
also not panic).
- `file_handle_stays_readable_after_the_hint` - reads back the exact
bytes after
hinting, so a regression that invalidated the handle cannot pass as a
scheduling tweak.
- `windows_file_handle_hint_call_succeeds_on_a_read_only_handle` -
asserts the
raw `SetFileInformationByHandle` return, which is what proves the
feature is
not silently no-opping in production. No elevation needed; `tempfile`
for temp
dirs.
## Gates
- `cargo fmt --all -- --check` clean
- `cargo clippy --workspace --all-targets -- -D warnings` clean
- `cargo test -p driven-core`: 431 passed, 0 failed (14 in `priority`)
- `cargo test -p driven-app`: 295 passed, 0 failed; `driven-chaos`: 44
passed
- LF endings, ASCII dashes only. No `ui/` changes.
Cross-target `clippy -D warnings` on
`x86_64`/`aarch64-unknown-linux-gnu`,
`x86_64-unknown-linux-musl`, and `x86_64`/`aarch64-apple-darwin` (a full
cross
`cargo check` is blocked by `ring`'s build script, so the module is
checked in
isolation). That caught a real break: with the `cfg(windows)` block
compiled
out, the early `return` in `apply_to_file_handle` became a
`needless_return`
error on every unix target. Fixed before pushing; CI's ubuntu/macos legs
are
the confirmation.
## Docs
`design/DESIGN.md` s11.2 gains the per-handle lever alongside the
thread-scoped
ones, including why `SetPriorityClass` is still not used (process-wide,
would
drag the UI/IPC threads down) and what remains unshaped on unix.
Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01JLB3E2Jm7knNJd37fVpH8X
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>1 parent e6c2c3e commit badd9c9
3 files changed
Lines changed: 262 additions & 25 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1272 | 1272 | | |
1273 | 1273 | | |
1274 | 1274 | | |
1275 | | - | |
| 1275 | + | |
1276 | 1276 | | |
1277 | 1277 | | |
1278 | 1278 | | |
| |||
1297 | 1297 | | |
1298 | 1298 | | |
1299 | 1299 | | |
1300 | | - | |
| 1300 | + | |
1301 | 1301 | | |
1302 | 1302 | | |
1303 | 1303 | | |
| |||
1309 | 1309 | | |
1310 | 1310 | | |
1311 | 1311 | | |
1312 | | - | |
| 1312 | + | |
1313 | 1313 | | |
1314 | 1314 | | |
1315 | 1315 | | |
| |||
1345 | 1345 | | |
1346 | 1346 | | |
1347 | 1347 | | |
1348 | | - | |
| 1348 | + | |
1349 | 1349 | | |
1350 | 1350 | | |
1351 | 1351 | | |
| |||
1362 | 1362 | | |
1363 | 1363 | | |
1364 | 1364 | | |
1365 | | - | |
| 1365 | + | |
1366 | 1366 | | |
1367 | 1367 | | |
1368 | 1368 | | |
| |||
4934 | 4934 | | |
4935 | 4935 | | |
4936 | 4936 | | |
4937 | | - | |
| 4937 | + | |
4938 | 4938 | | |
4939 | 4939 | | |
4940 | 4940 | | |
| |||
5275 | 5275 | | |
5276 | 5276 | | |
5277 | 5277 | | |
5278 | | - | |
| 5278 | + | |
5279 | 5279 | | |
5280 | 5280 | | |
5281 | 5281 | | |
| |||
6479 | 6479 | | |
6480 | 6480 | | |
6481 | 6481 | | |
6482 | | - | |
| 6482 | + | |
| 6483 | + | |
| 6484 | + | |
| 6485 | + | |
| 6486 | + | |
| 6487 | + | |
| 6488 | + | |
| 6489 | + | |
6483 | 6490 | | |
6484 | 6491 | | |
6485 | 6492 | | |
| |||
6488 | 6495 | | |
6489 | 6496 | | |
6490 | 6497 | | |
6491 | | - | |
| 6498 | + | |
| 6499 | + | |
| 6500 | + | |
| 6501 | + | |
| 6502 | + | |
| 6503 | + | |
| 6504 | + | |
| 6505 | + | |
| 6506 | + | |
| 6507 | + | |
| 6508 | + | |
| 6509 | + | |
6492 | 6510 | | |
6493 | 6511 | | |
6494 | 6512 | | |
6495 | 6513 | | |
6496 | 6514 | | |
| 6515 | + | |
| 6516 | + | |
| 6517 | + | |
| 6518 | + | |
| 6519 | + | |
6497 | 6520 | | |
6498 | 6521 | | |
6499 | 6522 | | |
| |||
11325 | 11348 | | |
11326 | 11349 | | |
11327 | 11350 | | |
11328 | | - | |
| 11351 | + | |
11329 | 11352 | | |
11330 | 11353 | | |
11331 | 11354 | | |
| |||
11383 | 11406 | | |
11384 | 11407 | | |
11385 | 11408 | | |
11386 | | - | |
| 11409 | + | |
11387 | 11410 | | |
11388 | 11411 | | |
11389 | 11412 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | | - | |
45 | | - | |
46 | | - | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
47 | 52 | | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
57 | 75 | | |
58 | 76 | | |
59 | 77 | | |
| |||
293 | 311 | | |
294 | 312 | | |
295 | 313 | | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
296 | 352 | | |
297 | 353 | | |
298 | 354 | | |
| |||
362 | 418 | | |
363 | 419 | | |
364 | 420 | | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
365 | 488 | | |
366 | 489 | | |
367 | 490 | | |
| |||
712 | 835 | | |
713 | 836 | | |
714 | 837 | | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
| 877 | + | |
| 878 | + | |
| 879 | + | |
| 880 | + | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
715 | 910 | | |
716 | 911 | | |
717 | 912 | | |
| |||
0 commit comments