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,30 @@ 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 when a {@code SERIALIZABLE} transaction aborts due to a concurrency conflict
+     * ({@link PessimisticLockingFailureException}).
+     *
+     * <p>Postgres SSI and Oracle ORA-08176 abort transactions that the application is expected to retry; this counter
+     * bounds how many times {@link #executeWithRetry(TransactionCallback)} re-attempts before giving up.
+     */
+    int maxTransactionAttempts = 10;
+
+    /**
+     * Initial backoff between transaction retries, in milliseconds. Doubles on each retry up to
+     * {@link #MAX_TRANSACTION_BACKOFF_MS}, with full jitter added to spread concurrent retriers.
+     */
+    long initialTransactionBackoffMs = 10L;
+
+    /** Cap on the exponential backoff between transaction retries, in milliseconds. */
+    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 +186,12 @@ 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 outside any wrapping transaction: most engines auto-commit DDL, but Oracle in
+        // particular needs the post-DDL SCN to settle before any SERIALIZABLE transaction
+        // starts reading - otherwise the first SELECT inside the wrapped block crosses
+        // recently-created indexes and aborts with ORA-08176.
+        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 +225,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 +305,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 +334,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 +353,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 +458,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 +638,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 +680,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 +722,122 @@ public void close() throws Exception {
         jt = null;
     }
 
+    /**
+     * Runs the given action in a SERIALIZABLE transaction, retrying with bounded exponential backoff if the underlying
+     * database aborts the transaction due to a concurrency conflict.
+     *
+     * <p>Postgres SSI ({@code SQLSTATE 40001}, translated by Spring to {@link PessimisticLockingFailureException}) and
+     * Oracle's analogous serialization failures are documented as retryable: SERIALIZABLE is defined in terms of
+     * application-level retry on abort. Without this layer, the queued-update consumer thread silently swallows the
+     * abort, dropping the batch of pending quota updates and letting the ledger drift out of sync with disk.
+     *
+     * <p>Non-concurrency exceptions are not retried. Other {@link ConcurrencyFailureException} subclasses thrown by
+     * inner SQL-level loops (their own race-handling exhaustion) are also not retried here; those loops have already
+     * done their work and re-attempting at the transaction level would only amplify cost.
+     *
+     * <p>When invoked from a method that is already inside a wrapping transaction (e.g. {@link #createLayerInternal}
+     * called from {@link #initialize}), the retry loop is skipped and the exception is allowed to propagate. Spring's
+     * {@code PROPAGATION_REQUIRED} reuses the outer transaction, so retrying here would just re-fail against the same
+     * stale snapshot - the only retry that can recover is the outer one, which will start a fresh transaction.
+     */
+    private <T> T executeWithRetry(TransactionCallback<T> action) {
+        if (TransactionSynchronizationManager.isActualTransactionActive()) {
+            // Nested call: let aborts bubble up to the outer retry, which can take a fresh snapshot.
+            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)} for the majority of call sites that don't return a
+     * value from the transaction. The {@link TransactionStatus} is passed through so callers can flag the transaction
+     * for rollback explicitly if needed.
+     */
+    private void executeWithRetry(Consumer<TransactionStatus> action) {
+        executeWithRetry((TransactionCallback<Void>) status -> {
+            action.accept(status);
+            return null;
+        });
+    }
+
+    /**
+     * Returns {@code true} if any throwable in the cause chain is a concurrency-driven transaction abort that the
+     * application is expected to retry. {@link org.geowebcache.diskquota.jdbc.SimpleJdbcTemplate SimpleJdbcTemplate}
+     * wraps Spring's translated exceptions in {@link ParametricDataAccessException}, so the actual abort type is
+     * typically a cause, not the top-level throwable.
+     *
+     * <p>Recognizes three families:
+     *
+     * <ul>
+     *   <li>Spring's {@link PessimisticLockingFailureException} (covers Postgres SSI aborts; Spring translates SQLSTATE
+     *       40001 to {@link org.springframework.dao.CannotAcquireLockException}).
+     *   <li>Any {@link SQLException} whose {@code SQLState} class is {@code "40"} (transaction rollback, including
+     *       serialization failures and deadlocks). Catches HSQL's {@link java.sql.SQLTransactionRollbackException},
+     *       which Spring translates to a bare {@link ConcurrencyFailureException} rather than to one of the
+     *       pessimistic-locking subclasses.
+     *   <li>Oracle vendor error codes that are explicit serialization-retry signals: ORA-08176 ({@code consistent read
+     *       failure; rollback data not available}) and ORA-08177 ({@code can't serialize access for this transaction}).
+     *       Spring's translator routes 08177 to the now-deprecated {@code CannotSerializeTransactionException} (a
+     *       sibling of {@code PessimisticLockingFailureException}, not a subclass) and leaves 08176 uncategorized;
+     *       matching by JDBC vendor code keeps the dialect-specific knowledge local to this predicate.
+     * </ul>
+     */
+    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 +897,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,53 @@ 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} on foreign keys, so its TILEPAGE -> TILESET FK is migrated to
+     * {@code DEFERRABLE INITIALLY DEFERRED} instead. Deferring the FK check to commit time both lets
+     * {@link #getRenameLayerStatement(String, String, String) renaming a layer} rewrite TILESET.KEY (and its dependent
+     * TILEPAGE.TILESET_ID rows) atomically, and removes the snapshot read against TILESET that fires on every plain
+     * INSERT INTO TILEPAGE - the primary trigger for 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.
+     * Oracle does not support {@code ON UPDATE CASCADE} on foreign keys, so the rename cannot rely on the FK to
+     * propagate the new {@code TILESET.KEY} prefix to {@code TILEPAGE.TILESET_ID}. Instead, this returns a PL/SQL
+     * anonymous block that rewrites both columns within a single transaction; the {@code TILEPAGE -> TILESET} FK is
+     * declared {@code DEFERRABLE INITIALLY DEFERRED} so the constraint is verified once at commit, with both updates
+     * already in place.
      *
-     * <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.
+     * <p>The block uses Oracle's {@code SUBSTR}/{@code INSTR}/{@code ||} idiom rather than SQL-standard
+     * {@code SUBSTRING ... FROM POSITION(...)} which Oracle does not provide.
      */
     @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 TILESET_ID LIKE :%s || '#%%';
+                END;
+                """
+                .formatted(prefix, newLayerName, newLayerName, oldLayerName, prefix, newLayerName, oldLayerName);
     }
 
     @Override