Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Change the default keepalive timeout#675

Merged
NickCao merged 2 commits intojumpstarter-dev:mainfrom
mangelajo:fix-grpc-timeout-default
Oct 1, 2025
Merged

Change the default keepalive timeout#675
NickCao merged 2 commits intojumpstarter-dev:mainfrom
mangelajo:fix-grpc-timeout-default

Conversation

@mangelajo
Copy link
Copy Markdown
Member

@mangelajo mangelajo commented Oct 1, 2025

The default timeout of 5s is excessively low and causes issues under pressure.

See https://github.com/jumpstarter-dev/jumpstarter/issues/674 for more details.

Fixes-Issue: jumpstarter-dev/jumpstarter#118

Summary by CodeRabbit

  • Chores
    • Updated default connection keepalive timing to use a significantly longer timeout.
    • Users may notice fewer transient reconnects during idle periods and more stable long-running sessions.
    • This change also reduces background keepalive traffic, which can be beneficial on constrained networks.
    • No action is required; the improvement applies automatically after upgrade.

The default timeout of 5s is excessively low and causes
issues under pressure.

See https://github.com/jumpstarter-dev/jumpstarter/issues/674
for more details.

Fixes-Issue: #674
@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 1, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit 59a8d1a
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/68dd2178eba60400080b3d08
😎 Deploy Preview https://deploy-preview-675--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 1, 2025

Walkthrough

Updated the default gRPC keepalive timeout in packages/jumpstarter/jumpstarter/common/grpc.py by changing keepalive_timeout_ms from 5000 to 180000 within _override_default_grpc_options. No other defaults or control flow were modified.

Changes

Cohort / File(s) Summary
gRPC defaults
packages/jumpstarter/jumpstarter/common/grpc.py
Increased default gRPC option keepalive_timeout_ms from 5000 to 180000 in _override_default_grpc_options; all other defaults unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • kirkbrauer
  • bennyz
  • NickCao

Poem

A bunny taps the timeout drum—tick, tock, long and sweet,
From five quick hops to minutes three, a steadier heartbeat.
In gRPC burrows, pings now linger, calm and bright,
Connections nestle warmly through the network’s gentle night.
🐇⏱️✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title succinctly and accurately describes the primary change in the changeset, namely updating the default keepalive timeout value, and does so without extraneous wording or ambiguity, making it easy for reviewers to understand the focus of the PR.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64390f8 and 5d73b9d.

📒 Files selected for processing (1)
  • packages/jumpstarter/jumpstarter/common/grpc.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: e2e
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (1)
packages/jumpstarter/jumpstarter/common/grpc.py (1)

61-61: LGTM! The increased timeout should resolve pressure-related issues.

The change from 5s to 180s (3 minutes) for keepalive_timeout_ms is reasonable given the PR's stated problem. With keepalive_time_ms at 20s, the client sends pings every 20 seconds, and 180s gives ample time for a response even under load or network congestion. The original 5s timeout was indeed quite aggressive and could cause premature connection termination during temporary spikes.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@NickCao NickCao merged commit bfea127 into jumpstarter-dev:main Oct 1, 2025
20 of 22 checks passed
@jumpstarter-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for release-0.7:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants