diskquota/jdbc: retry SERIALIZABLE aborts (#1527)

Concurrent writers hitting the same TILESET or TILEPAGE row produced
serialization aborts that escaped JDBCQuotaStore and reached
QueuedQuotaUpdatesConsumer, which logs at FINE and drops the
aggregated batch, silently losing pending quota updates and letting
the on-disk ledger drift out of sync. SERIALIZABLE isolation is kept
as-is; the missing piece was the application-level retry that
SERIALIZABLE assumes.

Wrap every JDBCQuotaStore transaction in a bounded-retry helper whose
predicate recognises serialization aborts from Postgres SSI, HSQL
(SQLState class "40"), and Oracle (vendor codes 8176/8177). Oracle
additionally gets a DEFERRABLE INITIALLY DEFERRED TILEPAGE -> TILESET
foreign key, the matching migrate-on-upgrade path, and a PL/SQL
renameLayer so TILESET.KEY and TILEPAGE.TILESET_ID rewrite atomically
at commit. Verified end-to-end against PostgreSQL and Oracle XE
testcontainers.

on-behalf-of: @camptocamp <info@camptocamp.com>

Author: groldan

geowebcache/diskquota/jdbc/src/main/java/org/geowebcache/diskquota/jdbc/JDBCQuotaStore.java

@@ -29,6 +29,8 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Consumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.sql.DataSource;
@@ -51,6 +53,7 @@
 import org.springframework.jdbc.datasource.DataSourceTransactionManager;
 import org.springframework.transaction.TransactionStatus;
 import org.springframework.transaction.support.TransactionCallback;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.transaction.support.TransactionTemplate;
 
 /**
@@ -86,6 +89,20 @@ public class JDBCQuotaStore implements QuotaStore {
     /** Max number of attempts we do to insert/update page stats in race-free mode */
     int maxLoops = 100;
 
+    /** Max attempts in {@link #executeWithRetry(TransactionCallback)} before propagating the abort. */
+    int maxTransactionAttempts = 10;
+
+    /** Initial backoff between transaction retries, in milliseconds; doubles each retry, with full jitter. */
+    long initialTransactionBackoffMs = 10L;
+
+    private static final long MAX_TRANSACTION_BACKOFF_MS = 500L;
+
+    /** Oracle ORA-08176: consistent read failure; rollback data not available. */
+    private static final int ORA_08176 = 8176;
+
+    /** Oracle ORA-08177: can't serialize access for this transaction. */
+    private static final int ORA_08177 = 8177;
+
     /** The executor used for asynch requests */
     ExecutorService executor;
 
@@ -159,10 +176,10 @@ public void initialize() {
             throw new IllegalStateException(
                     "Please provide both the sql dialect and the data " + "source before calling inizialize");
         }
-        tt.executeWithoutResult(status -> {
-
-            // setup the tables if necessary
-            dialect.initializeTables(schema, jt);
+        // DDL must run outside the wrapping transaction: Oracle auto-commits it and a SERIALIZABLE
+        // read across the just-created indexes would abort with ORA-08176 on the first SELECT.
+        dialect.initializeTables(schema, jt);
+        executeWithRetry(status -> {
 
             // get the existing table names
             List<String> existingLayers = jt.query(dialect.getAllLayersQuery(schema), (rs, rowNum) -> rs.getString(1));
@@ -196,7 +213,7 @@ public void createLayer(String layerName) throws InterruptedException {
     }
 
     private void createLayerInternal(final String layerName) {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             Set<TileSet> layerTileSets;
             if (!GLOBAL_QUOTA_NAME.equals(layerName)) {
                 layerTileSets = calculator.getTileSetsFor(layerName);
@@ -276,14 +293,14 @@ private Quota nonNullQuota(Quota optionalQuota) {
 
     @Override
     public void deleteLayer(final String layerName) {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             deleteLayerInternal(layerName);
         });
     }
 
     @Override
     public void deleteGridSubset(final String layerName, final String gridSetId) {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             // get the disk quota used by the layer gridset
             Quota quota = getUsedQuotaByLayerGridset(layerName, gridSetId);
             // we will subtracting the current disk quota value
@@ -305,7 +322,7 @@ public void deleteGridSubset(final String layerName, final String gridSetId) {
 
     public void deleteLayerInternal(final String layerName) {
         getUsedQuotaByLayerName(layerName);
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             // update the global quota
             Quota quota = getUsedQuotaByLayerName(layerName);
             quota.setBytes(quota.getBytes().negate());
@@ -324,7 +341,7 @@ public void deleteLayerInternal(final String layerName) {
 
     @Override
     public void renameLayer(final String oldLayerName, final String newLayerName) throws InterruptedException {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             String sql = dialect.getRenameLayerStatement(schema, "oldName", "newName");
             Map<String, Object> params = new HashMap<>();
             params.put("oldName", oldLayerName);
@@ -429,7 +446,7 @@ public TilePageCalculator getTilePageCalculator() {
     public void addToQuotaAndTileCounts(
             final TileSet tileSet, final Quota quotaDiff, final Collection<PageStatsPayload> tileCountDiffs)
             throws InterruptedException {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             getOrCreateTileSet(tileSet);
             updateQuotas(tileSet, quotaDiff);
 
@@ -609,7 +626,7 @@ private PageStats getPageStats(String pageStatsKey) {
     @Override
     @SuppressWarnings("unchecked")
     public Future<List<PageStats>> addHitsAndSetAccesTime(final Collection<PageStatsPayload> statsUpdates) {
-        return executor.submit(() -> (List<PageStats>) tt.execute(new QuotaStoreCallback(statsUpdates)));
+        return executor.submit(() -> (List<PageStats>) executeWithRetry(new QuotaStoreCallback(statsUpdates)));
     }
 
     @Override
@@ -651,7 +668,7 @@ private TilePage getSinglePage(Set<String> layerNames, boolean leastFrequentlyUs
 
     @Override
     public PageStats setTruncated(final TilePage page) throws InterruptedException {
-        return (PageStats) tt.execute((TransactionCallback<Object>) status -> {
+        return (PageStats) executeWithRetry((TransactionCallback<Object>) status -> {
             if (log.isLoggable(Level.FINE)) {
                 log.info("Truncating page " + page);
             }
@@ -693,6 +710,88 @@ public void close() throws Exception {
         jt = null;
     }
 
+    /**
+     * Runs {@code action} in a SERIALIZABLE transaction, retrying on concurrency aborts with bounded exponential
+     * backoff. If the call is already nested inside an active transaction the retry loop is skipped: Spring's
+     * {@code PROPAGATION_REQUIRED} would reuse the same stale snapshot, so only the outermost call can recover.
+     */
+    private <T> T executeWithRetry(TransactionCallback<T> action) {
+        if (TransactionSynchronizationManager.isActualTransactionActive()) {
+            return tt.execute(action);
+        }
+        long backoff = initialTransactionBackoffMs;
+        for (int attempt = 1; ; attempt++) {
+            try {
+                return tt.execute(action);
+            } catch (DataAccessException e) {
+                if (!isTransactionAbort(e)) {
+                    throw e;
+                }
+                if (attempt >= maxTransactionAttempts) {
+                    log.log(
+                            Level.WARNING,
+                            "DiskQuota transaction failed after " + attempt + " attempts: " + e.getMessage(),
+                            e);
+                    throw e;
+                }
+                long sleep = backoff + ThreadLocalRandom.current().nextLong(backoff);
+                try {
+                    Thread.sleep(sleep);
+                } catch (InterruptedException ie) {
+                    Thread.currentThread().interrupt();
+                    throw e;
+                }
+                if (log.isLoggable(Level.FINE)) {
+                    log.fine("DiskQuota transaction conflict on attempt "
+                            + attempt
+                            + "/"
+                            + maxTransactionAttempts
+                            + ", retrying after "
+                            + sleep
+                            + "ms: "
+                            + e.getMessage());
+                }
+                backoff = Math.min(backoff * 2, MAX_TRANSACTION_BACKOFF_MS);
+            }
+        }
+    }
+
+    /** Void variant of {@link #executeWithRetry(TransactionCallback)}. */
+    private void executeWithRetry(Consumer<TransactionStatus> action) {
+        executeWithRetry((TransactionCallback<Void>) status -> {
+            action.accept(status);
+            return null;
+        });
+    }
+
+    /**
+     * Walks the cause chain looking for a retryable concurrency abort. Spring's translator alone is not enough:
+     * SQLSTATE class {@code 40} catches HSQL's bare {@link ConcurrencyFailureException}, and Oracle vendor codes 8176
+     * and 8177 are needed because Spring leaves 8176 uncategorized and routes 8177 to a deprecated sibling of
+     * {@link PessimisticLockingFailureException}.
+     */
+    private static boolean isTransactionAbort(Throwable t) {
+        for (Throwable cause = t; cause != null; cause = cause.getCause()) {
+            if (cause instanceof PessimisticLockingFailureException) {
+                return true;
+            }
+            if (cause instanceof SQLException sqlException) {
+                String sqlState = sqlException.getSQLState();
+                if (sqlState != null && sqlState.startsWith("40")) {
+                    return true;
+                }
+                if (isRetryableOracleCode(sqlException.getErrorCode())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    private static boolean isRetryableOracleCode(int errorCode) {
+        return errorCode == ORA_08176 || errorCode == ORA_08177;
+    }
+
     /**
      * Maps a BigDecimal column into a Quota object
      *
@@ -752,7 +851,7 @@ public TilePage mapRow(ResultSet rs, int rowNum) throws SQLException {
 
     @Override
     public void deleteParameters(final String layerName, final String parametersId) {
-        tt.executeWithoutResult(status -> {
+        executeWithRetry(status -> {
             // first gather the disk quota used by the gridset, and update the global
             // quota
             Quota quota = getUsedQuotaByParametersId(parametersId);

geowebcache/diskquota/jdbc/src/main/java/org/geowebcache/diskquota/jdbc/OracleDialect.java

@@ -13,6 +13,9 @@
  */
 package org.geowebcache.diskquota.jdbc;
 
+import java.sql.DatabaseMetaData;
+import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.List;
 
 /**
@@ -60,7 +63,8 @@ BYTES NUMBER(%d) DEFAULT 0 NOT NULL
                         """
                         CREATE TABLE ${schema}TILEPAGE (
                          KEY VARCHAR(%d) PRIMARY KEY,
-                         TILESET_ID VARCHAR(%d) REFERENCES ${schema}TILESET(KEY) ON DELETE CASCADE,
+                         TILESET_ID VARCHAR(%d) REFERENCES ${schema}TILESET(KEY) ON DELETE CASCADE
+                           DEFERRABLE INITIALLY DEFERRED,
                          PAGE_Z SMALLINT,
                          PAGE_X INTEGER,
                          PAGE_Y INTEGER,
@@ -84,37 +88,46 @@ protected void addEmtpyTableReference(StringBuilder sb) {
     }
 
     /**
-     * No-op: Oracle does not support {@code ON UPDATE CASCADE} on foreign keys, so there is nothing portable to
-     * migrate. Companion to {@link #getRenameLayerStatement(String, String, String)}, which preserves the legacy
-     * LAYER_NAME-only behavior on this dialect.
+     * Oracle does not support {@code ON UPDATE CASCADE}, so the FK is migrated to {@code DEFERRABLE INITIALLY DEFERRED}
+     * instead. Deferring the check to commit time also drops the per-INSERT snapshot read on TILESET that triggers
+     * ORA-08176 under SERIALIZABLE.
      */
     @Override
-    public void migrateForeignKeys(String schema, SimpleJdbcTemplate template) {
-        // intentional no-op
+    protected boolean tilepageFkIsMigrated(ResultSet rs) throws SQLException {
+        return rs.getShort("DEFERRABILITY") == DatabaseMetaData.importedKeyInitiallyDeferred;
+    }
+
+    @Override
+    protected String tilepageFkAddSql(String prefixedTilepageName, String prefix) {
+        return """
+                ALTER TABLE %s ADD FOREIGN KEY (TILESET_ID)
+                REFERENCES %sTILESET(KEY)
+                ON DELETE CASCADE
+                DEFERRABLE INITIALLY DEFERRED
+                """
+                .formatted(prefixedTilepageName, prefix);
     }
 
     /**
-     * Oracle does not support {@code ON UPDATE CASCADE} on foreign keys, so the {@code TILEPAGE.TILESET_ID -> TILESET
-     * .KEY} FK declared above only cascades on delete. As a result this dialect cannot safely rewrite {@code TILESET
-     * .KEY} during a rename without first dealing with the dangling {@code TILEPAGE} rows.
-     *
-     * <p>For now Oracle keeps the legacy behavior of only updating {@code LAYER_NAME}; lookups by id against the
-     * renamed layer will continue to miss the row and cause {@code getOrCreateTileSet} to insert duplicates. Fixing
-     * this on Oracle (e.g. via {@code DEFERRABLE INITIALLY DEFERRED} constraints, or by disabling the FK around the
-     * rename) is tracked separately.
+     * PL/SQL anonymous block that rewrites TILESET.KEY and TILEPAGE.TILESET_ID together; the deferred FK is checked
+     * once at commit with both updates in place. Oracle has no {@code ON UPDATE CASCADE}, hence the manual rewrite, and
+     * no SQL-standard {@code SUBSTRING ... FROM POSITION(...)}, hence {@code SUBSTR}/{@code INSTR}.
      */
     @Override
     public String getRenameLayerStatement(String schema, String oldLayerName, String newLayerName) {
-        StringBuilder sb = new StringBuilder("UPDATE ");
-        if (schema != null) {
-            sb.append(schema).append(".");
-        }
-        sb.append("TILESET SET LAYER_NAME = :")
-                .append(newLayerName)
-                .append(" WHERE LAYER_NAME = :")
-                .append(oldLayerName);
-
-        return sb.toString();
+        String prefix = schema == null ? "" : schema + ".";
+        return """
+                BEGIN
+                  UPDATE %sTILESET
+                    SET KEY = :%s || SUBSTR(KEY, INSTR(KEY, '#')),
+                        LAYER_NAME = :%s
+                    WHERE LAYER_NAME = :%s;
+                  UPDATE %sTILEPAGE
+                    SET TILESET_ID = :%s || SUBSTR(TILESET_ID, INSTR(TILESET_ID, '#'))
+                    WHERE INSTR(TILESET_ID, :%s || '#') = 1;
+                END;
+                """
+                .formatted(prefix, newLayerName, newLayerName, oldLayerName, prefix, newLayerName, oldLayerName);
     }
 
     @Override