Skip to content

orchestrator-process: fix TCP proxy#35008

Merged
teskje merged 1 commit intoMaterializeInc:mainfrom
teskje:process-fix-tcp-proxy
Feb 16, 2026
Merged

orchestrator-process: fix TCP proxy#35008
teskje merged 1 commit intoMaterializeInc:mainfrom
teskje:process-fix-tcp-proxy

Conversation

@teskje
Copy link
Contributor

@teskje teskje commented Feb 13, 2026

This fixes a bug in the use of select! in the process orchestrator's TCP proxy. The select has two branches, an accept branch that accepts new TCP connections and appends them to a FuturesUnordered, and a driver branch that drives the FuturesUnordered. The driver branch had a Some(Err(e)) match pattern, to print a warning on errors.

On pattern mismatch, e.g., if one of the connection futures completed successfully, the select! macro doesn't complete but continues polling the remaining branches, i.e. the accept branch. This means we can get into a state where the macro only waits for new connection requests, but doesn't drive forward existing connections anymore. Existing connections will hang until a new connection is accepted, completing the select!.

The fix is to not do the Err matching in the select! macro, but in the handler body instead.

This fixes a bug in the use of `select!` in the process orchestrator's
TCP proxy. The select has two branches, an accept branch that accepts
new TCP connections and appends them to a `FuturesUnordered`, and a
driver branch that drives the `FuturesUnordered`. The driver branch had
a `Some(Err(e))` match pattern, to print a  warning on errors.

On pattern mismatch, e.g., if one of the connection futures completed
successfully, the `select!` macro doesn't complete but continues polling
the remaining branches, i.e. the accept branch. This means we can get
into a state where the macro only waits for new connection requests, but
doesn't drive forward existing connections anymore. Existing connections
will hang until a new connection is accepted, completing the `select!`.

The fix is to not do the `Err` matching in the `select!` macro, but in
the handler body instead.
@teskje teskje requested a review from a team as a code owner February 13, 2026 19:13
@github-actions
Copy link

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

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

Thank you!

@teskje teskje merged commit db278f3 into MaterializeInc:main Feb 16, 2026
134 checks passed
@teskje teskje deleted the process-fix-tcp-proxy branch February 16, 2026 11:07
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.

2 participants