Skip to content

Fix SessionManager write amplification causing lock contention #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
galatanovidiu merged 2 commits into from
Apr 8, 2026
+115 −11
Merged

Fix SessionManager write amplification causing lock contention #166

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42f195c
fix: Reduce SessionManager write amplification on validate_session()
galatanovidiu Apr 7, 2026
13218b3
fix: Clamp activity_update_interval to half of inactivity_timeout
galatanovidiu Apr 8, 2026
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
39 changes: 30 additions & 9 deletions includes/Transport/Infrastructure/SessionManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ final class SessionManager {
*/
private const DEFAULT_INACTIVITY_TIMEOUT = DAY_IN_SECONDS;

/**
* Minimum interval between last_activity writes in seconds.
*
* @var int
*/
private const DEFAULT_ACTIVITY_UPDATE_INTERVAL = 60;

/**
* Create a new session for a user
*
Expand Down Expand Up @@ -156,7 +163,7 @@ public static function get_all_user_sessions( int $user_id ): array {
/**
* Get configuration values.
*
* @return array<string, int> Configuration array.
* @return array{max_sessions: int, inactivity_timeout: int, activity_update_interval: int} Configuration array.
*/
private static function get_config(): array {
/**
Expand All @@ -183,9 +190,23 @@ private static function get_config(): array {
*/
$inactivity_timeout = (int) apply_filters( 'mcp_adapter_session_inactivity_timeout', self::DEFAULT_INACTIVITY_TIMEOUT );

/**
* Filters the minimum interval between session last_activity writes.
*
* To reduce write amplification, the session manager only updates
* `last_activity` if at least this many seconds have elapsed since
* the last write.
*
* @since n.e.x.t
*
* @param int $interval Minimum seconds between writes. Default 60.
*/
$activity_update_interval = (int) apply_filters( 'mcp_adapter_session_activity_update_interval', self::DEFAULT_ACTIVITY_UPDATE_INTERVAL );

return array(
'max_sessions' => $max_sessions,
'inactivity_timeout' => $inactivity_timeout,
'max_sessions' => $max_sessions,
'inactivity_timeout' => $inactivity_timeout,
'activity_update_interval' => $activity_update_interval,
);
}

Expand Down Expand Up @@ -254,9 +275,6 @@ public static function validate_session( int $user_id, string $session_id ): boo
return false;
}

// Opportunistic cleanup
self::cleanup_expired_sessions( $user_id );

$sessions = self::get_all_user_sessions( $user_id );

if ( ! isset( $sessions[ $session_id ] ) ) {
Expand All @@ -274,9 +292,12 @@ public static function validate_session( int $user_id, string $session_id ): boo
return false;
}

// Update last activity
$sessions[ $session_id ]['last_activity'] = time();
update_user_meta( $user_id, self::SESSION_META_KEY, $sessions );
// Throttle last_activity writes to reduce write amplification
$activity_update_interval = $config['activity_update_interval'];
if ( time() - $session['last_activity'] >= $activity_update_interval ) {
$sessions[ $session_id ]['last_activity'] = time();
update_user_meta( $user_id, self::SESSION_META_KEY, $sessions );
}

return true;
}
Expand Down
48 changes: 46 additions & 2 deletions tests/Unit/Transport/McpSessionManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ public function test_validation_updates_last_activity(): void {
$session_id = SessionManager::create_session( $this->test_user_id, array() );
$this->assertIsString( $session_id );

// Directly update the timestamp to simulate time passing
// Directly update the timestamp to simulate time passing beyond throttle window
$sessions = \WP\MCP\Transport\Infrastructure\SessionManager::get_all_user_sessions( $this->test_user_id );
$old_timestamp = time() - 2;
$old_timestamp = time() - 61;
$sessions[ $session_id ]['last_activity'] = $old_timestamp;
update_user_meta( $this->test_user_id, 'mcp_adapter_sessions', $sessions );

Expand Down Expand Up @@ -324,4 +324,48 @@ static function () {

remove_all_filters( 'mcp_adapter_session_inactivity_timeout' );
}

/**
* Test validation skips last_activity update within throttle window
*/
public function test_validation_skips_last_activity_update_within_throttle_window(): void {
$session_id = SessionManager::create_session( $this->test_user_id, array() );
$this->assertIsString( $session_id );

// Record the last_activity right after creation
$sessions_before = SessionManager::get_all_user_sessions( $this->test_user_id );
$original_activity = $sessions_before[ $session_id ]['last_activity'];

// Validate immediately (within the 60s throttle window)
$is_valid = SessionManager::validate_session( $this->test_user_id, $session_id );
$this->assertTrue( $is_valid );

// last_activity should remain unchanged
$sessions_after = SessionManager::get_all_user_sessions( $this->test_user_id );
$this->assertSame( $original_activity, $sessions_after[ $session_id ]['last_activity'] );
}

/**
* Test validate_session does not call cleanup
*/
public function test_validation_does_not_call_cleanup(): void {
// Create two sessions
$valid_session_id = SessionManager::create_session( $this->test_user_id, array() );
$expired_session_id = SessionManager::create_session( $this->test_user_id, array() );
$this->assertIsString( $valid_session_id );
$this->assertIsString( $expired_session_id );

// Backdate one session to make it expired
$sessions = SessionManager::get_all_user_sessions( $this->test_user_id );
$sessions[ $expired_session_id ]['last_activity'] = time() - ( DAY_IN_SECONDS + 3600 );
update_user_meta( $this->test_user_id, 'mcp_adapter_sessions', $sessions );

// Validate the valid session
$is_valid = SessionManager::validate_session( $this->test_user_id, $valid_session_id );
$this->assertTrue( $is_valid );

// The expired session should still exist (cleanup not called)
$sessions_after = SessionManager::get_all_user_sessions( $this->test_user_id );
$this->assertArrayHasKey( $expired_session_id, $sessions_after );
}
}
Loading