Skip to content

MoQForwarder: add passive channel subscribers that don't gate forwarding - #185

Closed
afrind wants to merge 278 commits into
facebookexperimental:mainfrom
openmoq:pr185
Closed

MoQForwarder: add passive channel subscribers that don't gate forwarding#185
afrind wants to merge 278 commits into
facebookexperimental:mainfrom
openmoq:pr185

Conversation

@afrind

@afrind afrind commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary:
Adds a passive flag to addChannelSubscriber() for the relay's own
internal observer chain (top-N / termination / cache) attached below a
local-forwarder primary. Passive subscribers are not real downstream
consumers, so they must not influence the forwarder's "are there active
consumers" accounting:

  • They increment passiveCount_ instead of calling addForwardingSubscriber(),
    so they don't count toward forwardingSubscribers_ or block onEmpty. The
    primary's onEmpty still fires once the last real cross-exec subscriber
    leaves.
  • In beginSubgroup(), resetting a passive subscriber's stale subgroup no
    longer sets anyReset. Passive subscribers never stop_sending, so counting
    them would mask the "no active consumers" signal and prevent a duplicate
    subgroup from propagating CANCELLED back to the publisher after all real
    consumers have stop_sent.

Test Plan:


gmarzot and others added 30 commits March 6, 2026 09:32
* CI: add ASAN build job, rename openmoq-main to main in workflows

* workflows: rename to lowercase, amd64 job name, updated Slack format

- openmoq-ci.yml → pull request ci; asan debug job; test (linux/macos/asan debug) reporters
- openmoq-publish-artifacts.yml → release publish; ubuntu-22.04-amd64 job name; structured Slack notification
- openmoq-upstream-sync.yml → upstream sync
- openmoq-auto-merge-sync.yml → sync auto-merge; update workflow_run trigger to match new CI name

* ci: simplify job names to linux / macos / asan debug

* ci: merge asan debug into build matrix
* ci: rename workflow 'pull request ci' → 'cmake'

* workflows: rename 'cmake' → 'build', 'release publish' → 'publish release'
…ase (#56)

- Rename workflow files: openmoq-* → omoq-* prefix
- Rename CI workflow: build → verify
- Rename publish workflow: publish release → publish artifacts
- Rename script: create-release.sh → publish-artifacts.sh
- Fix auto-merge trigger: "pull request ci" → "verify" (broken by #52)
- Consolidate auto-merge to single job (eliminates Skipped noise)
- Standardize all Slack notifications to curl one-liners
- Add email notifications for sync-blocked and sync-CI-failure
- Standardize all email notifications with verbose details
- Rolling snapshot-latest pre-release replaces per-commit build-<sha> releases
- Remove stale openmoq-main branch from triggers
- Update openmoq/README.md for current branch/workflow/script names
* Slack notifications: bold repo, code-formatted branch and hash

Use *org/repo* for bold and backtick-wrap branch/hash for Slack mrkdwn.

* Replace CI terminology with build/verify in notifications

Drop all user-facing "CI" references from Slack and email
notifications — use "build failed" instead.

* Improve notification labels: publish release, upstream sync, verification

- publish artifacts: "run:" → "publish release:" + "artifacts:"
- sync blocked: "sync paused" → "upstream sync paused"
- sync auto-merge: "build failed" → "verification failed"

* Refine Slack notification labels to match workflow names

- publish: "run:" → "publish release:", failure adds "failed"
- sync blocked: add branch/commit, "upstream sync paused:"
- sync failure: add branch/commit, "upstream sync failed: Run"
- verification: "verification failed: PR:" (drop run link)
* Isolate Python dependency tree from native build system

Summary:
**Context:**
Thrift Python builds require different library versions and configurations than native C++ builds. `fbthrift-python` does not require `proxygen`, yet `proxygen` is included as a dependency, which makes the build unnecessarily brittle. Current manifests mix Python and C++ dependencies.

**This diff:**
- New `libaio-python` manifest for Linux-only Python builds
- `-python` manifests (`fbthrift-python`, `folly-python`, `fizz-python`, `zlib-python`, `libiberty-python`) now depend on `-python` variants instead of base manifests
- Fix `zlib-python` download URL (github instead of `zlib.net`)
- Exclude `http2_helper` test extension from `fbthrift-python` build (removes `proxygen` dependency)
- **Bonus** - Add build status badges for the python builds.

Reviewed By: yfeldblum

Differential Revision: D94714599

fbshipit-source-id: 5d23e7591b67db3baaa57d54942429162c71fbdf

* Updating hashes

Summary:
GitHub commits:

facebook/fb303@d5b606c
facebook/fbthrift@9536113
facebook/folly@a6dcc83
facebook/hermes@93d758c
facebook/mvfst@d377900
facebook/proxygen@39cd8b3
facebook/wangle@76f5a9b
react/yoga@672f056
facebookexperimental/edencommon@8e64864
facebookexperimental/rust-shed@c49d788
facebookincubator/fizz@e31bc89
WhatsApp/eqwalizer@c75115d

Reviewed By: sdwilsh

fbshipit-source-id: 58377d69018220b96cbaf8bffc4f889f7584e70e

---------

Co-authored-by: Satish Kumar <satishvk@meta.com>
Co-authored-by: Open Source Bot <generatedunixname499836121@meta.com>
BUILD_SAMPLES was forced OFF during FetchContent to prevent dependency
targets from building their samples, but was never restored, so moxygen's
own sample executables were not built.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Create picoquic getdeps manifest with FETCH_PTLS enabled
- Add picoquic as dependency in moxygen manifest
Implement PicoQuicExecutor to integrate task execution and timer management
 with picoquic's packet loop. This executor enables async operations and
scheduled timeouts within the picoquic event loop.

Key components:
- Task queue with mutex-protected access
- Priority queue for timer management with cancellation support
- Packet loop callbacks to drain tasks and process expired timers
- Wake-up mechanism to set epoll timeout to 0 when work is pending
Implement WebTransport interface using picoquic's just-in-time (JIT) APIs.
This implementation uses mark_active_stream/mark_active_datagram to signal
readiness and prepare callbacks to send data when picoquic is ready.

Key features:
- JIT stream sending with prepare_to_send callback
- JIT datagram sending with prepare_datagram callback
- WtStreamManager integration for stream lifecycle management
- Bidirectional and unidirectional stream support
- Datagram queue for buffering outgoing datagrams
Implement MoQPicoQuicServer as a picoquic-based alternative to MoQServer.
This server implementation uses picoquic's network thread API and supports
multiple MoQT ALPN protocols with automatic negotiation.

Key features:
- Inherits from MoQServerBase for common server functionality
- ALPN negotiation with support for multiple MoQT protocol versions
- Picoquic network thread for async connection handling
- Shared PicoQuicExecutor for all sessions
- Per-connection context management with magic number validation
Standalone relay binary using MoQPicoQuicServer with signal-based
shutdown. Gated behind BUILD_SAMPLES.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Picks up the picotls fix for spurious ( ) in CORE_EXTRA_LIBS_DIRS
that caused CMake configure errors when brotli is installed.
Adds a build-pico job to the verify workflow that builds with
-DBUILD_PICOQUIC=ON to validate the PicoQuic standalone build
on Linux and macOS.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Static linking fixes, sync conflict auto-resolution, snapshot-latest draft fix

- BUNDLE_DEPS: add GFLAGS_SHARED=OFF and Boost_USE_STATIC_LIBS for
  CONFIG-mode packages that ignore CMAKE_FIND_LIBRARY_SUFFIXES
- BUNDLE_DEPS: use shared libunwind (system .a is not -fPIC on Ubuntu)
- upstream-sync: auto-resolve github_hashes conflicts with -X ours,
  notify Slack/email on unresolvable conflicts
- publish-artifacts: fix snapshot-latest release stuck as draft

* Fix BUNDLE_DEPS: Linux-only .a suffix, skip libunwind for bundled builds

* Keep libunwind .so for BUNDLE_DEPS: find with shared preference, skip folly re-find

* Fix glog transitive libunwind.a PIC failure on x86_64

FindGlog.cmake internally calls find_package(LibUnwind), which with
CMAKE_FIND_LIBRARY_SUFFIXES=".a" (set by BUNDLE_DEPS) picks up
libunwind.a. On Debian/Ubuntu x86_64, that file is not compiled with
-fPIC, causing PIE link failure. Patch glog::glog's link interface
to use libunwind.so instead.

* Make GFLAGS_SHARED OFF Linux-only to fix macOS dual linkage

On macOS, glog is always shared (homebrew has no libglog.a).
glog.dylib links gflags.dylib, so using static gflags (GFLAGS_SHARED
OFF) causes dual-linkage crashes. Keep gflags shared on macOS to
match glog.
Replace omoq-verify.yml and omoq-publish-artifacts.yml with a single
omoq-ci-pipeline.yml that runs the full pipeline: verify (build/test),
publish (BUNDLE_DEPS artifact builds), release (snapshot-latest), and
aggregate notifications — all as conditional jobs within one workflow.

On PRs: only build + build-pico jobs run.
On main push: build → publish → release → notify, with build-pico in
parallel. Release gates on all of build, build-pico, and publish.

This eliminates the problem where publish ran independently of verify
and could publish broken artifacts.

Add omoq-version-release.yml for manual versioned releases via
workflow_dispatch with a version input — promotes snapshot-latest
artifacts to a tagged non-prerelease.

Update sync auto-merge trigger to match renamed workflow ("ci pipeline").

Add upstream CI failure notification to upstream sync: when all scanned
commits have failing GitHub Actions, sends Slack + email alert.
…NAPSHOT_SHA guard

- Remove workflow_dispatch from ci-pr and ci-main (only needed for version-release)
- Set cancel-in-progress: false on ci-main (avoid cancelling mid-publish)
- Add contents: read to ci-pr permissions (explicit least-privilege)
- Guard empty SNAPSHOT_SHA in version-release
Consolidate verify + publish into ci-pipeline (#69)
omoq-sync-bot Bot and others added 26 commits May 24, 2026 07:45
sync: openmoq/picoquic pin to 5b873b6c8d63
…enMOQ-only tests

The sync merge replaced the openmoq MoQForwarderTest.cpp with the upstream
version, losing isPinned() from Subscriber and dropping openmoq-only tests.
Restore isPinned(), remove the orphaned DummyExecutor from MoQForwarderTest.cpp,
and collect all openmoq-only tests (passive subscriber, channel subscriber) plus
a new isPinned coverage test into OpenMOQForwarderTest.cpp.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sync: openmoq/picoquic pin to 076a21f43e77
The upstream sync merge (fd4bdee) resolved a conflict in the parse/write
region toward upstream, deleting the local parseTrackFilter definition and
the writeTrackFilter forward declaration while keeping their call sites.
This broke the build with 'not declared in this scope' for both symbols.

Restore parseTrackFilter (before parseVariableParam) and the writeTrackFilter
forward declaration, verbatim from main.
kAutoRequestID is a sentinel RequestID (max uint64) that callers can pass
in a JoiningFetch to let the session find the matching subscribe by track
name rather than requiring the caller to track the request ID manually.

resolveJoiningFetch handles both the auto case (searches pendingRequests_
and subTracks_ by FullTrackName, writes back the resolved ID) and the
explicit case (existing lookup + validation), replacing the inline block
in MoQSession::fetch with a single call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary:
Adds a `passive` flag to addChannelSubscriber() for the relay's own
internal observer chain (top-N / termination / cache) attached below a
local-forwarder primary. Passive subscribers are not real downstream
consumers, so they must not influence the forwarder's "are there active
consumers" accounting:

- They increment passiveCount_ instead of calling addForwardingSubscriber(),
  so they don't count toward forwardingSubscribers_ or block onEmpty. The
  primary's onEmpty still fires once the last real cross-exec subscriber
  leaves.
- In beginSubgroup(), resetting a passive subscriber's stale subgroup no
  longer sets anyReset. Passive subscribers never stop_sending, so counting
  them would mask the "no active consumers" signal and prevent a duplicate
  subgroup from propagating CANCELLED back to the publisher after all real
  consumers have stop_sent.

Test Plan:
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 1, 2026
@afrind

afrind commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Wrong repo

@afrind afrind closed this Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants