docs: document optional HA clustering - #11
Conversation
Server/panel 0.2.0 cluster mode was missing from the site. Co-authored-by: Cursor <cursoragent@cursor.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
📝 WalkthroughWalkthroughThe site now documents optional HA clustering across the server, panel, Docker deployment, native builds, comparison content, and a new dedicated guide. Navigation and sitemap entries now expose the clustering guide and updated release information. ChangesHA clustering documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
guides/openrtmp-vs-nginx-rtmp/index.php (1)
28-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlign the comparison metadata with the new summary.
The visible summary now includes optional HA clustering, but
$pageDescriptionat Line 4 remains unchanged.structuredData.descriptionuses$pageDescriptionat Line 11, so the page metadata and JSON-LD omit the new comparison topic. Update$pageDescriptionto include optional HA clustering.Proposed metadata update
-$pageDescription = 'Compare OpenRTMP and nginx-rtmp by architecture, deployment, APIs, stream keys, statistics, codec goals, and missing features.'; +$pageDescription = 'Compare OpenRTMP and nginx-rtmp by architecture, deployment, APIs, stream keys, statistics, codec goals, optional HA clustering, and missing features.';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@guides/openrtmp-vs-nginx-rtmp/index.php` at line 28, Update the $pageDescription metadata to mention optional HA clustering, keeping it aligned with the visible Summary callout so the page metadata and structuredData.description include this comparison topic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/index.php`:
- Around line 301-302: Update docs/index.php lines 301-302 to identify ports
1940 and 1941 as peer-only HA networking that operators must explicitly map when
clustering is enabled, rather than ports exposed by default. Update
guides/self-hosted-rtmp-server-docker/index.php line 53 to replace the
unconditional “The stack exposes” wording with conditional guidance for
clustered deployments.
In `@guides/rtmp-server-ha-clustering/index.php`:
- Around line 77-78: Update the cluster-node promotion example to use HTTPS or a
secure tunnel instead of plaintext HTTP, and explicitly state in the surrounding
guide that the bearer token must never be transmitted over unencrypted HTTP.
---
Nitpick comments:
In `@guides/openrtmp-vs-nginx-rtmp/index.php`:
- Line 28: Update the $pageDescription metadata to mention optional HA
clustering, keeping it aligned with the visible Summary callout so the page
metadata and structuredData.description include this comparison topic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cc5abbe-56c9-452c-868e-ba51ea8f9b2f
📒 Files selected for processing (9)
README.mddocs/index.phpdownload/index.phpguides/index.phpguides/openrtmp-vs-nginx-rtmp/index.phpguides/rtmp-server-ha-clustering/index.phpguides/self-hosted-rtmp-server-docker/index.phpindex.phpsitemap.xml
| <tr><td><code>1940</code></td><td>Cluster control plane (<code>CLUSTER_BIND</code>) — only when <code>CLUSTER_ENABLED=true</code></td></tr> | ||
| <tr><td><code>1941</code></td><td>Cluster media mesh (<code>CLUSTER_MEDIA_BIND</code>) — only when clustering is enabled</td></tr> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the default Docker port description.
The default deployment is standalone and does not map cluster ports. Present 1940 and 1941 as explicit peer-only HA networking that operators add when clustering is enabled.
docs/index.php#L301-L302: move these ports out of the “Ports exposed by default” list, or change the table heading and state that operators must explicitly map them for clustered nodes.guides/self-hosted-rtmp-server-docker/index.php#L53-L53: replace “The stack exposes” behavior with conditional HA network guidance.
📍 Affects 2 files
docs/index.php#L301-L302(this comment)guides/self-hosted-rtmp-server-docker/index.php#L53-L53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/index.php` around lines 301 - 302, Update docs/index.php lines 301-302
to identify ports 1940 and 1941 as peer-only HA networking that operators must
explicitly map when clustering is enabled, rather than ports exposed by default.
Update guides/self-hosted-rtmp-server-docker/index.php line 53 to replace the
unconditional “The stack exposes” wording with conditional guidance for
clustered deployments.
| <pre><code>curl -X POST http://10.0.0.1:8080/api/v1/cluster/nodes/2/promote \ | ||
| -H "Authorization: Bearer <api-token>"</code></pre> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not send the API bearer token over plaintext HTTP.
Lines 77-78 send Authorization: Bearer <api-token> to http://10.0.0.1:8080. A network observer can capture and replay this token to modify cluster membership. Run this operation through HTTPS or a secure tunnel. State this requirement in the guide.
Proposed documentation change
- curl -X POST http://10.0.0.1:8080/api/v1/cluster/nodes/2/promote \
+ # Use the HTTPS API endpoint, or run this through a secure tunnel.
+ curl -X POST https://admin.example.internal/api/v1/cluster/nodes/2/promote \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <pre><code>curl -X POST http://10.0.0.1:8080/api/v1/cluster/nodes/2/promote \ | |
| -H "Authorization: Bearer <api-token>"</code></pre> | |
| <pre><code># Use the HTTPS API endpoint, or run this through a secure tunnel. | |
| curl -X POST https://admin.example.internal/api/v1/cluster/nodes/2/promote \ | |
| -H "Authorization: Bearer <api-token>"</code></pre> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@guides/rtmp-server-ha-clustering/index.php` around lines 77 - 78, Update the
cluster-node promotion example to use HTTPS or a secure tunnel instead of
plaintext HTTP, and explicitly state in the surrounding guide that the bearer
token must never be transmitted over unencrypted HTTP.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dffee1ddc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <p>Joined nodes start as learners. After catch-up, promote to voter:</p> | ||
| <pre><code>curl -X POST http://10.0.0.1:8080/api/v1/cluster/nodes/2/promote \ | ||
| -H "Authorization: Bearer <api-token>"</code></pre> |
There was a problem hiding this comment.
If an operator follows the walkthrough with the single joiner shown, promoting node 2 creates a two-voter Raft group, whose majority remains two; losing either node therefore removes quorum rather than providing failover. The guide should require an odd, three-voter topology for HA or explicitly state that the demonstrated two-node setup cannot tolerate a voter failure.
Useful? React with 👍 / 👎.
| <pre><code>CLUSTER_ENABLED=true | ||
| CLUSTER_NODE_ID=2 | ||
| CLUSTER_JOIN=10.0.0.1:1940 | ||
| CLUSTER_SECRET=<same-secret> | ||
| LRTMP2_DB=/data/node2.db</code></pre> |
There was a problem hiding this comment.
For the documented multi-host setup, this joiner configuration supplies only the address of node 1 and never sets node 2's CLUSTER_ADVERTISE_ADDR or CLUSTER_MEDIA_ADVERTISE_ADDR. Other peers consequently have no peer-dialable control or media address for node 2 unless undeclared settings are supplied out of band, so the copy-and-paste join procedure cannot establish the bidirectional Raft/media topology described above. Add node 2's advertised addresses to this snippet.
Useful? React with 👍 / 👎.
|
|
||
| <h2 id="limits">Current limitations</h2> | ||
| <ul class="check-list"> | ||
| <li>After owner failure, publishers must reconnect to a public RTMP endpoint; automatic publisher migration is not implied.</li> |
There was a problem hiding this comment.
When operators follow the node-specific IP and port examples in this guide, a publisher configured for the failed owner's address cannot reach another node merely by reconnecting, and players connected to any failed node also lose their sockets. Raft and the media mesh do not provide client ingress failover, so the HA procedure needs a health-checked load balancer or failover DNS endpoint and should state that both publishers and players must reconnect through it.
Useful? React with 👍 / 👎.
1 participant
Summary
Test plan
/guides/rtmp-server-ha-clustering/and confirm layout/TOC on desktop and mobile/docs/#clusteranchors, feature tables, and port list (1940/1941)sitemap.xmlincludes the new guide URLMade with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit