Skip to content

Conversation

@derektriley
Copy link
Contributor

Description:
This pull request refactors the block node connection management logic to improve how connections are cleaned up and rescheduled, especially during stream resets and forced switches. The changes remove redundant cleanup methods, introduce a configurable delay for rescheduling connections after a forced switch, and update related tests to reflect the new behavior. The code now exposes connection state for easier management and testing.

Connection Cleanup and Rescheduling Improvements

  • Removed the connectionResetsTheStream and removeConnectionAndClearActive methods from BlockNodeConnectionManager, consolidating connection cleanup logic and making rescheduling more explicit. Stream resets now directly trigger selection of a new block node for streaming.
  • When closing a connection, the connection map is now updated directly from BlockNodeConnection

Forced Switch Handling

  • Added a new configuration property forcedSwitchRescheduleDelay to BlockNodeConnectionConfig, allowing the delay for rescheduling a closed active connection after a forced switch to be set via configuration.
  • Implemented logic to reschedule the previously active connection with the configured delay when a forced switch occurs.

Testing

  • Updated and removed obsolete tests for the old stream reset and cleanup logic, and added new tests for forced switch rescheduling, ensuring the new behavior is correctly validated.

Related issue(s):

Fixes #21734

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Derek Riley <[email protected]>
Signed-off-by: Derek Riley <[email protected]>
Signed-off-by: Derek Riley <[email protected]>
Signed-off-by: Derek Riley <[email protected]>
@derektriley derektriley added this to the v0.68 milestone Oct 22, 2025
@derektriley derektriley self-assigned this Oct 22, 2025
@derektriley derektriley requested review from a team as code owners October 22, 2025 19:25
@lfdt-bot
Copy link

lfdt-bot commented Oct 22, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...cks/impl/streaming/BlockNodeConnectionManager.java 81.25% 3 Missing ⚠️

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #21803      +/-   ##
============================================
- Coverage     71.81%   71.57%   -0.25%     
+ Complexity    24549    24487      -62     
============================================
  Files          2670     2675       +5     
  Lines        103887   103963      +76     
  Branches      10864    10881      +17     
============================================
- Hits          74608    74410     -198     
- Misses        25235    25489     +254     
- Partials       4044     4064      +20     
Files with missing lines Coverage Δ Complexity Δ
...app/blocks/impl/streaming/BlockNodeConnection.java 86.00% <100.00%> (ø) 71.00 <0.00> (ø)
...ra/node/config/data/BlockNodeConnectionConfig.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...cks/impl/streaming/BlockNodeConnectionManager.java 78.85% <81.25%> (-0.32%) 57.00 <2.00> (-3.00)

... and 143 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codacy-production
Copy link

codacy-production bot commented Oct 22, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.26% (target: -1.00%) 83.33%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (67d2375) 103792 78607 75.74%
Head commit (0e9828e) 103696 (-96) 78266 (-341) 75.48% (-0.26%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#21803) 18 15 83.33%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

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.

BlockBufferService force switching connections issue

2 participants