Skip to content

fix(server): LocalOrdererManager removeOrderer to no-op#24442

Merged
znewton merged 1 commit intomicrosoft:mainfrom
znewton:server/t9s-connection-bug
Apr 24, 2025
Merged

fix(server): LocalOrdererManager removeOrderer to no-op#24442
znewton merged 1 commit intomicrosoft:mainfrom
znewton:server/t9s-connection-bug

Conversation

@znewton
Copy link
Copy Markdown
Contributor

@znewton znewton commented Apr 23, 2025

Description

Fixes AB#35445.

LocalOrdererManager handles connection cleanup differently from Nexus' OrdererManager. #21528 incorrectly caused disconnect socket connection to sever all current orderer connections.

Note on memory use

I had a much longer PR in progress to share logic between the Nexus OrdererManager and LocalOrdererManager, but it got way too complicated, and really the number of connections held by T9s shouldn't apply memory pressure like it would in Nexus

Copilot AI review requested due to automatic review settings April 23, 2025 22:00
Copy link
Copy Markdown
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 AB#35445 by updating the LocalOrdererManager's removeOrderer function to become a no-op, ensuring that orderer connections in the local environment are not inadvertently disconnected.

  • Removed the deletion of local orderer connections and the associated call to close the connection.
  • Added a clarifying comment that explains local orderer connections are managed separately.

@github-actions github-actions bot added area: server Server related issues (routerlicious) base: main PRs targeted against main branch labels Apr 23, 2025
@znewton znewton merged commit 360ea12 into microsoft:main Apr 24, 2025
30 checks passed
@znewton znewton deleted the server/t9s-connection-bug branch April 24, 2025 16:03
markfields pushed a commit to markfields/FluidFramework that referenced this pull request Apr 24, 2025
)

## Description

Fixes
[AB#35445](https://dev.azure.com/fluidframework/internal/_workitems/edit/35445).

LocalOrdererManager handles connection cleanup differently from Nexus'
OrdererManager. microsoft#21528 incorrectly caused disconnect socket connection
to sever all current orderer connections.

<details>
<summary>Note on memory use</summary>

I had a much longer PR in progress to share logic between the Nexus
OrdererManager and LocalOrdererManager, but it got way too complicated,
and really the number of connections held by T9s shouldn't apply memory
pressure like it would in Nexus

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: server Server related issues (routerlicious) base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants