Skip to content

fix(net): resolve 1GB upload hang and macos build (Issue #1001 regression) #1035

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.

Sign up for GitHub

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

Jump to bottom
Merged
weisd merged 8 commits into from
Dec 7, 2025
Merged

fix(net): resolve 1GB upload hang and macos build (Issue #1001 regression) #1035

weisd merged 8 commits into from
Dec 7, 2025

Conversation

@Jitterx69
Copy link
Contributor

@Jitterx69 Jitterx69 commented Dec 7, 2025

This PR follows up on #1025 to fix a regression where large file uploads would hang on node power-off.

Changes:

  1. Data Plane: Enabled keepalives in rio client (TCP 10s, HTTP/2 5s).
  2. Server: Enforced SO_KEEPALIVE on incoming connections.
  3. Build: Fixed macOS/AArch64 compilation by gating profiling code.

Verified with cargo test -p rustfs-rio.

Jitterx69 and others added 8 commits December 6, 2025 16:08
@Jitterx69
fix(protos): enable app-layer keepalives (rustfs#1001)
3eca2e3 
Enables HTTP/2 keepalives and TCP keepalives in gRPC client to detect dead nodes (e.g., power loss) in ~8 seconds, preventing cluster hangs.
@weisd
Merge branch 'main' into fix/issue-1001-dead-node-detection
7d0a735
@Jitterx69
fix(net): enable data-plane and server-side keepalives
8288edc 
Enables reqwest keepalives in Rio (data stream) and TCP keepalives in Server (accepted sockets) to prevent hangs during large transfers if a peer dies.
@Jitterx69
fix(prof): fix macos/aarch64 profiling build error
40c0b7f 
Guard profiling logic with cfg(target_os=linux) and provide no-ops for other platforms to prevent compilation failures on macOS.
@Jitterx69
docs: update recovery report with data plane and build fixes
435a4ce
@Jitterx69
Merge branch 'main' into fix/issue-1001-dead-node-detection
1661394
@Jitterx69
style: fix rustfmt errors in profiling and http
e673509
@Jitterx69
style: fix clippy redundancy error in profiling cfg
94222b1
@houseme houseme requested review from Copilot and weisd December 7, 2025 09:34
Copilot AI reviewed Dec 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a critical regression from Issue #1001 where large file uploads (1GB+) would hang when a node experiences power loss. The fix implements a comprehensive keepalive strategy across both the control plane (gRPC) and data plane (HTTP/streaming), along with fixing a macOS/AArch64 build failure in the profiling module.

Key Changes:

  • Data Plane: Configured TCP and HTTP/2 keepalives in the reqwest client (10s TCP, 5s HTTP/2) to detect dead connections during large file streaming operations
  • Server Side: Enforced SO_KEEPALIVE on incoming TCP connections with aggressive timeouts (10s idle, 5s interval, 3 retries) to forcefully close dead client sockets
  • Cross-Platform Build: Gated Linux-specific profiling code (jemalloc_pprof, pprof) with #[cfg(target_os = "linux")] to resolve macOS compilation failures

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
rustfs/src/server/http.rs Adds TCP keepalive configuration (10s/5s/3) on incoming connections to detect and close dead client sockets
rustfs/src/profiling.rs Wraps all profiling code in #[cfg(target_os = "linux")] module with stub implementations for non-Linux platforms
crates/rio/src/http_reader.rs Configures HTTP client with TCP keepalive (10s), HTTP/2 keepalive (5s/3s), and connection timeout (5s) to prevent upload hangs
docs/cluster_recovery.md Documents the keepalive strategy covering control plane (gRPC), data plane (streaming), and build stability improvements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

houseme
houseme reviewed Dec 7, 2025
View reviewed changes
@weisd weisd merged commit cd6a26b into rustfs:main Dec 7, 2025
18 checks passed
@loverustfs loverustfs mentioned this pull request Dec 7, 2025
Open
@loverustfs
Copy link
Contributor

loverustfs commented Dec 7, 2025

Hey @weisd @houseme @Jitterx69 ,

The repair was unsuccessful. #1035

  1. During the upload, one server lost power, and the rustfsadmin user was unable to log in.
  2. Upload stopped.
  3. The performance page cannot be displayed.

It did not meet expectations.

image

@Jitterx69
Copy link
Contributor Author

Jitterx69 commented Dec 7, 2025

Hey @weisd @houseme @Jitterx69 ,

The repair was unsuccessful. #1035

  1. During the upload, one server lost power, and the rustfsadmin user was unable to log in.
  2. Upload stopped.
  3. The performance page cannot be displayed.

It did not meet expectations.

image

What I think is the keepalive fix helps the data plane (file transfers) fail fast than normal. Other than that :

  1. The metadata operations (login, server stats) may be blocking on the dead node because of Raft quorum, requiring priority.
  2. If connections to the dead node don't timeout properly at the gRPC/protos level, the cluster hangs.
  3. The console may be making synchronous calls to cluster metadata endpoints which is potentially clogging the dead node.
  4. If the rustfsadmin session was pinned to the failed node, login attempts would hang

Could be possible to fix via :

  1. Add dead connection eviction.
  2. Implement graceful degradation in the console.
  3. Make IAM sync, which does not blocks out the auth.
  4. Add connection eviction on error in PeerRestClient.

I will start debugging if you find my instincts well. @loverustfs

@loverustfs
Copy link
Contributor

loverustfs commented Dec 7, 2025

Hey @weisd @houseme @Jitterx69 ,
The repair was unsuccessful. #1035

  1. During the upload, one server lost power, and the rustfsadmin user was unable to log in.
  2. Upload stopped.
  3. The performance page cannot be displayed.

It did not meet expectations.
image

What I think is the keepalive fix helps the data plane (file transfers) fail fast than normal. Other than that :

  1. The metadata operations (login, server stats) may be blocking on the dead node because of Raft quorum, requiring priority.
  2. If connections to the dead node don't timeout properly at the gRPC/protos level, the cluster hangs.
  3. The console may be making synchronous calls to cluster metadata endpoints which is potentially clogging the dead node.
  4. If the rustfsadmin session was pinned to the failed node, login attempts would hang

Could be possible to fix via :

  1. Add dead connection eviction.
  2. Implement graceful degradation in the console.
  3. Make IAM sync, which does not blocks out the auth.
  4. Add connection eviction on error in PeerRestClient.

I will start debugging if you find my instincts well. @loverustfs

Okay, let's try this solution.
@Jitterx69 Thank you!

@Jitterx69 Jitterx69 mentioned this pull request Dec 8, 2025
Merged
15 tasks
Copilot AI mentioned this pull request Dec 8, 2025
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@houseme houseme houseme left review comments

Copilot code review Copilot Copilot left review comments

@weisd weisd weisd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jitterx69 @loverustfs @weisd @houseme