Skip to content

[Intermediate]: Add minNodeReadmitTime and maxNodeReadmitTime getters and settersΒ #550

@rwalworth

Description

@rwalworth

🧩 Intermediate Friendly

This issue is a good fit for contributors who are already familiar with the Hiero Swift SDK and feel comfortable navigating the codebase.

Intermediate Issues often involve:

  • Exploring existing implementations
  • Understanding how different components work together
  • Making thoughtful changes that follow established patterns

The goal is to support deeper problem-solving while keeping the task clear, focused, and enjoyable to work on.

🐞 Problem Description

The Client class has no way to configure how long a node remains excluded after experiencing errors. Currently, in Sources/Hiero/Client/NodeHealth.swift, the timing is hardcoded:

/// Duration to keep circuit open before testing recovery (5 minutes)
private static let circuitOpenDurationNanos: UInt64 = TimeInterval(5 * 60).nanoseconds

Other Hiero SDKs (Java, Go, C++) provide configurable node readmit times:

  • getMinNodeReadmitTime() / setMinNodeReadmitTime() - Minimum time before reconsidering a failed node
  • getMaxNodeReadmitTime() / setMaxNodeReadmitTime() - Maximum time a node can be excluded

This allows users to tune node recovery behavior based on their network conditions and reliability requirements.

Relevant files:

  • Sources/Hiero/Client/Client.swift - Main client class
  • Sources/Hiero/Client/NodeHealth.swift - Circuit breaker with hardcoded recovery duration
  • Sources/Hiero/Client/ConsensusNetwork.swift - Uses NodeHealth for node management

πŸ’‘ Expected Outcome

Add configurable node readmit time settings:

  1. getMinNodeReadmitTime() -> TimeInterval - Returns the minimum readmit time
  2. setMinNodeReadmitTime(_ time: TimeInterval) - Sets the minimum readmit time
  3. getMaxNodeReadmitTime() -> TimeInterval - Returns the maximum readmit time
  4. setMaxNodeReadmitTime(_ time: TimeInterval) - Sets the maximum readmit time

The implementation should:

  • Replace the hardcoded circuitOpenDurationNanos with configurable values
  • minNodeReadmitTime maps to when unhealthy nodes can be reconsidered
  • maxNodeReadmitTime maps to the circuit breaker recovery period
  • Store configuration in the Client
  • Default to current behavior (reasonable defaults)

🧠 Implementation Notes

Understanding the mapping:

In the Swift SDK's circuit breaker pattern:

  • Unhealthy state: Node is temporarily excluded with exponential backoff
  • Circuit open state: Node is excluded for a fixed recovery period (currently 5 minutes)

The readmit times in other SDKs map to:

  • minNodeReadmitTime: Initial/minimum backoff before retrying a failed node
  • maxNodeReadmitTime: Maximum time before forcing a retry (circuit open duration)

Suggested approach:

  1. Add private properties to Client:

    private let _minNodeReadmitTime: NIOLockedValueBox<TimeInterval>
    private let _maxNodeReadmitTime: NIOLockedValueBox<TimeInterval>
  2. Initialize with current defaults:

    self._minNodeReadmitTime = .init(0.25)      // 250ms (current initialBackoffInterval)
    self._maxNodeReadmitTime = .init(5 * 60)    // 5 minutes (current circuitOpenDuration)
  3. Add public getter/setter methods to Client

  4. Modify NodeHealth to accept these values instead of using hardcoded constants:

    • Pass values when creating ConsensusNetwork
    • Or create a NodeHealthConfig struct
  5. Update markUnhealthy() and circuit breaker logic to use the configured values

Key considerations:

  • The circuitOpenDurationNanos constant needs to become configurable
  • Consider how this interacts with the existing backoff intervals
  • Thread safety when values are changed at runtime
  • Changes span Client.swift, NodeHealth.swift, and possibly ConsensusNetwork.swift

Reference implementations:

  • Java: Client.getMinNodeReadmitTime() / setMinNodeReadmitTime(Duration) - stored in Network
  • Go: Client.GetMinNodeReadmitTime() / SetMinNodeReadmitTime(Duration)
  • C++: Client.getMinNodeReadmitTime() / setMinNodeReadmitTime(duration)

βœ… Acceptance Criteria

To help get this change merged smoothly:

  • Four methods added: getMinNodeReadmitTime, setMinNodeReadmitTime, getMaxNodeReadmitTime, setMaxNodeReadmitTime
  • Hardcoded values in NodeHealth.swift replaced with configurable values
  • Default behavior unchanged
  • Follow existing project conventions
  • Avoid breaking public APIs
  • Include tests where appropriate
  • Pass all CI checks

πŸ“‹ Contribution Guide

To help your contribution go as smoothly as possible, we recommend following these steps:

  • Comment /assign to request the issue
  • Wait for assignment
  • Fork the repository and create a branch
  • Set up the project using the instructions in README.md
  • Make the requested changes
  • Sign each commit using -s -S
  • Push your branch and open a pull request

Read Workflow Guide for step-by-step workflow guidance.
Read README.md for setup instructions.

❗ Pull requests cannot be merged without S and s signed commits.
See the Signing Guide.

πŸ“š Additional Context or Resources

If you have questions, the community is happy to help:

https://discord.com/channels/905194001349627914/1337424839761465364

Useful files to review:

  • Sources/Hiero/Client/NodeHealth.swift - Circuit breaker implementation
  • Sources/Hiero/ExponentialBackoff.swift - Backoff algorithm
  • Sources/Hiero/Client/ConsensusNetwork.swift - How NodeHealth is used

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: lowNon-urgent tasks, nice-to-have improvements, or minor issuesscope: apiRelated to the public SDK API surfacescope: grpcRelated to gRPC/protobuf/network layerskill: intermediateRequires familiarity with the codebase structure and SDK conceptsstatus: ready for devThe issue is fully defined and ready for a contributor

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions