Skip to content

Highlight MFA > Fix OOM issue when user count is > 100k #87

Merged
kat3samsin merged 11 commits intoproductionfrom
PLTFRM-1521/refactor-highlight-mfa-count
Aug 18, 2025
Merged

Highlight MFA > Fix OOM issue when user count is > 100k #87
kat3samsin merged 11 commits intoproductionfrom
PLTFRM-1521/refactor-highlight-mfa-count

Conversation

@kat3samsin
Copy link
Copy Markdown
Contributor

@kat3samsin kat3samsin commented Aug 15, 2025

Description

Fixed critical performance issue for sites with 100k+ users experiencing OOM when loading Users page and counting users without 2FA.

Problem: This was raised by @pandah3 here. The original implementation looped through all users calling Two_Factor_Core::is_user_using_two_factor() for each, causing O(n) database queries. With 201k users, this caused >768MB memory exhaustion and >60s timeouts.

Solution: Single optimized SQL query that JOINs users with usermeta, filters by roles/capabilities directly in SQL, and counts users without 2FA in one operation.

Results:

  • Execution time: >60s → 0.58s (103x faster)
  • Memory usage: >768MB → <1MB (768x reduction)
  • Database queries: 90,126 → 1
  • Success rate: 0% → 100%

Code: /modules/highlight-mfa-users/class-highlight-mfa-users.php

SELECT COUNT(DISTINCT u.ID)
FROM wp_users u
INNER JOIN wp_usermeta um_cap 
    ON u.ID = um_cap.user_id 
    AND um_cap.meta_key = 'wp_capabilities'
LEFT JOIN wp_usermeta um_2fa 
    ON u.ID = um_2fa.user_id 
    AND um_2fa.meta_key = '_two_factor_enabled_providers'
WHERE (role conditions)
AND (exclude conditions)
AND (um_2fa.meta_value IS NULL 
    OR um_2fa.meta_value = ''
    OR um_2fa.meta_value = 'a:0:{}')

Supported:

  • Role-based filtering (administrator, editor, custom roles)
  • Capability-based filtering (translates capabilities to roles)
  • User exclusions (skip lists, bot users)
  • Single site and specific blog in multisite
  • 1-hour caching with invalidation on changes

Limitations:

  • Bypasses Two-Factor plugin filters (two_factor_primary_provider_for_user, etc.)
  • No provider availability validation (ie: A user has "SMS" as their 2FA provider in the database, but the SMS provider plugin was later deactivated. The original code would detect this and count them as "no 2FA", but our SQL query would incorrectly count them as "has 2FA" since the meta value exists.)
  • No runtime capability modifications via user_has_cap filter
  • Assumes standard _two_factor_enabled_providers meta structure
  • Does not support network wide multisite counting (blog_id=0)

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally or in Codespaces (or has an appropriate fallback).
  • This change has relevant unit tests (if applicable).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  1. Checkout PR
  2. vip dev-env create && vip dev-env start
  3. Save this in your local dev-env as bulk-create-users.php (run vip dev-env info --slug=vip-security-boost to find the location)
bulk-create-users.php
<?php
/**
 * Bulk create 100k test users using direct database queries 
 * 
 * Usage:
 *   vip dev-env exec -- wp eval-file bulk-create-users.php
 * 
 */

global $wpdb, $argv;

$total_users = 100000;
$batch_size = 500;

echo "===========================================\n";
echo "Bulk User Creation Script\n";
echo "===========================================\n";
echo "Users to create: " . number_format($total_users) . "\n";
echo "Batch size: $batch_size\n";
echo "Estimated time: " . round($total_users / 500, 1) . " seconds\n";
echo "===========================================\n\n";

$start_time = microtime(true);
$created = 0;

// Store the hashed password once (it's the same for all users)
$hashed_password = wp_hash_password('password123');
$escaped_password = $wpdb->_real_escape($hashed_password);

// Get the starting user ID to ensure we don't conflict
$max_user_id = $wpdb->get_var("SELECT MAX(ID) FROM {$wpdb->users}");
$starting_user_id = $max_user_id ? $max_user_id + 1 : 1;

// Disable autocommit for faster inserts
$wpdb->query('SET autocommit = 0');

for ($batch_start = 1; $batch_start <= $total_users; $batch_start += $batch_size) {
    $batch_end = min($batch_start + $batch_size - 1, $total_users);
    $batch_count = $batch_end - $batch_start + 1;
    
    // Build bulk insert for users
    $user_values = [];
    $user_ids = [];
    
    for ($i = $batch_start; $i <= $batch_end; $i++) {
        $user_id = $starting_user_id + $i - 1;
        $user_ids[] = $user_id;
        
        $user_values[] = sprintf(
            "(%d, 'testuser%d', '%s', 'testuser%d', 'testuser%d@example.com', '', NOW(), '', 0, 'Test User %d')",
            $user_id,
            $i,
            $escaped_password,
            $i,
            $i,
            $i
        );
    }
    
    // Insert users in bulk with explicit IDs
    $sql = "INSERT INTO {$wpdb->users} 
            (ID, user_login, user_pass, user_nicename, user_email, user_url, 
             user_registered, user_activation_key, user_status, display_name) 
            VALUES " . implode(',', $user_values);
    
    $wpdb->query($sql);
    
    // Build bulk insert for usermeta
    $meta_values = [];
    
    foreach ($user_ids as $index => $user_id) {
        $user_num = $batch_start + $index;
        
        // Determine role (20% admin, 30% editor, 20% author, 20% contributor, 10% subscriber)
        $rand = rand(1, 100);
        if ($rand <= 20) {
            $capabilities = 'a:1:{s:13:"administrator";b:1;}';
            $level = '10';
        } elseif ($rand <= 50) {
            $capabilities = 'a:1:{s:6:"editor";b:1;}';
            $level = '7';
        } elseif ($rand <= 70) {
            $capabilities = 'a:1:{s:6:"author";b:1;}';
            $level = '2';
        } elseif ($rand <= 90) {
            $capabilities = 'a:1:{s:11:"contributor";b:1;}';
            $level = '1';
        } else {
            $capabilities = 'a:1:{s:10:"subscriber";b:1;}';
            $level = '0';
        }
        
        // Add capabilities
        $meta_values[] = sprintf(
            "(%d, '%scapabilities', '%s')",
            $user_id,
            $wpdb->prefix,
            $wpdb->_real_escape($capabilities)
        );
        
        // Add user level
        $meta_values[] = sprintf(
            "(%d, '%suser_level', '%s')",
            $user_id,
            $wpdb->prefix,
            $level
        );
        
        // Add nickname
        $meta_values[] = sprintf(
            "(%d, 'nickname', 'testuser%d')",
            $user_id,
            $user_num
        );
        
        // Add first and last name
        $meta_values[] = sprintf(
            "(%d, 'first_name', 'Test')",
            $user_id
        );
        
        $meta_values[] = sprintf(
            "(%d, 'last_name', 'User%d')",
            $user_id,
            $user_num
        );
        
        // 30% chance for 2FA enabled
        // if (rand(1, 100) <= 30) {
        //     $meta_values[] = sprintf(
        //         "(%d, '_two_factor_enabled_providers', 'a:1:{i:0;s:15:\"Two_Factor_Totp\";}')",
        //         $user_id
        //     );
        //     $meta_values[] = sprintf(
        //         "(%d, '_two_factor_provider', 'Two_Factor_Totp')",
        //         $user_id
        //     );
        // }
    }
    
    // Insert usermeta in bulk
    if (!empty($meta_values)) {
        $meta_sql = "INSERT INTO {$wpdb->usermeta} (user_id, meta_key, meta_value) VALUES " . 
                    implode(',', $meta_values);
        $wpdb->query($meta_sql);
    }
    
    // Commit this batch
    $wpdb->query('COMMIT');
    
    $created = $batch_end;
    $percent = round(($created / $total_users) * 100, 1);
    echo "Created $created / $total_users users ($percent%)...\r";
}

// Re-enable autocommit
$wpdb->query('SET autocommit = 1');

$end_time = microtime(true);
$execution_time = round($end_time - $start_time, 2);

echo "\n\nCompleted! Created $total_users test users in $execution_time seconds.\n";
echo "Average: " . round($total_users / $execution_time) . " users per second\n\n";

// Clear any caches
wp_cache_flush();

// Verify results
$total_count = $wpdb->get_var(
    "SELECT COUNT(*) FROM {$wpdb->users} WHERE user_login LIKE 'testuser%'"
);

$with_2fa_count = $wpdb->get_var(
    "SELECT COUNT(DISTINCT user_id) 
     FROM {$wpdb->usermeta} 
     WHERE meta_key = '_two_factor_enabled_providers' 
     AND user_id IN (SELECT ID FROM {$wpdb->users} WHERE user_login LIKE 'testuser%')"
);

$admin_count = $wpdb->get_var(
    "SELECT COUNT(*) FROM {$wpdb->usermeta} 
     WHERE meta_key = '{$wpdb->prefix}capabilities' 
     AND meta_value LIKE '%administrator%'
     AND user_id IN (SELECT ID FROM {$wpdb->users} WHERE user_login LIKE 'testuser%')"
);

$editor_count = $wpdb->get_var(
    "SELECT COUNT(*) FROM {$wpdb->usermeta} 
     WHERE meta_key = '{$wpdb->prefix}capabilities' 
     AND meta_value LIKE '%editor%'
     AND user_id IN (SELECT ID FROM {$wpdb->users} WHERE user_login LIKE 'testuser%')"
);

$author_count = $wpdb->get_var(
    "SELECT COUNT(*) FROM {$wpdb->usermeta} 
     WHERE meta_key = '{$wpdb->prefix}capabilities' 
     AND meta_value LIKE '%author%'
     AND user_id IN (SELECT ID FROM {$wpdb->users} WHERE user_login LIKE 'testuser%')"
);

$contributor_count = $wpdb->get_var(
    "SELECT COUNT(*) FROM {$wpdb->usermeta} 
     WHERE meta_key = '{$wpdb->prefix}capabilities' 
     AND meta_value LIKE '%contributor%'
     AND user_id IN (SELECT ID FROM {$wpdb->users} WHERE user_login LIKE 'testuser%')"
);

$subscriber_count = $total_count - $admin_count - $editor_count - $author_count - $contributor_count;

echo "Verification:\n";
echo "- Total test users: $total_count\n";
echo "- Administrators: $admin_count (" . round($admin_count/$total_count*100, 1) . "%)\n";
echo "- Editors: $editor_count (" . round($editor_count/$total_count*100, 1) . "%)\n";
echo "- Authors: $author_count (" . round($author_count/$total_count*100, 1) . "%)\n";
echo "- Contributors: $contributor_count (" . round($contributor_count/$total_count*100, 1) . "%)\n";
echo "- Subscribers: $subscriber_count (" . round($subscriber_count/$total_count*100, 1) . "%)\n";
echo "- Users with 2FA: $with_2fa_count (" . round($with_2fa_count/$total_count*100, 1) . "%)\n";
echo "- Users without 2FA: " . ($total_count - $with_2fa_count) . " (" . round(($total_count - $with_2fa_count)/$total_count*100, 1) . "%)\n";

// Optionally clean up test users
echo "\nTo remove test users, run:\n";
echo "vip dev-env exec -- wp eval \"global \\\$wpdb; \\\$wpdb->query('DELETE FROM {\\\$wpdb->users} WHERE user_login LIKE \\'testuser%\\''); \\\$wpdb->query('DELETE FROM {\\\$wpdb->usermeta} WHERE user_id NOT IN (SELECT ID FROM {\\\$wpdb->users})'); echo 'Test users deleted.';\"\n";
  1. Run vip dev-env exec -- wp eval-file bulk-create-users.php to create 100k test users
  2. Login to wp-admin and navigate to Users page
  3. Make sure it loads without issues and accurately counts the number of Admins and Editors created

@kat3samsin kat3samsin force-pushed the PLTFRM-1521/refactor-highlight-mfa-count branch from 27a68f9 to 019c481 Compare August 15, 2025 21:05
@kat3samsin kat3samsin force-pushed the PLTFRM-1521/refactor-highlight-mfa-count branch from d15b134 to 7a11994 Compare August 15, 2025 21:18
@kat3samsin kat3samsin marked this pull request as ready for review August 15, 2025 21:28
@kat3samsin kat3samsin requested review from a team and Copilot August 15, 2025 21:28
@kat3samsin kat3samsin changed the title fix:perf issue with counting users without 2fa using db query Highlight MFA > Fix OOM issue when user count is > 100k Aug 15, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a critical performance issue where counting users without 2FA for sites with 100k+ users was causing OOM errors and timeouts. The solution replaces the original O(n) approach that looped through all users with a single optimized SQL query.

Key changes:

  • Replaces individual user queries with a single JOIN-based SQL query for counting users without 2FA
  • Adds 1-hour caching with cache invalidation for the count results
  • Updates test expectations to account for the new SQL-based counting behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
modules/highlight-mfa-users/class-highlight-mfa-users.php Implements optimized SQL query to count users without 2FA, replacing the loop-based approach
tests/phpunit/test-highlight-mfa-users.php Updates test expectations and adds direct meta setting to work with the new SQL implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread modules/highlight-mfa-users/class-highlight-mfa-users.php Outdated
Comment thread modules/highlight-mfa-users/class-highlight-mfa-users.php
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@kat3samsin kat3samsin added this pull request to the merge queue Aug 18, 2025
Merged via the queue into production with commit 48bd988 Aug 18, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants