Optimize SQL leaderboard queries and harden async leaderboard cache

Rework the readLeaderboard ORDER BY tiebreak onto skills.user_id and add per-skill leaderboard indexes so the query avoids a join-then-filesort on both MySQL and MariaDB (benchmarked ~350ms -> ~40ms at 300k rows).

Indexes are added to the fresh CREATE TABLE schema and, for existing databases, via a new idempotent, best-effort ADD_SKILL_LEADERBOARD_INDEXES upgrade that probes INFORMATION_SCHEMA and continues startup even if an index cannot be created.

Make FlatFile leaderboard state thread-safe (ConcurrentHashMap + volatile) for the async refresher, rename McTopPositionPlaceholder to McTopValuePlaceholder to reflect that it returns the value, and add cross-engine integration tests covering fresh-install, migration, and idempotent index behavior.

Author: nossr50

Changelog.txt

@@ -1,7 +1,10 @@
 Version 2.2.054
     Fixed party/admin chat allowing player color code tokens without the 'mcmmo.chat.colors' permission
     Fixed diminished returns resetting in certain situations when players reconnected
+    Fixed potential thread-safety issues with FlatFile leaderboard data
     Added PlaceholderAPI leaderboard rank-position placeholders for all non-child skills and overall power level (See notes)
+    Improved SQL leaderboard query performance on large databases (See notes)
+    Added automatic per-skill leaderboard indexes for SQL databases (See notes)
     Added 'General.PlaceholderAPI.Leaderboards.Max_Tracked_Rank' to config.yml
     Added 'General.PlaceholderAPI.Leaderboards.Refresh_Interval_Seconds' to config.yml
     (Codebase) Added async leaderboard snapshot caching for rank-position PlaceholderAPI lookups
@@ -40,6 +43,13 @@ Version 2.2.054
     Default is 60 seconds.
     Placeholder requests are served from cache between refreshes.
 
+    -- SQL leaderboard performance --
+    Leaderboard queries on SQL databases are now much faster on servers with large player tables.
+    mcMMO will automatically add per-skill indexes to the skills table on startup if they are missing.
+    On very large databases the first startup after updating may take longer while these indexes are built.
+    This runs once, is safe to re-run, and mcMMO will continue to start normally even if an index cannot be added.
+    Fresh SQL installs include these indexes from the start.
+
 Version 2.2.053
     !! -- This build has important fixes for anyone using Paper (or forks of Paper), please read the notes carefully.
     It is completely safe to update to this version of mcMMO.

src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java

@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.TreeSet;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.logging.Level;
@@ -44,11 +45,14 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
     private static final Object fileWritingLock = new Object();
     private static final String LINE_ENDING = "\r\n";
 
-    private final @NotNull EnumMap<PrimarySkillType, List<PlayerStat>> leaderboardMap =
-            new EnumMap<>(PrimarySkillType.class);
+    // Concurrent because async refreshes (PlaceholderAPI cache, /mctop, /mcrank) read this while
+    // updateLeaderboards() rebuilds it. Values are immutable snapshots swapped in atomically.
+    private final @NotNull Map<PrimarySkillType, List<PlayerStat>> leaderboardMap =
+            new ConcurrentHashMap<>();
 
-    private @NotNull List<PlayerStat> powerLevels = new ArrayList<>();
-    private long lastUpdate = 0L;
+    // volatile: published to / read from multiple async threads (see leaderboardMap note).
+    private volatile @NotNull List<PlayerStat> powerLevels = new ArrayList<>();
+    private volatile long lastUpdate = 0L;
 
     private final @NotNull String usersFilePath;
     private final @NotNull File usersFile;

src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java

@@ -548,11 +548,16 @@ private boolean updateHudSettings(Connection connection, int userId, PlayerProfi
                 ? ALL_QUERY_VERSION
                 : skill.name().toLowerCase(Locale.ENGLISH);
 
+        // Tiebreak on skills.user_id (not users.user) so the sort can be resolved on the skills
+        // table before the join. A tiebreak column from the joined users table forces both MySQL
+        // and MariaDB to join every qualifying row and then filesort. Paired with a per-skill
+        // index (see ADD_SKILL_LEADERBOARD_INDEXES), this lets the engine drive the sort from the
+        // skills table and touch only the LIMIT window. Verified on MySQL 8 and MariaDB 10.11.
         String sql = "SELECT " + query + ", `user` FROM " + tablePrefix + "users " +
                 "JOIN " + tablePrefix + "skills ON (user_id = id) " +
                 "WHERE " + query + " > 0 " +
                 "AND NOT `user` = '\\_INVALID\\_OLD\\_USERNAME\\_' " +
-                "ORDER BY " + query + " DESC, `user` LIMIT ?, ?";
+                "ORDER BY " + query + " DESC, user_id LIMIT ?, ?";
 
         try (Connection connection = getConnection(PoolIdentifier.MISC);
                 PreparedStatement statement = connection.prepareStatement(sql)) {
@@ -1308,7 +1313,28 @@ private void ensureSkillsTable(Connection connection,
                 + "`maces` int(10) unsigned NOT NULL DEFAULT " + startingLevel + ","
                 + "`spears` int(10) unsigned NOT NULL DEFAULT " + startingLevel + ","
                 + "`total` int(10) unsigned NOT NULL DEFAULT " + totalLevel + ","
-                + "PRIMARY KEY (`user_id`)) "
+                + "PRIMARY KEY (`user_id`),"
+                // Leaderboard indexes: readLeaderboard sorts on a single column per scope.
+                // Fresh installs get them here; existing installs get them via the idempotent
+                // ADD_SKILL_LEADERBOARD_INDEXES upgrade. Required for MariaDB to avoid a filesort.
+                + "INDEX `idx_taming` (`taming`),"
+                + "INDEX `idx_mining` (`mining`),"
+                + "INDEX `idx_woodcutting` (`woodcutting`),"
+                + "INDEX `idx_repair` (`repair`),"
+                + "INDEX `idx_unarmed` (`unarmed`),"
+                + "INDEX `idx_herbalism` (`herbalism`),"
+                + "INDEX `idx_excavation` (`excavation`),"
+                + "INDEX `idx_archery` (`archery`),"
+                + "INDEX `idx_swords` (`swords`),"
+                + "INDEX `idx_axes` (`axes`),"
+                + "INDEX `idx_acrobatics` (`acrobatics`),"
+                + "INDEX `idx_fishing` (`fishing`),"
+                + "INDEX `idx_alchemy` (`alchemy`),"
+                + "INDEX `idx_crossbows` (`crossbows`),"
+                + "INDEX `idx_tridents` (`tridents`),"
+                + "INDEX `idx_maces` (`maces`),"
+                + "INDEX `idx_spears` (`spears`),"
+                + "INDEX `idx_total` (`total`)) "
                 + "DEFAULT CHARSET=" + CHARSET_SQL + ";";
 
         try (Statement createStatement = connection.createStatement()) {
@@ -1496,6 +1522,7 @@ private void checkDatabaseStructure(Connection connection, UpgradeType upgrade)
                     checkNameUniqueness(statement);
                 }
                 case ADD_SKILL_TOTAL -> checkUpgradeSkillTotal(connection);
+                case ADD_SKILL_LEADERBOARD_INDEXES -> checkUpgradeSkillLeaderboardIndexes(connection);
                 case ADD_UNIQUE_PLAYER_DATA -> checkUpgradeAddUniqueChimaeraWing(statement);
                 case SQL_CHARSET_UTF8MB4 -> updateCharacterSet(statement);
                 default -> {
@@ -1752,6 +1779,112 @@ private void checkUpgradeSkillTotal(final Connection connection) throws SQLExcep
     }
 
 
+    /**
+     * Ensures a secondary index exists on each column used by leaderboard queries (every
+     * non-child skill plus {@code total}).
+     * <p>
+     * {@link #readLeaderboard} sorts on a single column per scope. Without an index the engine must
+     * scan and filesort the whole {@code skills} table. On MariaDB the query rewrite alone is not
+     * enough (its optimizer keeps a temporary + filesort); the index is what lets it drive the sort
+     * from the {@code skills} table. Verified on MySQL 8 and MariaDB 10.11.
+     * <p>
+     * Resilience: each index is attempted independently and best-effort. A failure on one column is
+     * logged and skipped so the remaining indexes still get created, and no failure is allowed to
+     * propagate and abort startup - mcMMO keeps running without the index (slower leaderboards) if
+     * the DDL cannot be applied. The upgrade is only marked complete once every index is present, so
+     * partial or failed runs are retried on the next startup.
+     * <p>
+     * Cost note: this is a one-time migration that can be slow on very large tables, and each index
+     * adds write amplification on {@code saveUser}. Existing indexes are skipped, so the step is
+     * idempotent and safe to re-run.
+     */
+    private void checkUpgradeSkillLeaderboardIndexes(final Connection connection) {
+        final List<String> leaderboardColumns = List.of(
+                "taming", "mining", "woodcutting", "repair", "unarmed", "herbalism", "excavation",
+                "archery", "swords", "axes", "acrobatics", "fishing", "alchemy", "crossbows",
+                "tridents", "maces", "spears", "total");
+
+        boolean allIndexesEnsured = true;
+
+        try {
+            connection.setAutoCommit(false);
+
+            try (Statement statement = connection.createStatement()) {
+                for (final String column : leaderboardColumns) {
+                    if (!ensureLeaderboardIndex(connection, statement, column)) {
+                        allIndexesEnsured = false;
+                    }
+                }
+            }
+        } catch (SQLException ex) {
+            // Whole-operation failure (e.g. could not toggle autocommit); log and keep running.
+            allIndexesEnsured = false;
+            logSQLException(ex);
+        } finally {
+            try {
+                connection.setAutoCommit(true);
+            } catch (SQLException ignored) {
+                // best effort
+            }
+        }
+
+        // Only mark complete when every index is in place, so failures retry on the next startup.
+        if (allIndexesEnsured) {
+            mcMMO.getUpgradeManager()
+                    .setUpgradeCompleted(UpgradeType.ADD_SKILL_LEADERBOARD_INDEXES);
+        }
+    }
+
+    /**
+     * Best-effort creation of a single leaderboard index. Skips when one already exists on the
+     * column. Commits on success, rolls back and logs on failure. Never throws.
+     *
+     * @return {@code true} if the index exists (already present or newly created), {@code false} if
+     * the attempt failed and the column remains unindexed.
+     */
+    private boolean ensureLeaderboardIndex(final Connection connection, final Statement statement,
+            final String column) {
+        try {
+            if (leaderboardIndexExists(statement, column)) {
+                return true;
+            }
+
+            logger.info("Adding leaderboard index for column: " + column);
+            statement.executeUpdate("ALTER TABLE `" + tablePrefix + "skills` ADD INDEX `idx_"
+                    + column + "` (`" + column + "`) USING BTREE");
+            connection.commit();
+            return true;
+        } catch (SQLException ex) {
+            logger.warning("Could not add leaderboard index for column '" + column
+                    + "', continuing without it (leaderboards for this column may be slower): "
+                    + ex.getMessage());
+            try {
+                connection.rollback();
+            } catch (SQLException ignored) {
+                // best effort
+            }
+            return false;
+        }
+    }
+
+    /**
+     * @return {@code true} if any index already exists whose column is the given column.
+     */
+    private boolean leaderboardIndexExists(final Statement statement, final String column)
+            throws SQLException {
+        // CREATE INDEX IF NOT EXISTS is not portable (unsupported on MySQL 8), so probe the
+        // information schema instead. DATABASE() scopes this to the active mcMMO schema.
+        final String query = "SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS "
+                + "WHERE table_schema = DATABASE() "
+                + "AND table_name = '" + tablePrefix + "skills' "
+                + "AND column_name = '" + column + "'";
+
+        try (ResultSet resultSet = statement.executeQuery(query)) {
+            return resultSet.next() && resultSet.getInt(1) > 0;
+        }
+    }
+
+
     private void checkUpgradeDropSpout(final Statement statement) {
         ResultSet resultSet = null;
 

src/main/java/com/gmail/nossr50/datatypes/database/UpgradeType.java

@@ -12,6 +12,7 @@ public enum UpgradeType {
     ADD_SCOREBOARD_TIPS,
     DROP_NAME_UNIQUENESS,
     ADD_SKILL_TOTAL,
+    ADD_SKILL_LEADERBOARD_INDEXES,
     ADD_UNIQUE_PLAYER_DATA,
     FIX_SPELLING_NETHERITE_SALVAGE,
     FIX_SPELLING_NETHERITE_REPAIR,

…aceholders/McTopPositionPlaceholder.java …/placeholders/McTopValuePlaceholder.javasrc/main/java/com/gmail/nossr50/placeholders/McTopPositionPlaceholder.java renamed to src/main/java/com/gmail/nossr50/placeholders/McTopValuePlaceholder.java src/main/java/com/gmail/nossr50/placeholders/McTopPositionPlaceholder.java renamed to src/main/java/com/gmail/nossr50/placeholders/McTopValuePlaceholder.java

@@ -9,7 +9,7 @@
  * {@code %mcmmo_mctop_overall:<position>%}, {@code %mcmmo_mctop_all:<position>%},
  * and {@code %mcmmo_mctop_powerlevel:<position>%}.
  */
-public class McTopPositionPlaceholder implements Placeholder {
+public class McTopValuePlaceholder implements Placeholder {
     private final @Nullable PrimarySkillType skill;
     private final String overallToken;
     private final LeaderboardPlaceholderCache leaderboardCache;
@@ -18,7 +18,7 @@ public class McTopPositionPlaceholder implements Placeholder {
      * @param skill Skill scope, or {@code null} for overall leaderboard.
      * @param leaderboardCache Shared leaderboard snapshot cache.
      */
-    public McTopPositionPlaceholder(@Nullable PrimarySkillType skill,
+    public McTopValuePlaceholder(@Nullable PrimarySkillType skill,
             LeaderboardPlaceholderCache leaderboardCache) {
         this(skill, "overall", leaderboardCache);
     }
@@ -29,7 +29,7 @@ public McTopPositionPlaceholder(@Nullable PrimarySkillType skill,
      *                     {@code overall}, {@code all}, or {@code powerlevel}.
      * @param leaderboardCache Shared leaderboard snapshot cache.
      */
-    public McTopPositionPlaceholder(@Nullable PrimarySkillType skill,
+    public McTopValuePlaceholder(@Nullable PrimarySkillType skill,
             String overallToken,
             LeaderboardPlaceholderCache leaderboardCache) {
         this.skill = skill;

src/main/java/com/gmail/nossr50/placeholders/PapiExpansion.java

@@ -258,7 +258,7 @@ protected void init() {
 
         for (PrimarySkillType skill : SkillTools.NON_CHILD_SKILLS) {
             // %mcmmo_mctop_<skillname>:<position>%
-            registerPlaceholder(new McTopPositionPlaceholder(skill, leaderboardPlaceholderCache));
+            registerPlaceholder(new McTopValuePlaceholder(skill, leaderboardPlaceholderCache));
 
             // %mcmmo_mctop_name_<skillname>:<position>%
             registerPlaceholder(new McTopNamePlaceholder(skill, leaderboardPlaceholderCache));
@@ -291,19 +291,19 @@ protected void init() {
         registerPlaceholder(new XpRatePlaceholder(this));
 
         // %mcmmo_mctop_overall:<position>%
-        registerPlaceholder(new McTopPositionPlaceholder(null, leaderboardPlaceholderCache));
+        registerPlaceholder(new McTopValuePlaceholder(null, leaderboardPlaceholderCache));
 
         // %mcmmo_mctop_name_overall:<position>%
         registerPlaceholder(new McTopNamePlaceholder(null, leaderboardPlaceholderCache));
 
         // %mcmmo_mctop_all:<position>%
-        registerPlaceholder(new McTopPositionPlaceholder(null, "all", leaderboardPlaceholderCache));
+        registerPlaceholder(new McTopValuePlaceholder(null, "all", leaderboardPlaceholderCache));
 
         // %mcmmo_mctop_name_all:<position>%
         registerPlaceholder(new McTopNamePlaceholder(null, "all", leaderboardPlaceholderCache));
 
         // %mcmmo_mctop_powerlevel:<position>%
-        registerPlaceholder(new McTopPositionPlaceholder(null, "powerlevel", leaderboardPlaceholderCache));
+        registerPlaceholder(new McTopValuePlaceholder(null, "powerlevel", leaderboardPlaceholderCache));
 
         // %mcmmo_mctop_name_powerlevel:<position>%
         registerPlaceholder(new McTopNamePlaceholder(null, "powerlevel", leaderboardPlaceholderCache));

src/main/resources/config.yml

@@ -90,6 +90,8 @@ General:
             # Do not set this setting too high as it requires storing entries in memory.
             Max_Tracked_Rank: 100
             # How often to refresh cached leaderboard data (in seconds).
+            # Note: the FlatFile backend recomputes leaderboards at most once every 10 minutes,
+            # so intervals shorter than that mainly affect SQL (MySQL/MariaDB) backends.
             Refresh_Interval_Seconds: 60
 
 #