Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed — Connection counter double-decrement (2026-04-12)

- **monoio handler: `maxclients reached` after first disconnect.** `handle_connection_sharded_monoio` called `record_connection_closed()` unconditionally at its exit, while the caller (`conn_accept.rs`) also called it in the non-migration branch. The `AtomicU64` counter wrapped to `u64::MAX` on the second `fetch_sub`, causing every subsequent `try_accept_connection` to reject against `maxclients`. Removed the handler-level decrement to restore symmetry with the `try_accept_connection` increment owned by the caller. Verified: 10 sequential SETs now succeed; `redis-benchmark SET p=16 c=50` reports real throughput (1.25M+ req/s) instead of rejection errors.

### Added — Graph Engine Integration (v0.1.4, 2026-04-11)

- **Property graph engine** (`src/graph/`, feature-gated under `graph`): segment-aligned CSR storage with SlotMap generational indices, ArcSwap lock-free reads, Roaring validity bitmaps, and Rabbit Order compaction for cache locality. 8,500+ LOC, 319 tests.
Expand Down
7 changes: 6 additions & 1 deletion src/server/conn/handler_monoio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2327,6 +2327,11 @@ pub(crate) async fn handle_connection_sharded_monoio<
}
}

crate::admin::metrics_setup::record_connection_closed();
// NOTE: connection close is recorded by the caller (conn_accept.rs) to
// preserve symmetry with `try_accept_connection`, which owns the
// increment. Decrementing here too produces a double-decrement on the
// AtomicU64 counter — it wraps to u64::MAX on the second subtraction
// and all subsequent `try_accept_connection` comparisons against
// `maxclients` reject new connections.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
(MonoioHandlerResult::Done, None)
Comment on lines +2330 to 2336
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Migrated clients never decrement 🐞 Bug ≡ Correctness

After this PR, monoio connections that migrate to another shard will no longer call
record_connection_closed() when they finally disconnect on the target shard, so
CONNECTED_CLIENTS leaks upward. Over time this will incorrectly trip
try_accept_connection(maxclients) and reject new connections even when no clients are actually
connected.
Agent Prompt
## Issue description
`handle_connection_sharded_monoio()` no longer calls `record_connection_closed()`. For migrated monoio connections, the source shard correctly skips decrementing (connection stays alive), but the target shard’s migrated handler spawn also does not decrement after the handler finishes. This leaks `CONNECTED_CLIENTS`, eventually causing erroneous `maxclients` rejections.

## Issue Context
- Source shard: increments via `try_accept_connection(maxclients)`.
- Source shard: skips decrement when `_migrated == true`.
- Target shard: `spawn_migrated_monoio_connection()` spawns `handle_connection_sharded_monoio(..., Some(&state))` but does not call `record_connection_closed()` afterward.
- After this PR, there is no remaining place that decrements for migrated connections on final disconnect.

## Fix Focus Areas
- src/shard/conn_accept.rs[640-778]
- src/server/conn/handler_monoio.rs[2289-2337]

### Suggested implementation direction
Add `crate::admin::metrics_setup::record_connection_closed();` after awaiting `handle_connection_sharded_monoio()` in the `monoio::spawn` block inside `spawn_migrated_monoio_connection()`.

Optionally, update the comment in `handler_monoio.rs` to clarify that the caller owns close accounting for fresh accepts, but migrated-connection spawns must still decrement on final close.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
6 changes: 6 additions & 0 deletions src/shard/conn_accept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,12 @@ pub(crate) fn spawn_migrated_monoio_connection(
Some(&state),
)
.await;
// Migrated connection: the source shard's wrapper skipped the
// decrement (because `_migrated == true`), so this target-shard
// spawn site owns the balancing decrement. Without this the
// AtomicU64 `CONNECTED_CLIENTS` counter would leak upward on
// every migration and eventually trip `maxclients`.
crate::admin::metrics_setup::record_connection_closed();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
}
Err(e) => {
Expand Down
Loading