Skip to content

fix: disable throttled error#221

Merged
heilhead merged 2 commits into
mainfrom
fix/disable-throttled-error
Jun 12, 2025
Merged

fix: disable throttled error#221
heilhead merged 2 commits into
mainfrom
fix/disable-throttled-error

Conversation

@heilhead

Copy link
Copy Markdown
Contributor

Description

This disables the 'throttled' error, as it's possible that it can eat a lot of memory during system stall.

How Has This Been Tested?

Not tested.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@heilhead heilhead requested review from Copilot and xDarksome June 12, 2025 15:48

Copilot AI left a comment

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.

Pull Request Overview

This PR disables the throttled error handling to prevent potential memory issues during system stalls. Key changes include:

  • Removal of throttled error handling logic in the QUIC server.
  • Elimination of the THROTTLED constant in the RPC error definition.
  • Simplification of the pulse monitor's run function signature by removing the unnecessary mutable binding.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/rpc/src/quic/server.rs Removed throttled error handling logic in connections.
crates/rpc/src/lib.rs Removed the THROTTLED constant from the Error struct.
crates/pulse_monitor/src/lib.rs Adjusted run function signature for clarity.
Comments suppressed due to low confidence (2)

crates/rpc/src/quic/server.rs:220

  • Consider adding a comment explaining the decision to skip sending the throttled error to improve future maintainability.
continue;

crates/rpc/src/lib.rs:214

  • Ensure that all references to the THROTTLED constant are updated or removed from documentation and other parts of the codebase, as it's no longer used.
const THROTTLED: Self = Self::new(error_code::THROTTLED);

// Note: Requires a stream that would immediately yield the initial cluster
// state.
pub async fn run<C>(mut cluster_stream: C)
pub async fn run<C>(cluster_stream: C)

Copilot AI Jun 12, 2025

Copy link

Choose a reason for hiding this comment

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

Removing the mutable binding for cluster_stream improves clarity; verify that mutable access is not required within the function.

Copilot uses AI. Check for mistakes.
@heilhead heilhead merged commit 05be133 into main Jun 12, 2025
12 checks passed
@heilhead heilhead deleted the fix/disable-throttled-error branch June 12, 2025 16:32
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.

4 participants