admin_cli destroys storage RDMA QPs without graceful close, leaving server-side orphan QPs that later fail WRType::CHECK
- Dominant language
- C++
- Stars
- 10.2k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
# Title
`admin_cli` destroys storage RDMA QPs without graceful close, leaving server-side orphan QPs that later fail `WRType::CHECK`
# Body
## Summary
We observed that a short-lived `admin_cli` command which creates a
`StorageClient` can destroy its storage-facing RC QPs without sending the 3FS
graceful-close message. The storage-side acceptor QPs remain `READY`/`RTS` and
are only detected as dead much later, when the periodic zero-byte
`WRType::CHECK` RDMA Write exhausts the RC retry budget.
We reproduced the complete lifecycle for one QP and found that the cause is the
shutdown order in `admin_cli`: `IBManager::stop()` runs before `StorageClient`
destruction. Consequently, `Transport::~Transport()` calls
`IBManager::close()` after `socketManager_` has already been reset, and
`IBManager::closeImpl()` silently lets the `IBSocket` destruct without calling
`closeGracefully()`.
The relevant code is unchanged on current `main` at
`22fca04564c7cc230fd8b9523b8b92864e1dad47`.
## Environment
- 3FS code path: identical to upstream `main` commit
`22fca04564c7cc230fd8b9523b8b92864e1dad47` for the files involved below.
- Transport: RC RDMA.
- Topology: one management node and five storage services.
- Trigger: a short-lived `admin_cli create-target` invocation that accesses an
already existing target. No fio or storage data workload is required for the
isolated reproduction.
- The additional log fields used below are observability-only; they do not
change close, CHECK, QP or retry behavior.
## Production observation
One deployment command connected to storage1 through storage5 in sequence and
then exited. Approximately 1783.5 seconds after each storage-side QP became
ready, all five QPs reported the same first completion error within a 0.47 ms
window:
| Storage | Storage QPN | `admin_cli` peer QPN | READY to first fault |
|---|---:|---:|---:|
| storage1 | 41 | 217 | 1783.754361 s |
| storage2 | 38 | 218 | 1783.565653 s |
| storage3 | 38 | 219 | 1783.838937 s |
| storage4 | 38 | 220 | 1783.650263 s |
| storage5 | 38 | 221 | 1783.461406 s |
The first completion on every QP had these fields:
```text
state=READY
closed=false
wc_status=transport retry counter exceeded
wc_opcode=IBV_WC_RDMA_WRITE
wr=[WRType::CHECK]
vendor_err=8
byte_len=0
```
These QPs were established by `admin_cli` before the data workload started;
they were not application data QPs.
## Isolated one-QP reproduction
We then ran one idempotent `create-target` command against an existing target,
with no fio process running, and recorded both QP endpoints and process
lifecycle:
```text
21:30:10.015409 storage: acceptor local_qpn=56 peer_qpn=297 READY
21:30:10.015543 admin_cli PID 192794: initiator local_qpn=297 peer_qpn=56 READY
21:30:10.017792 admin_cli: IBSocketManager::stopAndJoin starts
21:30:10.018022 admin_cli: StorageClient stop starts afterwards
21:30:10.018402 admin QPN297 destroy begins: state=READY closed=false first_fault=false
21:30:10.018697 admin QPN297 destroy completes
21:59:30.609055 storage QPN56 -> QPN297 is still present in RTS
21:59:33.967138 storage QPN56 first fault: WRType::CHECK / RETRY_EXC_ERR / closed=false
21:59:35.613820 storage QPN56 has disappeared from rdma res
```
Measured durations:
```text
storage READY -> CHECK retry-exceeded: 1763.951729 s
admin QP destroy -> CHECK retry-exceeded: 1763.948441 s
```
As a control, the same `admin_cli` process gracefully closed its mgmtd QP and
logged the local close post before QP destruction. Only the storage-facing QP
was destroyed with `closed=false`.
## Source analysis
### 1. `IBManager` is stopped before `storageClient` is destructed
`storageClient` is declared before the `IBManager::stop()` scope guard:
- [`admin_cli.cc` lines 95-111](https://github.com/deepseek-ai/3FS/blob/22fca04564c7cc230fd8b9523b8b92864e1dad47/src/client/bin/admin_cli.cc#L95-L111)
```cpp
std::shared_ptr storageClient;
...
SCOPE_EXIT { hf3fs::net::IBManager::stop(); };
```
C++ local objects are destroyed in reverse declaration order, so the scope
guard stops `IBManager` before `storageClient` is destructed.
The normal exit path explicitly stops `mgmtdClient` and the generic network
client, but does not stop or reset `storageClient`, `metaClient` or
`coreClient`:
- [`admin_cli.cc` lines 229-237](https://github.com/deepseek-ai/3FS/blob/22fca04564c7cc230fd8b9523b8b92864e1dad47/src/client/bin/admin_cli.cc#L229-L237)
### 2. The late `StorageClient` destructor drops transports after the manager is gone
`StorageClientImpl::~StorageClientImpl()` calls `stop()`, which stops its
messengers and drops their transports:
- [`StorageClientImpl.cc` lines 1410 and 1467-1492](https://github.com/deepseek-ai/3FS/blob/22fca04564c7cc230fd8b9523b8b92864e1dad47/src/client/storage/StorageClientImpl.cc#L1410-L1410)
An RDMA `Transport` destructor passes its socket to `IBManager::close()`:
- [`Transport.cc` lines 59-64](https://github.com/deepseek-ai/3FS/blob/22fca04564c7cc230fd8b9523b8b92864e1dad47/src/common/net/Transport.cc#L59-L64)
### 3. `closeImpl()` does not close the socket if the manager has stopped
`IBManager::reset()` first stops and resets `socketManager_`. Later,
`closeImpl()` only hands the socket to the manager when `socketManager_` is
non-null:
- [`IBDevice.cc` lines 822-842](https://github.com/deepseek-ai/3FS/blob/22fca04564c7cc230fd8b9523b8b92864e1dad47/src/common/net/ib/IBDevice.cc#L822-L842)
```cpp
void IBManager::closeImpl(IBSocket::Ptr socket) {
if (socketManager_) {
socketManager_->close(std::move(socket));
}
}
```
When `socketManager_ == nullptr`, the function parameter is simply destructed.
The drainer, which is responsible for calling `closeGracefully()`, is bypassed.
### 4. The peer is left waiting for the application-level close notification
The normal 3FS close path posts a zero-byte
`IBV_WR_RDMA_WRITE_WITH_IMM` carrying `ImmData::close()`. Direct local QP
destruction does not send that application-level notification.
The storage acceptor therefore still considers the socket `READY`. Its
periodic liveness CHECK is a zero-byte, signaled `IBV_WR_RDMA_WRITE`; after the
peer QP no longer exists, that CHECK eventually completes with
`IBV_WC_RETRY_EXC_ERR`.
## Expected behavior
Before `IBManager::stop()`:
1. storage/meta/core clients should be explicitly stopped and released;
2. their transports should enter the socket drainer;
3. storage-facing QPs should post the graceful-close message;
4. the storage acceptor should receive the close notification and reclaim its
QP in bounded time.
## Actual behavior
The initiator QP is destroyed with `state=READY` and `closed=false`. The peer
acceptor QP remains `RTS` until a later CHECK consumes the full RC retry budget
and reports `transport retry counter exceeded`.
## Impact
- Short-lived administrative operations can leave one orphan QP per contacted
storage service.
- The errors appear much later and can be incorrectly attributed to the active
data workload or RDMA fabric.
- With a large retry budget, stale QPs and CHECK WRs can survive for many
minutes before being reclaimed.
- Repeated deployment/admin operations can accumulate misleading transport
errors and consume QP resources.
## Suggested fix
At minimum, explicitly stop and release all clients that may own RDMA
transports before stopping the generic client and `IBManager`, for example:
```text
stop/reset metaClient, storageClient and coreClient
stop mgmtdClient
stop generic net::Client
stop IBManager last
```
It may also be useful for `IBManager::closeImpl()` to log or reject a
`READY`/not-closed socket when `socketManager_ == nullptr`, rather than silently
destroying it.
A regression test could run a storage-using `admin_cli` command and assert that:
1. the client side sends graceful close before QP destruction;
2. the server side receives the close and removes the acceptor QP promptly;
3. no matching `WRType::CHECK / IBV_WC_RETRY_EXC_ERR` appears after the original
retry horizon.
## Possibly related issue
Issue #192 reports a different symptom (a crash while closing an IBSocket on an
E810 device) and includes `admin_cli`/`Transport::~Transport()` in the stack.
This report is not a duplicate: the symptom here is a non-crashing, silent
bypass of graceful close followed by delayed server-side CHECK retry exhaustion.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.