Fix SessionManager write amplification causing lock contention#166
Fix SessionManager write amplification causing lock contention#166galatanovidiu merged 2 commits intotrunkfrom
Conversation
validate_session() was writing to the same wp_usermeta row on every MCP request (last_activity update + cleanup_expired_sessions), causing InnoDB row-level lock contention when 4+ concurrent requests hit simultaneously during MCP initialization. Two changes: - Throttle last_activity writes to once per 60s (configurable via mcp_adapter_session_activity_update_interval filter) - Remove cleanup_expired_sessions() call — it already runs in create_session() and is unnecessary on the validation hot path
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR reduces database lock contention during MCP initialization by limiting how often session validation writes to the mcp_adapter_sessions usermeta row.
Changes:
- Adds a configurable throttle (default 60s) for persisting
last_activityupdates duringSessionManager::validate_session(). - Removes opportunistic
cleanup_expired_sessions()calls fromvalidate_session()to avoid extra writes. - Updates/extends unit tests to reflect throttling behavior and the removal of cleanup-on-validate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| includes/Transport/Infrastructure/SessionManager.php | Introduces activity_update_interval config + throttled last_activity writes; removes cleanup call from validation. |
| tests/Unit/Transport/McpSessionManagerTest.php | Adjusts existing timestamp-based validation test and adds tests for throttling and no-cleanup behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #166 +/- ##
============================================
+ Coverage 88.06% 88.25% +0.18%
- Complexity 1241 1243 +2
============================================
Files 54 54
Lines 4030 4035 +5
============================================
+ Hits 3549 3561 +12
+ Misses 481 474 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Prevents sessions from expiring despite active use when inactivity_timeout is configured lower than the throttle interval. Also normalizes negative intervals to zero.
gziolo
left a comment
gziolo
left a comment
There was a problem hiding this comment.
Checked this out locally — lint, PHPStan, and the full test suite all pass.
The fix lands in the right place and should mitigate the problem nicely. Fresh sessions start with last_activity set to now, so the init burst falls inside the throttle window and simply skips the writes that were hurting us.
Summary
Why: MCP initialization sends 4+ concurrent HTTP requests (initialize, tools/list, resources/list, prompts/list), each hitting
SessionManager::validate_session()which unconditionally writeslast_activityand runscleanup_expired_sessions()— producing 4–8 writes to the samewp_usermetarow. This causes row-level lock contention that can cascade into database timeout failures.What:
last_activitywrites to once per 60 seconds (configurable via newmcp_adapter_session_activity_update_intervalfilter) — freshly-created sessions produce zero writes during the initialization burstcleanup_expired_sessions()call fromvalidate_session()— it already runs increate_session()Test plan
test_validation_updates_last_activitypasses (adjusted to exceed throttle window)test_validation_skips_last_activity_update_within_throttle_window— no write within 60stest_validation_does_not_call_cleanup— expired sessions survive validation