diff --git a/AGENTS.md b/AGENTS.md index 532551286e4..7cf56fea2e1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ Trust this checkout's source over published API references, which may describe a The API is the most important thing to get right: a bad shape costs a breaking change in every language. -- Feature PRs add migration notes under `## Unreleased` in the package's `MIGRATE.md`; only the release cut writes version sections. +- Only the release cut edits `MIGRATE.md`: a breaking PR puts its migration note in its Impact section, and the cut writes it into the package's guide. - Replace a callback parameter with a handle, an event, or a Producer/Consumer split. - Nest related API, configuration, and output fields under their shared role. - Propose alternatives when a name or shape feels awkward. diff --git a/py/moq-rs/tests/test_local.py b/py/moq-rs/tests/test_local.py index 6d0c672b8b5..f85c26f736e 100644 --- a/py/moq-rs/tests/test_local.py +++ b/py/moq-rs/tests/test_local.py @@ -1594,6 +1594,9 @@ def test_encode_audio_with_opus_object(): broadcast.close() +@pytest.mark.skip( + reason="flaky under load: the first read can stall past 5 s; see quest/m0/test-flakes-2/audio-playout-delay.md" +) async def test_audio_consumer_reports_its_playout_delay(): """A fixed playout delay is reported back as is; an estimated one moves as frames are read.""" origin = moq.OriginProducer() diff --git a/py/moq-rs/tests/test_socket.py b/py/moq-rs/tests/test_socket.py index 3821ace915b..2123ad94190 100644 --- a/py/moq-rs/tests/test_socket.py +++ b/py/moq-rs/tests/test_socket.py @@ -108,7 +108,10 @@ async def test_tcp_rst_and_endpoint_close_are_errors(): assert reset.value.details.code == 64 + 17 client, server = await tcp(a, b) a.close() - with pytest.raises(moq.Error.Cancelled): + # The stream's reset and the endpoint's retraction race, so either shutdown error can + # reach the reader. The contract promises no clean EOF here, not a cause. + shutdown: tuple[type[BaseException], ...] = (moq.Error.Cancelled, moq.Error.Closed) + with pytest.raises(shutdown): await server.consumer.read_frame() b.close() diff --git a/quest/m0/test-flakes-2/README.md b/quest/m0/test-flakes-2/README.md index 3359ece99e1..9475129619c 100644 --- a/quest/m0/test-flakes-2/README.md +++ b/quest/m0/test-flakes-2/README.md @@ -95,6 +95,12 @@ test, which failed two merge-queue batches and two loaded local runs. Added 2026-11-05 (maintainer, `/quest-audit`): Interop's P2P data-channel timeout, one child. +Added 2026-11-05 (maintainer, `/quest-complete` follow-ups): moq-net's +pool-debt flight test, the Python audio playout-delay test, and the io_uring +tests' shared locked-memory budget, one child each. On 2026-11-06, moq-ffi's +concurrent-ready test, quarantined here instead of in the unrelated +[moq-dev/moq#7981](https://github.com/moq-dev/moq/pull/7981). + Public API: none. Wire: none. ## Required @@ -106,3 +112,7 @@ Public API: none. Wire: none. - [Diagnose and re-enable the FFI datagram connection test](/quest/m0/test-flakes-2/ffi-datagram-roundtrip.md) - diagnose the default-connection timeout and re-enable the quarantined test - [Test PulseAudio servers stay off the desktop's D-Bus](/quest/m0/test-flakes-2/pulse-dbus.md) - concurrent media runs on one desktop never collide on `org.PulseAudio1` - [The P2P data-channel test transfers its frame](/quest/m0/test-flakes-2/p2p-data-channel.md) - Interop's two-peer data-channel test stops stalling after a failed send +- [moq-net's pool-debt flight test holds its bound in parallel runs](/quest/m0/test-flakes-2/pool-debt-flight.md) - `pool_debt_sheds_a_flight_without_its_cached_group` stays under `2 * POOL` when it shares a process with moq-net's other tests +- [The Python audio playout-delay test reads its frames](/quest/m0/test-flakes-2/audio-playout-delay.md) - `test_audio_consumer_reports_its_playout_delay` stops timing out on its first read in the merge queue +- [io_uring tests fit the shared locked-memory budget](/quest/m0/test-flakes-2/uring-memlock.md) - concurrent moq-relay and moq-uring test runs stop exhausting the user's `RLIMIT_MEMLOCK`, fixed at the cause rather than by raising the limit +- [The FFI concurrent-ready test wakes both calls under load](/quest/m0/test-flakes-2/ffi-concurrent-ready.md) - `concurrent_ready_calls_all_wake` stops stalling its reader drain under load and is re-enabled diff --git a/quest/m0/test-flakes-2/audio-playout-delay.md b/quest/m0/test-flakes-2/audio-playout-delay.md new file mode 100644 index 00000000000..9c27110599c --- /dev/null +++ b/quest/m0/test-flakes-2/audio-playout-delay.md @@ -0,0 +1,53 @@ +# [S] The Python audio playout-delay test reads its frames + +## Goal + +`test_audio_consumer_reports_its_playout_delay` in +`py/moq-rs/tests/test_local.py` reads both written frames from each of its +consumers in loaded runs and in the merge queue, fixed at its cause under +the [epic](/quest/m0/test-flakes-2/README.md)'s rules, never by raising its +5-second wait. Done when the PR explains the stalled read and shows the run +that reproduced it now passing. + +## Plan + +Added 2026-11-05 (maintainer, /quest-complete follow-ups), from the merge +queue entry for [moq-dev/moq#7951](https://github.com/moq-dev/moq/pull/7951) +at `04a53405db`, +[run 42145929336](https://github.com/moq-dev/moq/actions/runs/42145929336/job/127657266646) +(2026-11-06 01:51Z): `1 failed, 124 passed`. The first read timed out: + +```text +> assert isinstance(await asyncio.wait_for(anext(fixed), timeout=5.0), moq.audio.Frame) +moq-rs/tests/test_local.py:1630: +E TimeoutError +``` + +The consumer with the fixed 80 ms delay (`estimate=False`) never returned +its first frame within 5 s. The land fixer +([note](https://github.com/moq-dev/moq/pull/7951#issuecomment-6674925814)) +found the test and its implementation unchanged by the PR, and the full +Python suite passed locally (125 tests). The PR landed on requeue. No other +failed run from 2026-11-03 to 2026-11-06 shows this test, and no other PR +comment names it. + +It failed again on 2026-11-06 in the merge queue entry for +[moq-dev/moq#7904](https://github.com/moq-dev/moq/pull/7904) at `565d07a3f6` +(same TimeoutError). The land fixer found the PR unrelated and the full +Python suite passing locally, then stopped on the repeated failure. The test +is now skipped with `@pytest.mark.skip` pointing here; remove the skip with +the fix. + +Lead, unverified: the test writes only two 20 ms Opus frames (40 ms) and +doesn't finish the track until after both reads. Any path that needs more +input before the first frame goes out can stall the read for good. Candidates +are the encoder's lookahead, the decoder's warmup trim, the fixed delay +waiting for 80 ms of buffered audio, or a subscription that isn't live when +the frames are written. Which one applies depends on timing, which fits a +failure that appears only under load. + +Reproduce first: run the test repeatedly while `just check` loads the host, +and record whether the encoder published the frames and where the decoder +held them. Then fix whichever of the test or the decoder is wrong. + +Public API: none. Wire: none. diff --git a/quest/m0/test-flakes-2/ffi-concurrent-ready.md b/quest/m0/test-flakes-2/ffi-concurrent-ready.md new file mode 100644 index 00000000000..814cb07794b --- /dev/null +++ b/quest/m0/test-flakes-2/ffi-concurrent-ready.md @@ -0,0 +1,33 @@ +# [S] The FFI concurrent-ready test wakes both calls under load + +## Goal + +`concurrent_ready_calls_all_wake` in `rs/moq-ffi/src/test.rs` passes +reliably in loaded runs, fixed at its cause under the +[epic](/quest/m0/test-flakes-2/README.md)'s rules, and its `#[ignore]` and +`sh/gh/test-inventory.allow` entry are removed. Done when the PR explains +the stall and shows the loaded run that reproduced it now passing. + +## Plan + +Added 2026-11-06 (maintainer's standing rule that flakes join this epic, +`/quest-complete`). The `/quest-iterate` agent on +[moq-dev/moq#7981](https://github.com/moq-dev/moq/pull/7981) saw it time out +in a local `quest gate just ci` while draining the reader during the two +concurrent `ready` calls; it passed on unchanged `main`, and #7981 doesn't +touch moq-ffi. Quarantined in +[moq-dev/moq#8024](https://github.com/moq-dev/moq/pull/8024) with every +assertion kept. + +Lead, unverified: the drain loop reads until both parked `ready` calls have +finished, but it checks them only between reads. If the reader takes the +last of the 25 frames before both calls finish (their wakes run on the FFI +runtime, which lags under load), the next `read_frame` waits on a group that +gets no more frames and never finishes, so it times out as "the read +stalled". Check whether that is the failure before choosing between waiting +on the calls once the window has room and fixing a lost wake in the group. + +Reproduce first: run the test repeatedly with `--run-ignored all` beside +`just check --all` load under `quest gate`, and keep the failing output. + +Public API: none. Wire: none. diff --git a/quest/m0/test-flakes-2/pool-debt-flight.md b/quest/m0/test-flakes-2/pool-debt-flight.md new file mode 100644 index 00000000000..e599e4a51e6 --- /dev/null +++ b/quest/m0/test-flakes-2/pool-debt-flight.md @@ -0,0 +1,67 @@ +# [S] moq-net's pool-debt flight test holds its bound in parallel runs + +## Goal + +`pool_debt_sheds_a_flight_without_its_cached_group` in +`rs/moq-net/src/model/track.rs` keeps the pool under its `2 * POOL` bound +when it shares a process with the rest of moq-net's tests, fixed at its +cause under the [epic](/quest/m0/test-flakes-2/README.md)'s rules, without +loosening the bound. Its sibling `pool_debt_sheds_a_flight` failed beside it +in two of the runs below, so check it against the same cause. Done when the +PR explains the overshoot and shows the parallel run that reproduced it now +passing. + +## Plan + +Added 2026-11-05 (maintainer, /quest-complete follow-ups). The test landed +in [moq-dev/moq#7944](https://github.com/moq-dev/moq/pull/7944) (merged +2026-11-05 18:36Z). No CI or merge-queue run has failed it: CI runs nextest, +one process per test, and it passed there in every run checked (for example +[run 42104077314](https://github.com/moq-dev/moq/actions/runs/42104077314)). +Every failure came from land fixers running plain `cargo test`, where tests +share one process on parallel threads. Each passed alone, with +`--test-threads=1`, and under nextest: + +- [moq-dev/moq#7937](https://github.com/moq-dev/moq/pull/7937) + ([note](https://github.com/moq-dev/moq/pull/7937#issuecomment-6667877930), + 2026-11-05 19:58Z): `frame 763: 8402340 bytes held`. +- [moq-dev/moq#7670](https://github.com/moq-dev/moq/pull/7670) + ([note](https://github.com/moq-dev/moq/pull/7670#issuecomment-6668545954), + 20:30Z): both pool-debt tests failed in parallel. Then + ([note](https://github.com/moq-dev/moq/pull/7670#issuecomment-6670516495), + 21:52Z) `cargo test --locked -p moq-net -p moq-archive --all-features` + failed twice: `frame 500: 8402340 bytes held`, then + `frame 523: 8402372 bytes held`. +- [moq-dev/moq#7951](https://github.com/moq-dev/moq/pull/7951) + ([note](https://github.com/moq-dev/moq/pull/7951#issuecomment-6670771027), + 22:06Z): `frame 500: 8402340 bytes held`. It failed again in that PR's + later validation + ([note](https://github.com/moq-dev/moq/pull/7951#issuecomment-6674925814)). +- [moq-dev/moq#7988](https://github.com/moq-dev/moq/pull/7988) + ([note](https://github.com/moq-dev/moq/pull/7988#issuecomment-6671969800), + 23:12Z, and again at + [00:05Z](https://github.com/moq-dev/moq/pull/7988#issuecomment-6672860745) + and [01:07Z](https://github.com/moq-dev/moq/pull/7988#issuecomment-6673858792)): + both pool-debt tests failed in parallel, holding about 8.4 MB. + +The overshoot is always about 13.7 KB past the 8,388,608-byte bound, less +than one of the test's frames (8 MiB is about 500 of them). + +Ruled out (pullfrog on +[moq-dev/moq#8024](https://github.com/moq-dev/moq/pull/8024#discussion_r4489748136)): +the pool's 100 ms access tick. `Inner::tick` moves only in `Pool::advance` +(`rs/moq-net/src/model/cache.rs`), reached from `Pool::poll`, and the test +builds a bare `Pool::new` that it never polls, so every access stamp is the +same tick plus its boost however the threads are scheduled. + +Lead, unverified: the overshoot is a near-constant 13.7 KB, not a drifting +amount, so look at the per-write charge and shed path, at what a write +charges before the flight's debt is paid. Since the failures need parallel +threads, also rule out process-wide state shared between tests in one +binary, and any other time source the pool reads (its wall-clock epoch). + +Reproduce first: run plain `cargo test -p moq-net --all-features` (not +nextest) on parallel threads until it fails, and record the frame and bytes +held. Then fix the cause. + +Public API: none. Wire: none. diff --git a/quest/m0/test-flakes-2/uring-memlock.md b/quest/m0/test-flakes-2/uring-memlock.md new file mode 100644 index 00000000000..3baefd8b2d0 --- /dev/null +++ b/quest/m0/test-flakes-2/uring-memlock.md @@ -0,0 +1,80 @@ +# [S] io_uring tests fit the shared locked-memory budget + +## Goal + +moq-relay's and moq-uring's io_uring tests stop failing ring setup with +`ENOMEM` on the user's shared `RLIMIT_MEMLOCK` when several test runs share +one host, as they do under parallel quest sessions. The fix works at the +cause, by bounding how much locked memory the tests take at once across +invocations or by sizing their rings to fit, not by raising the limit +blindly. Done when the PR shows concurrent runs that failed before now +passing on a host with the default 8 MiB limit. + +## Plan + +Added 2026-11-05 (maintainer, /quest-complete follow-ups). + +What [moq-dev/moq#7593](https://github.com/moq-dev/moq/pull/7593) (merged +2026-11-02 17:04Z) did: it widened the `io-uring` nextest group in +`.config/nextest.toml` to every moq-relay test naming `uring` as a whole +segment, with `package(moq-uring)` already in it. It also made a refused +relay worker report its `RLIMIT_MEMLOCK` error through `Ready::wait` +instead of a timeout. It kept the group at `max-threads = 4`. Its quest +scoped out the rest on purpose: "The group caps rings only within one +nextest invocation; separate invocations still share the budget, as +`rs/moq-uring/README.md` documents, and this quest does not change that." +It gives no reason beyond that scope. A nextest group also does nothing +under plain `cargo test`, which the land fixers run. + +So the budget is still shared by: + +- separate nextest invocations from the same user (several checkouts or + sessions running `just check` at once), each allowed four ring tests; +- plain `cargo test`, which runs moq-uring's and moq-relay's ring tests in + parallel with no group at all. + +`sh/rs/nextest.sh` raises hosted CI to 64 MiB, and no CI or merge-queue run +from 2026-11-03 to 2026-11-06 failed on it. The one io_uring failure in that +window, +[run 41736459964](https://github.com/moq-dev/moq/actions/runs/41736459964)'s +`embed::uring_custom_route_and_quic`, was the socket-ownership check +[moq-dev/moq#7823](https://github.com/moq-dev/moq/pull/7823) fixed. The +failures below are local runs, on hosts with an 8 MiB limit where the +report names it, and all but the last are on bases that include #7593: + +- [moq-dev/moq#7951](https://github.com/moq-dev/moq/pull/7951) (PR body, + 2026-11-05): two `quest gate just ci` runs on `eb0c56312b` "failed only in + io_uring tests in moq-relay and moq-uring: `io_uring setup failed: Cannot + allocate memory ... RLIMIT_MEMLOCK (8192 KiB), shared by every process of + this user`". The third run passed unchanged. +- [moq-dev/moq#7375](https://github.com/moq-dev/moq/pull/7375) (PR body, + merged 2026-11-03): `quest gate just check` failed once each in + `bitrate_caps::uring_session_is_held_to_its_caps` and + `uring::tests::a_group_stopped_before_serving_joins`, both `ENOMEM` on the + 8 MiB limit. +- [moq-dev/moq#7313](https://github.com/moq-dev/moq/pull/7313) (PR body, + merged 2026-11-03): a moq-relay uring test in `just check`. Then 11 + moq-uring integration tests failed in a single + `just rs test --workspace --no-fail-fast`. One invocation stays within its + four-test group, so other processes of the same user must have held part + of the budget. +- [moq-dev/moq#7508](https://github.com/moq-dev/moq/pull/7508) (PR body, + merged 2026-11-03): `moq-uring::integration mtu::discovery_finds_a_larger_mtu` + with `Cannot allocate memory`. The PR doesn't say whether that run + included #7593. + +Reproduce first: on a host with an 8 MiB limit, run two or three +`just rs test -p moq-uring` or relay `--features io-uring` invocations at +once (and one plain `cargo test`) until a ring setup fails. Then measure what +one ring test locks. `rs/moq-uring/src/worker.rs` counts 16 bytes per CQ +entry, but the registered buffer rings (`register_buf_ring_with_flags` in +`udp.rs`) may be most of it. Integration tests keep the production +256/2048 geometry, while unit tests use 128/512. + +Choose the fix from that measurement and prefer the simplest. Options +include serializing ring tests across invocations through a user-wide lock +that both nextest and `cargo test` respect, or sizing the test rings and +buffer pools so the worst-case concurrency fits. Raise the limit only with +a measured reason, not as the fix. + +Public API: none. Wire: none. diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index f4f1fde0c14..7a44fa1c5e5 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -9455,6 +9455,7 @@ async fn ready_after_abort_is_closed() { /// Two calls waiting on the same group both wake once the reader drains, though the group /// keeps only one writer waker. #[tokio::test] +#[ignore = "flaky under load: the reader can drain every frame before both ready calls finish; see quest/m0/test-flakes-2/ffi-concurrent-ready.md"] async fn concurrent_ready_calls_all_wake() { let broadcast = MoqBroadcastProducer::new().unwrap(); let track = broadcast.publish_track("tcp".into(), None).unwrap(); diff --git a/sh/gh/test-inventory.allow b/sh/gh/test-inventory.allow index 90bd29be6e8..45e9fe71542 100644 --- a/sh/gh/test-inventory.allow +++ b/sh/gh/test-inventory.allow @@ -4,6 +4,7 @@ # Flaky under load; the quest makes it reliable, then removes its `#[ignore]`. ignore moq-ffi::test::datagrams_cross_a_default_connection /quest/m0/test-flakes-2/ffi-datagram-roundtrip.md +ignore moq-ffi::test::concurrent_ready_calls_all_wake /quest/m0/test-flakes-2/ffi-concurrent-ready.md ignore moq-ffi::test::requested_datagrams_cross_a_default_connection /quest/m0/test-flakes-2/ffi-datagram-roundtrip.md # Need a GPU, a desktop, or a device that no CI runner has.