From c1f8fe3779c51229ac7f6fc878d729c87c82b7be Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 8 Sep 2026 09:37:06 +0200 Subject: [PATCH] OF-3364: Limit MUC history reload query at the database level MUCPersistenceManager#loadHistory previously fetched all rows since the configured reload window and then scrolled/trimmed the ResultSet client-side to keep only the last maxNumber messages. This relied on scrollable, TYPE_SCROLL_INSENSITIVE result sets, which are not reliably supported across all database backends (e.g. disabled outright for CockroachDB), and could transfer far more data than was ultimately retained for rooms with long histories. Instead, build the query dynamically using DbConnectionManager's result-set-limit facilities (LIMIT / TOP / FETCH FIRST ... ROWS ONLY, per database type) so the limit is pushed down to the database. Also adds debug logging around the reload window and returned row count to make future over-fetching easier to spot. (cherry picked from commit 69b8f1d3cb098ea5aec311e4965aaa2a198377c5) --- .../muc/spi/MUCPersistenceManager.java | 76 +++++++++++++++---- 1 file changed, 60 insertions(+), 16 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java index f9a5ca1274..bc64ae4560 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java @@ -108,9 +108,6 @@ public class MUCPersistenceManager { "SELECT jid, affiliation FROM ofMucAffiliation WHERE roomID=?"; private static final String LOAD_MEMBERS = "SELECT jid, nickname FROM ofMucMember WHERE roomID=?"; - private static final String LOAD_HISTORY = - "SELECT sender, nickname, logTime, subject, body, stanza FROM ofMucConversationLog " + - "WHERE logTime>? AND roomID=? AND (nickname IS NOT NULL OR subject IS NOT NULL) ORDER BY logTime"; private static final String RELOAD_ALL_ROOMS_WITH_RECENT_ACTIVITY = "SELECT roomID, creationDate, modificationDate, name, naturalName, description, " + "lockedDate, emptyDate, canChangeSubject, maxUsers, publicRoom, moderated, membersOnly, " + @@ -953,6 +950,7 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) { Log.debug("Loading room history for room '{}' (max: {})", room.getJID(), maxNumber == -1 ? "all" : maxNumber); + final boolean applyLimit = maxNumber > -1; final List oldMessages = new LinkedList<>(); if (room.isLogEnabled() && maxNumber != 0) { @@ -962,7 +960,8 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) try { // Reload historic messages from the database. con = DbConnectionManager.getConnection(); - pstmt = con.prepareStatement(LOAD_HISTORY, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + final String sql = buildHistoryQuery(maxNumber); + pstmt = con.prepareStatement(sql); // Reload the history, using "muc.history.reload.limit" (days) if present long from = 0; @@ -974,22 +973,23 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) from = System.currentTimeMillis() - (BigInteger.valueOf(86400000).multiply(BigInteger.valueOf(reloadLimitDays))).longValue(); } - pstmt.setString(1, StringUtils.dateToMillis(new Date(from))); - pstmt.setLong(2, room.getID()); + int paramIndex = 1; + if (applyLimit && DbConnectionManager.isResultSetLimitKeywordPrefix()) { + pstmt.setInt(paramIndex++, maxNumber); + } + pstmt.setString(paramIndex++, StringUtils.dateToMillis(new Date(from))); + pstmt.setLong(paramIndex++, room.getID()); + if (applyLimit && !DbConnectionManager.isResultSetLimitKeywordPrefix()) { + pstmt.setInt(paramIndex++, maxNumber); + } + + Log.debug("Executing bounded history query for room '{}' using SQL: {}", room.getJID(), sql); + rs = pstmt.executeQuery(); // When reloading history, make sure that the old data is removed from memory before re-adding it. room.getRoomHistory().purge(); - try { - if (maxNumber > -1 && rs.last()) { - // Try to skip to the last few rows from the result set. - rs.relative(maxNumber * -1); - } - } catch (SQLException e) { - Log.debug("Unable to skip to the last {} rows of the result set.", maxNumber, e); - } - while (rs.next()) { String senderJID = rs.getString("sender"); String nickname = rs.getString("nickname"); @@ -999,6 +999,14 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) String stanza = rs.getString("stanza"); oldMessages.add(room.getRoomHistory().parseHistoricMessage(senderJID, nickname, sentDate, subject, body, stanza)); } + + if (applyLimit) { + // Rows came back newest-first (DESC) so the LIMIT/TOP/FETCH FIRST kept the last N messages. + // Reverse to restore chronological order before they're added to history. + Collections.reverse(oldMessages); + } + + Log.debug("Room '{}': database returned {} rows for the bounded history query (limit: {}).", room.getJID(), oldMessages.size(), applyLimit ? maxNumber : "none"); } finally { DbConnectionManager.closeConnection(rs, pstmt, con); } @@ -1008,8 +1016,8 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) if (!oldMessages.isEmpty()) { room.getRoomHistory().addOldMessages(oldMessages); } + Log.debug("Loaded {} messages() of room history for room '{}' (max: {})", oldMessages.size(), room.getJID(), maxNumber == -1 ? "all" : maxNumber); - // If the room does not include the last subject in the history, then recreate one if possible. if (!room.getRoomHistory().hasChangedSubject() && room.getSubject() != null && !room.getSubject().isEmpty()) { final Message subject = room.getRoomHistory().parseHistoricMessage(room.getSelfRepresentation().getOccupantJID().toString(), null, room.getModificationDate(), room.getSubject(), null, null); @@ -1017,6 +1025,42 @@ public static void loadHistory(@Nonnull final MUCRoom room, final int maxNumber) } } + /** + * Builds the SQL for loading room history, applying a database-appropriate row limit when maxNumber is + * non-negative. When a limit is applied, rows are ordered by logTime DESC (most recent first) so the limit keeps + * the *last* N messages; callers must reverse the result to restore chronological order. + */ + private static String buildHistoryQuery(final int maxNumber) + { + final boolean applyLimit = maxNumber > -1; + final StringBuilder sql = new StringBuilder("SELECT "); + + if (applyLimit && DbConnectionManager.isResultSetLimitKeywordPrefix()) { + sql.append(DbConnectionManager.getResultSetLimitKeyword().name()).append(" (?) "); + } + + sql.append("sender, nickname, logTime, subject, body, stanza FROM ofMucConversationLog ") + .append("WHERE logTime>? AND roomID=? AND (nickname IS NOT NULL OR subject IS NOT NULL) ") + .append("ORDER BY logTime ").append(applyLimit ? "DESC" : "ASC"); + + if (applyLimit && !DbConnectionManager.isResultSetLimitKeywordPrefix()) + { + final DbConnectionManager.ResultSetLimitKeyword keyword = DbConnectionManager.getResultSetLimitKeyword(); + switch (keyword) { + case LIMIT: + sql.append(" LIMIT ?"); + break; + case FETCH_FIRST: + sql.append(" FETCH FIRST ? ROWS ONLY"); + break; + default: + throw new IllegalStateException("Unexpected non-prefix result-set limit keyword: " + keyword); + } + } + + return sql.toString(); + } + /** * Updates the room's subject in the database. *