Skip to content

Preserve SQL timeout causes in user storage #357

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
BenCodez merged 3 commits into from
Oct 3, 2026
Merged

Preserve SQL timeout causes in user storage #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b0fcd6c
Preserve SQL acquisition failures in user storage
BenCodez Oct 3, 2026
630aaff
Propagate PostgreSQL UUID lookup SQL failures
BenCodez Oct 3, 2026
2304bff
Preserve Bukkit shared SQL read and delete failures
BenCodez Oct 3, 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
47 changes: 34 additions & 13 deletions AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public boolean isUseBatchUpdates() {
}

public void loadData() {
try (Connection con = mysql.getConnectionManager().getConnection()) {
try (Connection con = mysql.getConnectionManager().getConnectionChecked()) {
useBatchUpdates = con != null && con.getMetaData().supportsBatchUpdates();
} catch (SQLException e) {
debug(e);
Expand Down Expand Up @@ -160,15 +160,15 @@ public boolean containsKeyQuery(String uuid) {
return false;
}
String sql = "SELECT 1 FROM " + qi(tableName) + " WHERE " + qi("uuid") + " = ?::uuid LIMIT 1;";
try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
PreparedStatement ps = conn.prepareStatement(sql)) {
ps.setString(1, uuid);
try (ResultSet rs = ps.executeQuery()) {
return rs.next();
}
} catch (SQLException e) {
debug(e);
return false;
throw new IllegalStateException("Failed to check UUID existence", e);
}
}

Expand Down Expand Up @@ -225,7 +225,7 @@ public void forEachUser(java.util.function.BiConsumer<UUID, ArrayList<Column>> p
+ " ASC " + " LIMIT " + pageSize + ";";
}

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement ps = conn.prepareStatement(sqlStr, ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_READ_ONLY)) {

Expand Down Expand Up @@ -435,7 +435,7 @@ public String getUUIDOrThrow(String playerName) throws SQLException {
String query = "SELECT " + qi("uuid") + " FROM " + qi(tableName) + " WHERE " + qi("PlayerName") + "=?;";
plugin.devDebug("DB QUERY: " + query);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(query)) {
sql.setString(1, playerName);
try (ResultSet rs = sql.executeQuery()) {
Expand All @@ -462,7 +462,7 @@ public ArrayList<Integer> getNumbersInColumn(String column) {
String sqlStr = "SELECT " + qi(column) + " FROM " + qi(tableName) + ";";
plugin.devDebug("DB QUERY: " + sqlStr);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(sqlStr);
ResultSet rs = sql.executeQuery()) {

Expand All @@ -481,7 +481,7 @@ public ArrayList<Column> getRowsNameQuery() {
String sqlStr = "SELECT " + qi("PlayerName") + " FROM " + qi(tableName) + ";";
plugin.devDebug("DB QUERY: " + sqlStr);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(sqlStr);
ResultSet rs = sql.executeQuery()) {

Expand All @@ -500,7 +500,7 @@ public ArrayList<Column> getRowsQuery() {
String sqlStr = "SELECT " + qi("uuid") + " FROM " + qi(tableName) + ";";
plugin.devDebug("DB QUERY: " + sqlStr);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(sqlStr);
ResultSet rs = sql.executeQuery()) {

Expand Down Expand Up @@ -529,7 +529,7 @@ public ConcurrentHashMap<UUID, String> getRowsUUIDNameQuery() {
String sqlStr = "SELECT " + qi("uuid") + ", " + qi("PlayerName") + " FROM " + qi(tableName) + ";";
plugin.devDebug("DB QUERY: " + sqlStr);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(sqlStr);
ResultSet rs = sql.executeQuery()) {

Expand Down Expand Up @@ -573,15 +573,26 @@ public ConcurrentHashMap<UUID, String> getRowsUUIDNameQuery() {
// -------------------------

public void deletePlayer(String uuid) {
deletePlayer(uuid, false);
}

/** Delete for shared storage: a failed operation must not acknowledge cache removal. */
public void deletePlayerStrict(String uuid) {
deletePlayer(uuid, true);
}

private void deletePlayer(String uuid, boolean strict) {
String q = "DELETE FROM " + qi(tableName) + " WHERE " + qi("uuid") + "=?;";
plugin.devDebug("DB QUERY: " + q);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Abort deletion when checked acquisition fails

If connection acquisition times out before the DELETE, the new checked call throws an SQLException that deletePlayer() catches, after which it still mutates the UUID/name caches and returns normally. BukkitSqlUserBackend.delete() therefore reports success to SharedUserDataRuntime.remove(), which removes the user cache even though the database row remains and can reappear on a later load; propagate the acquisition failure before changing cache state.

AGENTS.md reference: AGENTS.md:L76-L76

Useful? React with 👍 / 👎.

PreparedStatement statement = conn.prepareStatement(q)) {
bindUuid(statement, 1, uuid);
statement.executeUpdate();
} catch (SQLException | IllegalArgumentException e) {
debug(e);
if (strict) throw new IllegalStateException("Failed to delete SQL user", e);
return;
}

uuids.remove(uuid);
Expand All @@ -600,7 +611,7 @@ public void executeQuery(String str) {
}

public void executeQueryReturn(String str) {
try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(str);
ResultSet rs = sql.executeQuery()) {
plugin.devDebug("DB QUERY: " + str);
Expand All @@ -614,7 +625,7 @@ public HashMap<UUID, ArrayList<Column>> getAllQuery() {
String query = "SELECT * FROM " + qi(tableName) + ";";
plugin.devDebug("DB QUERY: " + query);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();
PreparedStatement sql = conn.prepareStatement(query);
ResultSet rs = sql.executeQuery()) {

Expand Down Expand Up @@ -680,14 +691,23 @@ public ArrayList<Column> getExact(String uuid) {
return getExactQuery(new Column("uuid", new DataValueString(uuid)));
}

/** Read for shared storage: never publish a schema-only snapshot after SQL failure. */
public ArrayList<Column> getExactStrict(String uuid) {
return getExactQuery(new Column("uuid", new DataValueString(uuid)), true);
}

public ArrayList<Column> getExactQuery(Column column) {
return getExactQuery(column, false);
}

private ArrayList<Column> getExactQuery(Column column, boolean strict) {
ArrayList<Column> result = new ArrayList<>();

String query = "SELECT * FROM " + qi(tableName) + " WHERE " + qi(column.getName()) + "=?;";

plugin.devDebug("DB QUERY: " + query);

try (Connection conn = mysql.getConnectionManager().getConnection();
try (Connection conn = mysql.getConnectionManager().getConnectionChecked();

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Propagate checked acquisition failures from Bukkit reads

When the pool is exhausted, getConnectionChecked() now throws SQLException, but the existing catch below logs it and returns a schema-only Column list. The production BukkitSqlUserBackend.read() delegates to getExact(), and SharedUserDataRuntime.populateInternal() publishes the resulting nonempty map as a valid snapshot, so callers observe null/default values while the original timeout is lost; use a throwing read path for the shared backend instead.

AGENTS.md reference: AGENTS.md:L76-L76

Useful? React with 👍 / 👎.

PreparedStatement sql = conn.prepareStatement(query)) {
if (dbType == DbType.POSTGRESQL && "uuid".equalsIgnoreCase(column.getName())) {
bindUuid(sql, 1, column.getValue().getString());
Expand Down Expand Up @@ -731,6 +751,7 @@ public ArrayList<Column> getExactQuery(Column column) {
}
} catch (SQLException | ArrayIndexOutOfBoundsException e) {
debug(e);
if (strict) throw new IllegalStateException("Failed to read SQL user", e);
}

for (String col : getColumns()) {
Expand Down
4 changes: 2 additions & 2 deletions ...ore/src/main/java/com/bencodez/advancedcore/bukkit/user/storage/BukkitSqlUserBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private IllegalStateException enumerationFailure(String storage, Throwable failu
private List<Column> read(UserStorage storage, UUID uuid) {
requireOpen();
requireStorage(storage);
if (storage == UserStorage.MYSQL) return mysql().getExact(uuid.toString());
if (storage == UserStorage.MYSQL) return mysql().getExactStrict(uuid.toString());
synchronized (sqliteOperations) { return table().getExact(primary(uuid)); }
}

Expand All @@ -147,7 +147,7 @@ private boolean contains(UserStorage storage, UUID uuid) {
private void delete(UserStorage storage, UUID uuid) {
requireOpen();
requireStorage(storage);
if (storage == UserStorage.MYSQL) mysql().deletePlayer(uuid.toString());
if (storage == UserStorage.MYSQL) mysql().deletePlayerStrict(uuid.toString());
else synchronized (sqliteOperations) { table().delete(primary(uuid)); }
}

Expand Down
24 changes: 12 additions & 12 deletions ...dCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public SqlUserStorage user(UUID uuid) {
requireAdmissionOpen();
JdbcSqlUserStorage.Dialect dialect = JdbcSqlUserStorage.Dialect.fromDbType(table.getMysql().getConnectionManager().getDbType());
SqlUserStorage delegate = new JdbcSqlUserStorage(UserStorage.MYSQL, uuid, table.getTableName(), schema,
() -> table.getMysql().getConnectionManager().getConnection(), dialect, logger);
() -> table.getMysql().getConnectionManager().getConnectionChecked(), dialect, logger);
return new SqlUserStorage() {
@Override public List<Column> readRow(UserStorage storage) { return withOperation(() -> delegate.readRow(storage)); }
@Override public boolean contains(UserStorage storage) { return withOperation(() -> delegate.contains(storage)); }
Expand Down Expand Up @@ -102,7 +102,7 @@ private List<UserPageEntry> readUserPage(String cursor) {
+ (cursor == null ? "" : " AND " + uuidColumn + " > ?")
+ " ORDER BY " + uuidColumn + " ASC LIMIT ?";
JdbcSqlUserStorage.Dialect dialect = JdbcSqlUserStorage.Dialect.fromDbType(table.getDbType());
try (Connection connection = table.getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) {
try (Connection connection = table.getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) {
int index = 1;
if (cursor != null) {
if (dialect == JdbcSqlUserStorage.Dialect.POSTGRESQL) dialect.bindUuid(statement, index++, UUID.fromString(cursor)); else statement.setString(index++, cursor);
Expand Down Expand Up @@ -239,7 +239,7 @@ void ensureUuidType() {
+ uuidType + " USING NULLIF(" + uuidColumn + ", '')::uuid;"
: "ALTER TABLE " + quote(tableName) + " MODIFY " + uuidColumn + " "
+ normaliseTypeForDb(uuidType) + ";";
try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
catch (SQLException ddlFailure) {
try { if (columnNeedsAlter(SqlUserSchema.UUID_COLUMN, uuidType)) throw ddlFailure; }
catch (SQLException inspectionFailure) {
Expand All @@ -251,7 +251,7 @@ void ensureUuidType() {
}

void ensureUuidUnique() {
try (Connection connection = getMysql().getConnectionManager().getConnection()) {
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked()) {
java.sql.DatabaseMetaData metadata = connection.getMetaData();
if (metadata == null || hasUniqueUuidConstraint(connection, metadata)) return;
String indexName = tableName + "_uuid_unique";
Expand Down Expand Up @@ -326,7 +326,7 @@ void ensureColumn(SqlUserSchema.ColumnDefinition column) {
rememberColumn(column); return;
}
String sql = "ALTER TABLE " + quote(tableName) + " ADD COLUMN " + quote(column.name()) + " " + normaliseTypeForDb(column.sqlType()) + ";";
try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
catch (SQLException ddlFailure) {
if (!isDuplicateColumn(ddlFailure)) throw ddlFailure;
try {
Expand All @@ -347,7 +347,7 @@ void ensureColumn(SqlUserSchema.ColumnDefinition column) {

private void renamePostgresColumn(String storedName, String requestedName) throws SQLException {
String sql = "ALTER TABLE " + quote(tableName) + " RENAME COLUMN " + quote(storedName) + " TO " + quote(requestedName) + ";";
try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); }
catch (SQLException renameFailure) { String current = findRegisteredColumn(requestedName); if (!requestedName.equals(current)) throw renameFailure; }
}

Expand Down Expand Up @@ -390,7 +390,7 @@ private void migrateRetainedColumnToString(String storedName,
+ (attributes.comment() == null || attributes.comment().isEmpty() ? ""
: " COMMENT '" + quoteMysqlLiteral(attributes.comment()) + "'")
+ ";";
try (Connection connection = getMysql().getConnectionManager().getConnection();
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked();
PreparedStatement statement = connection.prepareStatement(sql)) {
statement.executeUpdate();
} catch (SQLException ddlFailure) {
Expand All @@ -412,7 +412,7 @@ private void migrateRetainedColumnToString(String storedName,
.append(" SET DEFAULT (").append(defaultExpression).append(")::text")
.append(declaredStorageType ? "::" + targetType : "");
sql.append(';');
try (Connection connection = getMysql().getConnectionManager().getConnection();
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked();
PreparedStatement statement = connection.prepareStatement(sql.toString())) {
statement.executeUpdate();
} catch (SQLException ddlFailure) {
Expand Down Expand Up @@ -585,7 +585,7 @@ private static int[] declaredTypeParameters(String sqlType) {
private MysqlColumnAttributes mysqlColumnAttributes(String name) throws SQLException {
String sql = "SELECT IS_NULLABLE, COLUMN_DEFAULT, EXTRA, COLUMN_COMMENT, COLUMN_TYPE "
+ "FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME=? AND COLUMN_NAME=?";
try (Connection connection = getMysql().getConnectionManager().getConnection();
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked();
PreparedStatement statement = connection.prepareStatement(sql)) {
statement.setString(1, tableName);
statement.setString(2, name);
Expand Down Expand Up @@ -643,7 +643,7 @@ private String postgresColumnDefault(String name) throws SQLException {
+ "AND default_value.adnum=attribute.attnum "
+ "WHERE attribute.attrelid=pg_catalog.to_regclass(?) AND attribute.attname=? "
+ "AND attribute.attnum>0 AND NOT attribute.attisdropped";
try (Connection connection = getMysql().getConnectionManager().getConnection();
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked();
PreparedStatement statement = connection.prepareStatement(sql)) {
statement.setString(1, regclass);
statement.setString(2, name);
Expand All @@ -654,7 +654,7 @@ private String postgresColumnDefault(String name) throws SQLException {
}

private RegisteredColumnType registeredColumnType(String name) throws SQLException {
try (Connection connection = getMysql().getConnectionManager().getConnection();
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked();
PreparedStatement statement = connection.prepareStatement(
"SELECT * FROM " + quote(tableName) + " WHERE 1=0");
ResultSet result = statement.executeQuery()) {
Expand All @@ -675,7 +675,7 @@ private void rememberColumn(SqlUserSchema.ColumnDefinition column) {
}

private String findRegisteredColumn(String name) throws SQLException {
try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + quote(tableName) + " WHERE 1=0"); ResultSet result = statement.executeQuery()) {
try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + quote(tableName) + " WHERE 1=0"); ResultSet result = statement.executeQuery()) {
ResultSetMetaData metadata = result.getMetaData(); String exactMatch = null; String foldedMatch = null; int foldedMatches = 0;
for (int i = 1; i <= metadata.getColumnCount(); i++) {
String storedName = metadata.getColumnName(i);
Expand Down
33 changes: 33 additions & 0 deletions ...ava/com/bencodez/advancedcore/core/user/storage/sql/MysqlBackendReviewRegressionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.sql.SQLTransientConnectionException;
import java.util.UUID;
import com.bencodez.advancedcore.api.user.UserStorage;
import com.bencodez.simpleapi.sql.data.DataValueInt;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -119,6 +123,35 @@ private void existingSpelling(DbType type, String name) throws Exception {
fixture.assertClosed();
}

@Test void sqlRequiredOperationsPreserveAcquisitionTimeoutAndRecover() throws Exception {
for (DbType type : new DbType[]{DbType.MYSQL, DbType.MARIADB, DbType.POSTGRESQL}) {
Fixture fixture = new Fixture(type, "Points");
try (var managers = fixture.managers(); var backend = fixture.open()) {
ConnectionManager manager = managers.constructed().get(0);
var user = backend.user(UUID.randomUUID());
SQLTransientConnectionException timeout = new SQLTransientConnectionException("Pool exhausted", "08001");
doReturn(null).when(manager).getConnection();
doThrow(timeout).when(manager).getConnectionChecked();
List<org.junit.jupiter.api.function.Executable> operations = List.of(
() -> user.readRow(UserStorage.MYSQL),
() -> user.contains(UserStorage.MYSQL),
() -> user.delete(UserStorage.MYSQL),
() -> user.write(UserStorage.MYSQL, "Points", new DataValueInt(7)),
() -> user.transaction(UserStorage.MYSQL, transaction -> fail("No transaction work before acquisition")),
backend::enumerateUsers);
for (var operation : operations) {
assertSame(timeout, assertThrows(IllegalStateException.class, operation).getCause(), type.name());
assertTrue(backend.isOpen(), "Timeout must not close backend");
}
doAnswer(ignored -> fixture.connection()).when(manager).getConnectionChecked();
assertTrue(user.readRow(UserStorage.MYSQL).isEmpty());
assertFalse(user.contains(UserStorage.MYSQL));
assertTrue(backend.enumerateUsers().isEmpty());
}
fixture.assertClosed();
}
}

private static final class Fixture {
final DbType type;
final String storedColumn;
Expand Down
Loading
Loading