-
-
Notifications
You must be signed in to change notification settings - Fork 248
quest: plan GPU pool reservations and the io_uring handshake flush #3880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # [S] GPU frame pool back-pressure is a reservation, not an error | ||
|
|
||
| ## Goal | ||
|
|
||
| A caller feeding imported Vulkan frames through `moq_video::frame::cuda::Converter` | ||
| can drop a frame when the bounded GPU pool is full without matching an error. | ||
| Exhaustion is expected back-pressure; only real failures are errors. | ||
|
|
||
| ## Plan | ||
|
|
||
| `Converter::convert` and `cuda::Frame::resize` take a pooled buffer and report | ||
| a full pool as `Error::Unsupported` with a prose message (#3869), so the CARLA | ||
| bridge in moq.pro can only drop-and-continue by matching the text. | ||
|
|
||
| Split the reservation from the work, the way the bandwidth allocator hands out | ||
| a `Reservation`: `Converter::reserve() -> Option<Slot>` returns `None` when | ||
| every buffer is live, and `Slot::convert(&vulkan::Frame) -> Result<Frame, Error>` | ||
| does the GPU work on the held buffer, failing only for a genuine error. The | ||
| slot returns its buffer to the pool on drop, converted or not. Apply the same | ||
| shape to the resize pool. Keep the pool itself crate-private and its capacity | ||
|
Comment on lines
+19
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,180p' quest/next/gpu-pool-reservation.mdRepository: moq-dev/moq Length of output: 1777 🤖 get_repo_knowledge executed:
Length of output: 10602 Define ownership transfer after successful 🤖 Prompt for AI Agents |
||
| bound unchanged. | ||
|
|
||
| Tests on the injected allocator: `reserve` yields exactly `capacity` slots and | ||
| then `None`, dropping an unconverted slot frees it, and a failed conversion | ||
| does not leak the buffer. Update the `just rs vulkan-cuda` hardware test and | ||
| `doc/lib/rs/moq-video.md` inline. | ||
|
|
||
| Public API: `moq-video` 0.0.x, breaking for `Converter::convert` callers. Wire: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- quest spec ---'
cat -n quest/next/gpu-pool-reservation.md
printf '%s\n' '--- candidate resize declarations/usages ---'
rg -n -C 3 'Frame::resize|fn resize|resize\s*\(' --glob '!target/**' --glob '!node_modules/**' .Repository: moq-dev/moq Length of output: 41999 🏁 Script executed: set -e
printf '%s\n' '--- quest spec ---'
cat -n quest/next/gpu-pool-reservation.md
printf '%s\n' '--- candidate resize declarations/usages ---'
rg -n -C 3 'Frame::resize|fn resize|resize\s*\(' --glob '!target/**' --glob '!node_modules/**' .Repository: moq-dev/moq Length of output: 41852 🤖 get_repo_knowledge executed:
Length of output: 2592 🏁 Script executed: set -e
find . -path './.git' -prune -o -type f -print | sort | sed -n '1,240p'Repository: moq-dev/moq Length of output: 7268 Include
🤖 Prompt for AI Agents |
||
| none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [GPU conversion and NVENC](/quest/main/video-gpu-encode.md) - the converter and pool this reshapes | ||
| - [Bandwidth allocator](/quest/next/2848-follow-the-bandwidth-grant-in-moq-audio-instead-of.md) - the reservation-handle precedent | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # [M] An io_uring dial resolves once its last handshake flight is sent | ||
|
|
||
| ## Goal | ||
|
|
||
| `moq_uring::quic::client::connect` hands back a connection only after the | ||
| handshake's final flight has been written to the socket, so a worker that | ||
| stops right after the dial resolves cannot strand a peer that is still waiting | ||
| for it. A worker that stops before that point fails the dial loudly. No new | ||
| public API or wire format. | ||
|
|
||
| ## Plan | ||
|
|
||
| Today the dial resolves when the handshake completes locally, while the | ||
| client's last flight is still queued behind pacing on the worker (#3865). If | ||
| the worker stops in that window nothing is sent, the server times the | ||
| connection out, and the client holds a connection that looks established. The | ||
| behaviour is documented but easy to hit from a short-lived task. | ||
|
|
||
| Reproduce it first on Linux CI: dial, stop the worker immediately, and show | ||
| the server never accepts. Then keep the dial future pending until the driver | ||
|
Comment on lines
+19
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,80p' quest/next/uring-handshake-flush.md
sed -n '140,185p' rs/moq-uring/src/quic/noq/endpoint.rs
sed -n '45,70p' rs/moq-uring/src/quic/client.rsRepository: moq-dev/moq Length of output: 3855 Await The goal describes stopping after the dial resolves, but the reproduction step only says “dial, stop the worker immediately.” Await 🤖 Prompt for AI Agents |
||
| reports the handshake flight flushed, and return `quic::Error` when the worker | ||
| stops before then. The flush belongs in moq-uring's connect and establish | ||
| path; moq-tokio is unaffected. Do not add a timeout or a retry, and do not | ||
| drain sends from worker shutdown. | ||
|
|
||
| Linux CI covers the stopped-worker dial (now refused or fully sent, never | ||
| stranded), a normal dial still resolving promptly, and a paced flight on a | ||
| slow link. Update the moq-uring docs where the window was described. | ||
|
|
||
| Public API: none. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [io_uring handshake cancellation](/quest/next/uring-handshake-cancel.md) - the same establish path, for a dropped dial | ||
| - [uring identity](https://github.com/moq-dev/moq/pull/3865) - where the window was found | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify what the caller drops when
reserve()returnsNone.Noneis the absence of aSlot. It is not a reservation that can be dropped. State that the caller drops or skips the input frame when no slot is available.Suggested wording
📝 Committable suggestion
🤖 Prompt for AI Agents