Skip to content

fix: make thrift transport fallback pool-aware#654

Open
shenyj3 wants to merge 3 commits into
mainfrom
agent/shmipc-transport-fallback
Open

fix: make thrift transport fallback pool-aware#654
shenyj3 wants to merge 3 commits into
mainfrom
agent/shmipc-transport-fallback

Conversation

@shenyj3

@shenyj3 shenyj3 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • move thrift address fallback into a shared pool-aware transport acquirer
  • keep shmipc on its direct SessionManager path while pooling UDS/TCP by the actual candidate address
  • share candidate acquisition across pingpong and multiplex without cloning requests, and remove the legacy client-side fallback transport wrappers
  • deliver already-buffered response bytes before surfacing an underlying stream close/error
  • close cancelled or failed shmipc RPC streams and recycle only successfully completed streams
  • update shmipc 0.2 to feature/polling-to-eventfd at c2747ce9

Root cause

The previous make-transport wrapper could dial a fallback address after the outer connection pool had already selected the primary address as its key. A successful UDS fallback could therefore be stored under the shmipc key, making later requests stick to the wrong transport.

The pingpong path also recycled shmipc streams after failed calls. A failed send can leave unsent bytes in the stream buffer, while cancellation can drop the RPC future before normal recycling runs; either case can leave an unusable stream available for a later request. Separately, BufReader could poll the underlying stream even while it still held response bytes, allowing a close/error to hide an already-buffered TTHeader frame.

Impact

Fallback transports are pooled under the address that was actually dialed. Completed buffered responses remain readable when the peer closes immediately afterward, and failed or cancelled shmipc streams are closed instead of being leaked or reused.

Validation

  • cargo +nightly fmt --all -- --check
  • cargo +nightly clippy -p volo-thrift --no-default-features --features shmipc,multiplex -- --deny warnings
  • cargo +nightly test -p volo-thrift --features shmipc,multiplex (36 passed)
  • cargo +nightly test -p volo --features shmipc (22 passed, 1 ignored)

中文说明

变更概述

  • 将 Thrift 地址降级逻辑移入共享且感知连接池键的 transport acquirer
  • shmipc 继续使用 SessionManager 的直连路径,UDS/TCP 则按实际拨号的候选地址进行连接池复用
  • pingpong 与 multiplex 共享候选地址获取流程,无需克隆请求,并移除旧的客户端 fallback transport wrapper
  • 在返回底层连接关闭或错误前,优先交付已经缓冲的响应数据
  • 关闭被取消或执行失败的 shmipc RPC 流,仅复用成功完成请求的流
  • shmipc 0.2 更新到 feature/polling-to-eventfd 分支的 c2747ce9

根因

旧的 make-transport wrapper 可能在外层连接池已经选定主地址作为连接池键之后,才改为拨号 fallback 地址。因此,成功建立的 UDS fallback 连接可能被存入 shmipc 地址对应的连接池项,导致后续请求持续复用错误的传输类型。

此外,pingpong 路径会在调用失败后仍然回收 shmipc 流。发送失败可能在发送缓冲区中留下未发送数据,而请求取消会在正常回收逻辑执行前直接丢弃 RPC future;这两种情况都会让不可用的流留给后续请求复用。另一方面,BufReader 在内部仍有响应数据时也可能继续轮询底层流,使连接关闭或错误掩盖已经缓冲的完整 TTHeader 帧。

影响

fallback transport 现在会按实际拨号地址进入对应连接池。即使对端在发送完成后立即关闭连接,已缓冲的完整响应仍可正常读取;失败或取消的 shmipc 流会被关闭,不再泄漏或被后续请求复用。

验证

  • cargo +nightly fmt --all -- --check
  • cargo +nightly clippy -p volo-thrift --no-default-features --features shmipc,multiplex -- --deny warnings
  • cargo +nightly test -p volo-thrift --features shmipc,multiplex(36 个测试通过)
  • cargo +nightly test -p volo --features shmipc(22 个测试通过,1 个忽略)

shenyj3 added 2 commits July 15, 2026 03:05
Preserve buffered reads before surfacing stream errors, close cancelled shmipc RPC streams, and prevent failed streams from returning to the pool. Refresh the polling-to-eventfd dependency revision.
@shenyj3
shenyj3 force-pushed the agent/shmipc-transport-fallback branch from 42ba042 to 5a275a0 Compare July 14, 2026 19:14
@shenyj3
shenyj3 marked this pull request as ready for review July 14, 2026 19:18
@shenyj3
shenyj3 requested review from a team July 14, 2026 19:18
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.39506% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.42%. Comparing base (ce5c0b6) to head (95e7048).

Files with missing lines Patch % Lines
volo-thrift/src/transport/dial.rs 87.97% 41 Missing ⚠️
volo-thrift/src/transport/pingpong/client.rs 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #654      +/-   ##
==========================================
+ Coverage   46.20%   48.42%   +2.22%     
==========================================
  Files         163      164       +1     
  Lines       20307    20691     +384     
==========================================
+ Hits         9382    10019     +637     
+ Misses      10925    10672     -253     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant