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 all commits
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
45 changes: 36 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,29 @@ 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 );

// Clamp: interval must be less than inactivity timeout to prevent
// sessions from expiring despite active use.
if ( $activity_update_interval >= $inactivity_timeout ) {
$activity_update_interval = (int) ( $inactivity_timeout / 2 );
}

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

Expand Down Expand Up @@ -254,9 +281,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 +298,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
81 changes: 79 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,81 @@ 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 );
}

/**
* Test session stays alive when inactivity timeout is less than default throttle interval
*/
public function test_validation_clamps_throttle_when_inactivity_timeout_is_low(): void {
// Set inactivity timeout to 30s (less than the default 60s throttle)
add_filter(
'mcp_adapter_session_inactivity_timeout',
static function () {
return 30;
}
);

$session_id = SessionManager::create_session( $this->test_user_id, array() );
$this->assertIsString( $session_id );

// Backdate last_activity by 16s (more than half of 30s timeout = clamped interval of 15s)
$sessions = SessionManager::get_all_user_sessions( $this->test_user_id );
$sessions[ $session_id ]['last_activity'] = time() - 16;
update_user_meta( $this->test_user_id, 'mcp_adapter_sessions', $sessions );

// Validate — should succeed AND update last_activity because 16s > clamped interval (15s)
$is_valid = SessionManager::validate_session( $this->test_user_id, $session_id );
$this->assertTrue( $is_valid );

$sessions_after = SessionManager::get_all_user_sessions( $this->test_user_id );
$this->assertGreaterThan(
$sessions[ $session_id ]['last_activity'],
$sessions_after[ $session_id ]['last_activity']
);

remove_all_filters( 'mcp_adapter_session_inactivity_timeout' );
}
}
Loading