Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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

}
15 changes: 15 additions & 0 deletions src/shard/conn_accept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,11 @@ pub(crate) fn spawn_migrated_monoio_connection(
fd,
e
);
// Source shard's `try_accept_connection` already counted this
// client; its wrapper skipped the decrement because the migration
// succeeded from the source's perspective. Since we cannot hand
// ownership to the handler, we own the balancing decrement here.
crate::admin::metrics_setup::record_connection_closed();
return; // std_stream Drop closes FD
}
match monoio::net::TcpStream::from_std(std_stream) {
Expand Down Expand Up @@ -765,6 +770,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 All @@ -773,6 +784,10 @@ pub(crate) fn spawn_migrated_monoio_connection(
shard_id,
e
);
// Same rationale as the set_nonblocking failure above:
// the source-side increment stands but no handler will run,
// so we own the balancing decrement.
crate::admin::metrics_setup::record_connection_closed();
}
}
}
Loading