Skip to content

moq-stats decrement publish count on abandoned publishers also - #158

Closed
akash-a-n wants to merge 1 commit into
facebookexperimental:mainfrom
openmoq:bugfix/moq-stats-upstream
Closed

moq-stats decrement publish count on abandoned publishers also#158
akash-a-n wants to merge 1 commit into
facebookexperimental:mainfrom
openmoq:bugfix/moq-stats-upstream

Conversation

@akash-a-n

@akash-a-n akash-a-n commented May 5, 2026

Copy link
Copy Markdown
Contributor

We noticed a discrepancy in moq-stats. An abandoned publisher doesn't trigger any metric,
This change moves moq_stat's onPublishDone to deliverPublishDoneAndRemove so that if a publisher abandons, onPublishDone stat callback is triggered

Original PR in openmoq: openmoq#148

… abandoned publisher are correctly decremented
@meta-cla

meta-cla Bot commented May 5, 2026

Copy link
Copy Markdown

Hi @akash-a-n!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

Comment thread moxygen/MoQSession.cpp
<< "deliverPublishDoneAndRemove: Delivering PUBLISH_DONE to app; statusCode="
<< folly::to_underlying(pendingPublishDone_->statusCode)
<< " alias=" << alias_ << " requestID=" << requestID_;
MOQ_SUBSCRIBER_STATS(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akash-a-n Publish_Done has meaning wrt to moqt spec so moving it here tracks publish_done received on the wire incorrectly. Can we have another callback for "subscription ended"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.
I think #159 addresses it with a new callbacks

@akash-a-n

Copy link
Copy Markdown
Contributor Author

Closing this in favour of #159

@akash-a-n akash-a-n closed this May 11, 2026
@akash-a-n
akash-a-n deleted the bugfix/moq-stats-upstream branch May 11, 2026 12:41
meta-codesync Bot pushed a commit that referenced this pull request May 13, 2026
…one semantics (#159)

Summary:
- onPublishDone fires only when a PUBLISH_DONE wire frame is received/sent, not for session-close synthesized cases
- Add onSubscriptionBegin/onSubscriptionEnd to MoQStatsCallback (common base) for an active-subscription gauge that can never go negative; fires on both SUBSCRIBE and PUBLISH paths (publisher and subscriber sides)
- onSubscriptionEnd fires at every subscription termination path: subscriber: deliverPublishDoneAndRemove, unsubscribe publisher: sendPublishDone, onUnsubscribe, cleanup
- Switch stats test mocks to NiceMock to suppress uninteresting-call warnings

Co-authored by: akash-a-n

This is an alternative to #158

Pull Request resolved: #159

Reviewed By: sandarsh

Differential Revision: D104433947

Pulled By: afrind

fbshipit-source-id: cdd86716c2bc9634dd37ad55240978a0224605d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants