perf: speed up rust fast-test server#5059
Open
lucarlig wants to merge 9 commits into
Open
Conversation
8c30fbb to
7cac5dd
Compare
Signed-off-by: lucarlig <luca.carlig@ibm.com>
7cac5dd to
8a26277
Compare
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
msureshkumar88
requested changes
Jun 5, 2026
Collaborator
msureshkumar88
left a comment
There was a problem hiding this comment.
Good performance direction overall. A few changes needed before this can merge.
Blocking:
- E2E parity test stale tool name hints — the new test will unconditionally skip in CI after the rename (see inline comment)
- Predictable session IDs — sequential integers are trivially enumerable; replace with a random token
- Unbounded session growth — no cap means the session store is an OOM vector
mcp_error_response_with_statusinterpolatesmessagedirectly into JSON without escaping — inconsistent withmcp_text_result_responsewhich usesserde_json::to_stringechodelay has no upper bound — a client can stall a handler with an arbitrarily large value
Non-blocking suggestions:
FastTestServerrmcp layer is never mounted — either remove the ~150-line dead path or document that it's an unreachable reference implementationtools/listhardcodes a raw JSON string that duplicates the#[tool]declarations — two sources of truth that can driftmcp_base_headers()allocates a newHeaderMapon every response — aLazyLock<HeaderMap>or axum tuple response would eliminate this allocation from the hot pathFastTestServer::request_countis never incremented in production (only in the dead rmcp path)- PR says "Refs #4497" — confirm whether this closes the issue or partially addresses it
Signed-off-by: lucarlig <lucarlig@protonmail.com>
Signed-off-by: lucarlig <lucarlig@protonmail.com>
msureshkumar88
approved these changes
Jun 5, 2026
Collaborator
msureshkumar88
left a comment
There was a problem hiding this comment.
All requested changes addressed and verified against the source:
- Session IDs now use
Uuid::new_v4()— no longer enumerable - Session cap enforced at 10 000; initialize returns 503 when full, with a unit test covering the boundary
mcp_error_response_with_statususesserde_json::to_stringfor message escaping — JSON injection path closed- Echo delay capped at 60 s via
validate_delay()in both the MCP and REST paths - Parity test hints updated to
fast-timeprefix — no longer silently skips in CI - rmcp layer removed entirely (dead code + stale deps gone)
mcp_json_responsereturns an axum tuple — HeaderMap alloc eliminated from the hot path
Good work turning these around cleanly. Ready to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This draft updates the Rust
fast-test-serverto reduce MCP benchmark latency:/mcpthrough a lean direct Streamable HTTP JSON-RPC handler for this fixed tool surfaceoutputSchemaechodelay and jitter behavior on the direct handlerDELETE /mcp2025-11-25) while accepting older initialize payloads such as2024-11-05/versionendpoint withmcp-go's latest protocol version and pin that in its unit testchrono-tzso IANA zones, DST, half-hour offsets, and UTCZformatting match Go fast-time behaviorconvert_timeoutputs, and checks Rust latency stays in the same CF-routed band as Gotokio-utilfrom this serverBenchmark with
sdk-benchmarkfromsco3/mcp-benchmark, single user,-i 100 -r 10000,get_system_timewith{"timezone":"UTC"}:For context, the local Go fast-time run on the same benchmark shape measured:
Verification
go test ./...inmcp-servers/go/fast-time-servercargo fmt --checkinmcp-servers/rust/fast-test-servercargo testinmcp-servers/rust/fast-test-servercargo build --releaseinmcp-servers/rust/fast-test-serverJWT_SECRET_KEY=my-test-key-but-now-longer-than-32-bytes MCP_CLI_BASE_URL=http://127.0.0.1:8080 .venv/bin/python -m pytest tests/live_gateway/mcp/test_fast_server_go_rust_parity.py -q -sconvert_timeparity passed forAmerica/New_YorkDST andAsia/Kolkatahalf-hour offset cases0.025996s, Rust avg0.026905s,FAST_SERVER_PARITY_CALLS=5127.0.0.1:19080:2024-11-05request payload and received2025-11-25tools/listwith issued session returned HTTP 200Note:
make testing-upcould not be used in this local environment because required GHCR images returned 403, so the live parity verification above used local CF and local upstream processes registered through the same/gatewaysAPI path.Closes #4497