-
Notifications
You must be signed in to change notification settings - Fork 59
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
Increase nginx proxy_read_timeout #781
Conversation
WalkthroughThe pull request modifies the Nginx configuration template by adding a Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cloudflared/rootfs/etc/nginx/template/nginx.conf.gtpl (1)
43-43
: Consider a more moderate timeout value.While setting
proxy_read_timeout
to 1 day will prevent log duplication, it's an unusually long timeout that could potentially:
- Tie up server resources with stale connections
- Make it harder to detect actual connection issues
- Impact server performance under high load
Consider using a shorter timeout (e.g., 1 hour) and implementing a reconnection strategy on the client side:
- proxy_read_timeout 1d; + proxy_read_timeout 1h;For a more robust solution, consider:
- Implementing WebSocket for real-time log streaming
- Using Server-Sent Events (SSE) with automatic reconnection
- Adding client-side retry logic with exponential backoff
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cloudflared/rootfs/etc/nginx/template/nginx.conf.gtpl
(1 hunks)
🔇 Additional comments (1)
cloudflared/rootfs/etc/nginx/template/nginx.conf.gtpl (1)
51-51
: LGTM: Proper file formatting.The addition of a newline at the end of the file follows good file formatting practices.
I'm not sure this PR is effective to fix proxy timeout end-to-end. proxy_read_timeout for Cloudflare itself is 100s: https://developers.cloudflare.com/fundamentals/reference/connection-limits/ Without being on an enterprise plan, I haven't found a way to override this. I see timeouts for live-logs at 1.7min in the developer console, which aligns with the above. |
I'm afraid you're right, I only tested for a little over a minute but not >100 seconds. I guess we then are only fixing a problem for customers who have subscribed to the enterprise plan... |
Proposed Changes
#744 (reply in thread)
Increase timeout to 1d to prevent HA from duplicating latest log entries.
Related Issues
Summary by CodeRabbit