From 9a086de1184ea099a30ed2ec4a9bdfa374c4c24b Mon Sep 17 00:00:00 2001
From: Maxim Thomas <maxim.thomas@gmail.com>
Date: Thu, 10 Sep 2026 08:32:12 +0000
Subject: [PATCH] [#903] Grant the first replay of a conflict, and bound the rest by its class (#904)
---
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java | 556 +++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 497 insertions(+), 59 deletions(-)
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
index 2c339a9..0aff8f7 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
@@ -17,10 +17,12 @@
import org.forgerock.opendj.server.config.server.JDBCBackendCfg;
import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.backends.jdbc.JDBCStorage.Conflict;
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.StorageRuntimeException;
import org.opends.server.backends.pluggable.spi.TreeName;
import org.opends.server.backends.pluggable.spi.WriteOperation;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
import org.opends.server.types.DirectoryException;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
@@ -40,6 +42,7 @@
import java.sql.Statement;
import java.util.Properties;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Predicate;
@@ -60,16 +63,22 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.AFTER_LOCK_WAIT;
+import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.NONE;
+import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.PROMPT;
+import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.UNKNOWN_ENGINE;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
/**
- * Tests how a failure is classified - as a transaction conflict, or as a connection the database dropped - which
- * is what decides whether {@link JDBCStorage#write} replays the operation, and how long it waits before it does;
- * and what {@code write()} itself does with that verdict, replay and pool alike.
+ * Tests how a failure is classified - as a transaction conflict of one class or another, or as a connection the
+ * database dropped - which is what decides whether {@link JDBCStorage#write} replays the operation, whether its
+ * first replay is granted regardless of the clock, how long the replays may go on for and how long it waits
+ * before each of them; and what {@code write()} itself does with that verdict, replay and pool alike.
* <p>
* Runs without a database: the failures the drivers report are reproduced as synthetic
* {@link SQLException}s carrying the same vendor error number and SQLState, and the writes that carry them run
@@ -84,6 +93,8 @@
private static final String MYSQL = "com.mysql.cj.jdbc.ConnectionImpl";
private static final String ORACLE = "oracle.jdbc.driver.T4CConnection";
private static final String POSTGRES = "org.postgresql.jdbc.PgConnection";
+ /** A MySQL-wire-compatible driver, whose class name carries no engine this backend recognises. */
+ private static final String MARIADB = "org.mariadb.jdbc.Connection";
/** The tree every write of this test opens; the table name behind it is a hash of this name. */
private static final TreeName TREE = new TreeName("dc=example,dc=com", "id2entry");
@@ -121,6 +132,16 @@
{
}
+ /**
+ * A MySQL-wire-compatible driver none of the four engines is recognised in - MariaDB Connector/J, an Aurora-
+ * or Percona-branded one. It reports a lock wait timeout as 1205 under class 40 exactly as Connector/J does,
+ * and a backend created under {@code com.mysql.cj.jdbc} opens through it: every DDL of {@code openTree()} is
+ * guarded by a catalog read, so an existing backend issues none of it.
+ */
+ interface mariadbConnection extends Connection
+ {
+ }
+
/** A failure whose cause chain is a cycle, to check that walking it terminates. */
private static final class SelfCausedException extends RuntimeException
{
@@ -133,62 +154,192 @@
}
}
+ /**
+ * The failures the engines report, and the class each of them belongs to: {@link Conflict#NONE} for a failure no
+ * replay resolves, and otherwise how promptly the engine reporting it does so, which is all the class decides.
+ */
@DataProvider
public Object[][] failures()
{
return new Object[][] {
- // SQL Server picking a transaction as the deadlock victim: the failure this retry exists for
- { "mssql deadlock victim", sql(1205, "40001"), MSSQL, true },
+ // SQL Server picking a transaction as the deadlock victim: the failure this retry exists for. It is reported
+ // as promptly as the deadlock monitor runs, but only once the victim has waited out a lock wait of its own,
+ // which SQL Server leaves unbounded - the wait belongs to the attempt, not to the reporting
+ { "mssql deadlock victim", sql(1205, "40001"), MSSQL, PROMPT },
// a deployment may add xopenStates=true to its connection URL, which reports the same deadlock as 42000
- { "mssql deadlock victim, xopenStates", sql(1205, "42000"), MSSQL, true },
+ { "mssql deadlock victim, xopenStates", sql(1205, "42000"), MSSQL, PROMPT },
// the conflict of most other engines is carried by the SQLState, under a vendor number of their own
- { "postgres serialization failure", sql(0, "40001"), POSTGRES, true },
- { "postgres deadlock detected", sql(0, "40P01"), POSTGRES, true },
+ { "postgres serialization failure", sql(0, "40001"), POSTGRES, PROMPT },
+ { "postgres deadlock detected", sql(0, "40P01"), POSTGRES, PROMPT },
// Connector/J replaces the server side HY000 of both conditions with 40001, so neither needs a number here
- { "mysql deadlock", sql(1213, "40001"), MYSQL, true },
- // not a deadlock, but transient in the same way and equally resolved by a replay
- { "mysql lock wait timeout", sql(1205, "40001"), MYSQL, true },
+ { "mysql deadlock", sql(1213, "40001"), MYSQL, PROMPT },
+ // not a deadlock, but transient in the same way and equally resolved by a replay - and the one conflict of
+ // them all that an engine reports only after a lock wait timeout of its own, innodb_lock_wait_timeout
+ { "mysql lock wait timeout", sql(1205, "40001"), MYSQL, AFTER_LOCK_WAIT },
// the rollback a MySQL group replication conflict reports, error 3101, which the driver maps to 40000
- { "mysql group replication rollback", sql(3101, "40000"), MYSQL, true },
+ { "mysql group replication rollback", sql(3101, "40000"), MYSQL, PROMPT },
// Oracle maps ORA-00060 to SQLState 61000, so only its error number identifies the deadlock
- { "oracle deadlock detected", sql(60, "61000"), ORACLE, true },
+ { "oracle deadlock detected", sql(60, "61000"), ORACLE, PROMPT },
// the conflict reaches JDBCStorage.write() wrapped, so the whole cause chain has to be walked
- { "wrapped once", new StorageRuntimeException(sql(1205, "40001")), MSSQL, true },
+ { "wrapped once", new StorageRuntimeException(sql(1205, "40001")), MSSQL, PROMPT },
{ "wrapped twice",
new DirectoryException(OTHER, raw("unchecked"), new StorageRuntimeException(sql(1205, "40001"))), MSSQL,
- true },
+ PROMPT },
// the vendor numbers collide across engines, so they must not be matched driver-independently:
// ORA-01205 "not a data file" is fatal, and no replay resolves it
- { "oracle not a data file", sql(1205, "64000"), ORACLE, false },
+ { "oracle not a data file", sql(1205, "64000"), ORACLE, NONE },
// and a lock wait timeout is a MySQL number: 1205 means nothing of the kind to PostgreSQL
- { "postgres unrelated 1205", sql(1205, "22001"), POSTGRES, false },
+ { "postgres unrelated 1205", sql(1205, "22001"), POSTGRES, NONE },
// two class 40 states are rollbacks that a replay must not repeat: 40003 leaves the outcome of the
// transaction unknown, and 40002 is an integrity constraint violation that a replay would only hit again
- { "statement completion unknown", sql(0, "40003"), POSTGRES, false },
- { "transaction integrity constraint violation", sql(0, "40002"), POSTGRES, false },
- // ... but the state of a conflict is still matched whatever vendor number carries it
- { "class 40 is driver independent", sql(0, "40001"), null, true },
+ { "statement completion unknown", sql(0, "40003"), POSTGRES, NONE },
+ { "transaction integrity constraint violation", sql(0, "40002"), POSTGRES, NONE },
+ // ... but the state of a conflict is still matched whatever vendor number carries it, which is what makes
+ // an unrecognised engine replayable at all. Which class of conflict it is cannot be told, though: the
+ // grant rests on the engine not having bounded the wait already, and of this engine that is not known
+ { "class 40 is driver independent", sql(0, "40001"), null, UNKNOWN_ENGINE },
+ // the case that costs: a MySQL-wire-compatible driver reports innodb_lock_wait_timeout as 1205 under
+ // class 40 exactly as Connector/J does, and reading the number only under a name carrying "mysql" would
+ // hand it the grant - a second full 50 s wait, the one thing the window exists to refuse
+ { "mysql wire compatible lock wait timeout", sql(1205, "40001"), MARIADB, UNKNOWN_ENGINE },
+ { "class 40 with 1205, no driver", sql(1205, "40001"), null, UNKNOWN_ENGINE },
// nothing a replay can resolve
- { "primary key violation", sql(2627, "23000"), MSSQL, false },
- { "syntax error", sql(102, "S0001"), MSSQL, false },
- { "no SQLState", sql(0, null), MSSQL, false },
- { "not a SQLException", new IllegalStateException("connection closed"), MSSQL, false },
- { "wrapped, not a conflict", new StorageRuntimeException(sql(2627, "23000")), MSSQL, false },
- { "no failure at all", null, MSSQL, false },
+ { "primary key violation", sql(2627, "23000"), MSSQL, NONE },
+ { "syntax error", sql(102, "S0001"), MSSQL, NONE },
+ { "no SQLState", sql(0, null), MSSQL, NONE },
+ { "not a SQLException", new IllegalStateException("connection closed"), MSSQL, NONE },
+ { "wrapped, not a conflict", new StorageRuntimeException(sql(2627, "23000")), MSSQL, NONE },
+ { "no failure at all", null, MSSQL, NONE },
// a vendor number is never matched without a driver to key it off, since the engines collide on it
- { "unknown driver", sql(1205, "HY000"), null, false },
- { "cyclic cause chain", new SelfCausedException(), MSSQL, false },
+ { "unknown driver", sql(1205, "HY000"), null, NONE },
+ // the state the MySQL server itself gives a lock wait timeout, before Connector/J remaps it to class 40: the
+ // number read to date that conflict refines a match its state has already made, and never makes one of its own
+ { "mysql lock wait timeout, server state", sql(1205, "HY000"), MYSQL, NONE },
+ { "cyclic cause chain", new SelfCausedException(), MSSQL, NONE },
+
+ // the chain is walked to its end, not stopped at its first conflict: a hop carrying a bare class 40 state
+ // is a conflict by itself, and returning it would hand the lock wait timeout it wraps - a wait MySQL has
+ // already bounded - the replay that only the conflicts nothing bounds are granted
+ { "lock wait timeout under a bare class 40 wrapper", sql(0, "40001", sql(1205, "40001")), MYSQL,
+ AFTER_LOCK_WAIT },
+ { "bare class 40 wrapper over a deadlock", sql(0, "40001", sql(1213, "40001")), MYSQL, PROMPT },
};
}
+ /**
+ * Whether a failure is a conflict at all decides that it is replayed; which class of conflict it is decides
+ * only whether its first replay is granted regardless of the clock.
+ */
@Test(dataProvider = "failures")
- public void testIsRetryableConflict(String name, Throwable failure, String driver, boolean expected)
+ public void testConflictClass(String name, Throwable failure, String driver, Conflict expected)
{
- assertEquals(JDBCStorage.isRetryableConflict(failure, driver), expected, name);
+ assertEquals(conflictOf(failure, driver), expected, name);
+ }
+
+ /**
+ * Which replays happen. The window bounds them from the first attempt, with one grant: a conflict its engine
+ * reports promptly is given its first replay whatever the clock says, since the wait charged to the attempt
+ * that hit it is unbounded and no window survives it. A conflict the engine reported only after a lock wait
+ * timeout of its own gets no such grant - that wait is bounded already, and repeating it is what the window
+ * refuses.
+ */
+ @DataProvider
+ public Object[][] replays()
+ {
+ return new Object[][] {
+ // the engine asked for the transaction to be rerun after a wait nothing here bounds, and no clock denies
+ // that first rerun. The failure of run 33010633197 is the case: SQL Server leaves the lock wait unbounded,
+ // so its deadlock monitor picked a victim ~12 s into the first attempt, and master replayed it zero times
+ { "deadlock reported after a long lock wait", 1, seconds(12), sql(1205, "40001"), MSSQL, true },
+ { "deadlock reported later than any window", 1, seconds(600), sql(1205, "40001"), MSSQL, true },
+ // and exactly at the window, which is the boundary the grant is decided on: elapsed >= window, not > it.
+ // The rows around this one bracket that point without standing on it, and a > there would refuse the first
+ // replay of #903 to every conflict reported at the window to the nanosecond
+ { "deadlock at the window, first attempt", 1, seconds(10), sql(1205, "40001"), MSSQL, true },
+
+ // the grant is one replay, not an exemption: from the second attempt on the window governs, so that a
+ // conflict which never clears is failed rather than never returned
+ { "deadlock within the window", 2, seconds(9), sql(1205, "40001"), MSSQL, true },
+ { "deadlock at the window", 2, seconds(10), sql(1205, "40001"), MSSQL, false },
+ // the same elapsed time that was granted on attempt 1 is refused on attempt 2: one grant, and only one
+ { "deadlock past the window", 2, seconds(12), sql(1205, "40001"), MSSQL, false },
+ // a MySQL deadlock is reported as promptly as any other engine reports one, so it is granted the same
+ { "mysql deadlock, first attempt", 1, seconds(12), sql(1213, "40001"), MYSQL, true },
+ { "mysql deadlock, past the window", 2, seconds(12), sql(1213, "40001"), MYSQL, false },
+
+ // MySQL reports a lock wait timeout only after innodb_lock_wait_timeout, 50 s by default: that wait is
+ // bounded by the engine, so the window is measured against it from the first attempt and a second 50 s wait
+ // is refused - which is the whole reason the window was introduced
+ { "mysql lock wait timeout at the default 50 s", 1, seconds(50), sql(1205, "40001"), MYSQL, false },
+ { "mysql lock wait timeout past the window", 1, seconds(12), sql(1205, "40001"), MYSQL, false },
+ // ... and a deployment that tuned innodb_lock_wait_timeout below the window still gets its replays
+ { "mysql lock wait timeout tuned under the window", 1, seconds(3), sql(1205, "40001"), MYSQL, true },
+ { "mysql lock wait timeout, second attempt within", 2, seconds(6), sql(1205, "40001"), MYSQL, true },
+ { "mysql lock wait timeout, second attempt at the window", 2, seconds(10), sql(1205, "40001"), MYSQL, false },
+ // the same 1205 under a MySQL-wire-compatible driver, which is what a backend created under Connector/J
+ // and opened through MariaDB Connector/J reports: the window governs it from the first attempt too, since
+ // a grant here would buy the second innodb_lock_wait_timeout the rows above refuse
+ { "mysql wire compatible lock wait timeout", 1, seconds(12), sql(1205, "40001"), MARIADB, false },
+ { "mysql wire compatible conflict within the window", 1, seconds(3), sql(1205, "40001"), MARIADB, true },
+
+ // the attempt count bounds every class, whatever the window has left. It is the bound that rarely fires:
+ // reaching it takes ten attempts inside a 10 s window, which only a conflict reported in milliseconds
+ // leaves room for - a conflict preceded by a wait longer than the window stops at two attempts, the
+ // granted one included, and the line reporting the replay names the window for that reason
+ { "last attempt left", 9, 0L, sql(1205, "40001"), MSSQL, true },
+ { "attempts exhausted", 10, 0L, sql(1205, "40001"), MSSQL, false },
+ // a failure carrying no conflict class at all still passes these bounds: what makes it replayable is
+ // replayReason(), which write() asks first, and a dropped connection carries no class 40 state
+ { "a drop, which no class describes", 1, 0L, sql(2627, "23000"), MSSQL, true },
+ };
+ }
+
+ /**
+ * Composed the way {@code write()} composes it: the class is read off the failure once, and the bounds are
+ * asked of the class. Whether the failure is worth replaying at all is {@code replayReason()}, tested apart.
+ */
+ @Test(dataProvider = "replays")
+ public void testReplayable(String name, int attempt, long elapsedNanos, Throwable failure, String driver,
+ boolean expected)
+ {
+ assertEquals(JDBCStorage.replayableWithin(attempt, elapsedNanos, conflictOf(failure, driver)),
+ expected, name);
+ }
+
+ /**
+ * The grant is reported rather than inferred: the line reporting a replay says "granted past it" only where
+ * the loop really took that branch. Pinned apart from {@link #testReplayable} because the two agree today by
+ * construction - a replay past the window can only be the grant - and it is that coincidence, not the claim,
+ * that a later change to the bounds would take away.
+ */
+ @Test
+ public void testTheGrantIsTheOnlyReplayPastTheWindow()
+ {
+ // the grant, and the only shape of it: the first replay of a conflict reported past the window
+ assertTrue(JDBCStorage.grantedPastTheWindow(1, seconds(12), PROMPT), "the conflict of #903 was not granted");
+ // inside the window nothing is granted - the window itself allows the replay, and the line says nothing
+ assertFalse(JDBCStorage.grantedPastTheWindow(1, seconds(9), PROMPT), "a replay inside the window was granted");
+ // and past the first attempt, or for a wait the engine already bounded, there is no grant at all
+ assertFalse(JDBCStorage.grantedPastTheWindow(2, seconds(12), PROMPT), "a second replay was granted");
+ assertFalse(JDBCStorage.grantedPastTheWindow(1, seconds(12), AFTER_LOCK_WAIT), "a bounded wait was granted");
+ assertFalse(JDBCStorage.grantedPastTheWindow(1, seconds(12), UNKNOWN_ENGINE),
+ "an engine whose wait cannot be vouched for was granted");
+ assertFalse(JDBCStorage.grantedPastTheWindow(1, seconds(12), NONE), "a failure carrying no conflict");
+
+ // every replay the bounds allow past the window is that grant, which is what lets the line name it
+ for (int attempt = 1; attempt < 12; attempt++)
+ {
+ for (Conflict conflict : Conflict.values())
+ {
+ final boolean pastTheWindow = JDBCStorage.replayableWithin(attempt, seconds(11), conflict);
+ assertEquals(pastTheWindow, JDBCStorage.grantedPastTheWindow(attempt, seconds(11), conflict),
+ "attempt " + attempt + " of a " + conflict + " conflict past the window");
+ }
+ }
}
@DataProvider
@@ -251,9 +402,9 @@
public void testADroppedConnectionIsReplayedOnlyBeforeTheCommit()
{
final SQLException dropped = sql(0, "08006");
- assertEquals(JDBCStorage.replayReason(dropped, POSTGRES, false, false, false),
+ assertEquals(replayReason(dropped, POSTGRES, false, false, false),
"a connection the database dropped");
- assertNull(JDBCStorage.replayReason(dropped, POSTGRES, true, false, false),
+ assertNull(replayReason(dropped, POSTGRES, true, false, false),
"an in doubt transaction was replayed");
}
@@ -265,10 +416,10 @@
public void testAConnectionTheDriverClosedIsADroppedOne()
{
final SQLException killed = sql(596, "S0001");
- assertNull(JDBCStorage.replayReason(killed, MSSQL, false, false, false), "S0001 was replayed on its own");
- assertEquals(JDBCStorage.replayReason(killed, MSSQL, false, false, true),
+ assertNull(replayReason(killed, MSSQL, false, false, false), "S0001 was replayed on its own");
+ assertEquals(replayReason(killed, MSSQL, false, false, true),
"a connection the database dropped");
- assertNull(JDBCStorage.replayReason(killed, MSSQL, true, false, true),
+ assertNull(replayReason(killed, MSSQL, true, false, true),
"an in doubt transaction was replayed");
}
@@ -281,9 +432,9 @@
@Test
public void testAnAttemptThatCommittedPartOfItsWorkIsNotReplayed()
{
- assertNull(JDBCStorage.replayReason(sql(0, "40001"), POSTGRES, false, true, false), "a conflict was replayed");
- assertNull(JDBCStorage.replayReason(sql(0, "08006"), POSTGRES, false, true, false), "a drop was replayed");
- assertNull(JDBCStorage.replayReason(sql(596, "S0001"), MSSQL, false, true, true), "a drop was replayed");
+ assertNull(replayReason(sql(0, "40001"), POSTGRES, false, true, false), "a conflict was replayed");
+ assertNull(replayReason(sql(0, "08006"), POSTGRES, false, true, false), "a drop was replayed");
+ assertNull(replayReason(sql(596, "S0001"), MSSQL, false, true, true), "a drop was replayed");
}
/**
@@ -297,8 +448,8 @@
public void testAConflictIsNotReadFromTheReleaseOfTheConnection()
{
final SQLException onRelease = suppressing(sql(2627, "23000"), sql(0, "40000"));
- assertFalse(JDBCStorage.isRetryableConflict(onRelease, POSTGRES), "a conflict was read from the release");
- assertNull(JDBCStorage.replayReason(onRelease, POSTGRES, true, false, false),
+ assertEquals(conflictOf(onRelease, POSTGRES), NONE, "a conflict was read from the release");
+ assertNull(replayReason(onRelease, POSTGRES, true, false, false),
"a transaction the commit left in doubt was replayed");
// the same shape carrying a drop instead: read, since the release is where a drop is stated at all
@@ -328,16 +479,16 @@
public void testAConflictIsReplayedFromEitherPhase()
{
final SQLException conflict = sql(0, "40001");
- assertEquals(JDBCStorage.replayReason(conflict, POSTGRES, false, false, false), "a conflict");
- assertEquals(JDBCStorage.replayReason(conflict, POSTGRES, true, false, false), "a conflict");
+ assertEquals(replayReason(conflict, POSTGRES, false, false, false), "a conflict");
+ assertEquals(replayReason(conflict, POSTGRES, true, false, false), "a conflict");
}
/** Everything else fails the operation, as it did before either replay existed. */
@Test
public void testAFailureOfTheStatementIsNotReplayed()
{
- assertNull(JDBCStorage.replayReason(sql(2627, "23000"), MSSQL, false, false, false));
- assertNull(JDBCStorage.replayReason(sql(2627, "23000"), MSSQL, true, false, false));
+ assertNull(replayReason(sql(2627, "23000"), MSSQL, false, false, false));
+ assertNull(replayReason(sql(2627, "23000"), MSSQL, true, false, false));
}
/** The delay grows with the attempt, so that the replays outlast a contention lasting more than a few ms. */
@@ -367,7 +518,7 @@
@Test
public void testConflictSummaryNamesTheStateAndTheNumber()
{
- final String summary = JDBCStorage.conflictSummary(
+ final String summary = conflictSummary(
new DirectoryException(OTHER, raw("unchecked"), new StorageRuntimeException(sql(1205, "40001"))), POSTGRES);
assertTrue(summary.contains("40001"), summary);
assertTrue(summary.contains("1205"), summary);
@@ -382,26 +533,51 @@
@Test
public void testConflictSummaryNamesTheFailureTheReplayWasDecidedOn()
{
- final String summary = JDBCStorage.conflictSummary(
+ final String summary = conflictSummary(
new StorageRuntimeException(suppressing(sql(2627, "23000"), sql(0, "08006"))), POSTGRES);
assertTrue(summary.contains("08006"), summary);
assertFalse(summary.contains("23000"), summary);
}
+ /**
+ * The line names the link the class was decided on, which is not always the first conflict of the chain:
+ * {@code conflictVerdict()} keeps the most specific class it finds, so a wrapper carrying a bare class 40 state
+ * is walked past to the lock wait timeout underneath it. Naming the wrapper would print "error 0" for a replay
+ * whose whole bound was chosen by the 1205 it never shows. Asked of one verdict per chain, the way
+ * {@code write()} asks it: the class and the link are one answer, and two walks could disagree about them.
+ */
+ @Test
+ public void testConflictSummaryNamesTheLinkTheClassWasDecidedOn()
+ {
+ final SQLException lateUnderAWrapper = sql(0, "40001", sql(1205, "40001"));
+ final JDBCStorage.ConflictVerdict late = JDBCStorage.conflictVerdict(lateUnderAWrapper, MYSQL);
+ assertEquals(late.conflict, AFTER_LOCK_WAIT);
+ final String summary = JDBCStorage.conflictSummary(late, lateUnderAWrapper);
+ assertTrue(summary.contains("1205"), summary);
+
+ // the same chain under a driver that gives 1205 no such meaning is a prompt conflict, and the first link
+ // of it is the one the decision was taken on
+ final SQLException sameChain = sql(0, "40001", sql(1205, "40001"));
+ final JDBCStorage.ConflictVerdict prompt = JDBCStorage.conflictVerdict(sameChain, POSTGRES);
+ assertEquals(prompt.conflict, PROMPT);
+ final String firstLink = JDBCStorage.conflictSummary(prompt, sameChain);
+ assertTrue(firstLink.contains("error 0"), firstLink);
+ }
+
/** A failure carrying no SQLException at all, and a cyclic cause chain, still have to yield something loggable. */
@Test
public void testConflictSummaryTerminatesWithoutASQLException()
{
- assertTrue(JDBCStorage.conflictSummary(new IllegalStateException("connection closed"), POSTGRES).contains("closed"));
- assertTrue(JDBCStorage.conflictSummary(new SelfCausedException(), POSTGRES).contains("SelfCausedException"));
- assertEquals(JDBCStorage.conflictSummary(null, POSTGRES), "null");
+ assertTrue(conflictSummary(new IllegalStateException("connection closed"), POSTGRES).contains("closed"));
+ assertTrue(conflictSummary(new SelfCausedException(), POSTGRES).contains("SelfCausedException"));
+ assertEquals(conflictSummary(null, POSTGRES), "null");
}
/** A statement that carries neither a conflict nor a drop is still the one the summary names. */
@Test
public void testConflictSummaryFallsBackToTheFirstFailureOfTheChain()
{
- final String summary = JDBCStorage.conflictSummary(new StorageRuntimeException(sql(2627, "23000")), POSTGRES);
+ final String summary = conflictSummary(new StorageRuntimeException(sql(2627, "23000")), POSTGRES);
assertTrue(summary.contains("23000"), summary);
// the fallback names the statement, not the rollback of the release behind it: this is where a replay
@@ -409,7 +585,7 @@
// and the walk reaches the suppressed exceptions of a failure before its cause
final StorageRuntimeException killedSession = new StorageRuntimeException(sql(596, "S0001"));
killedSession.addSuppressed(sql(0, "25P02"));
- final String decidedOnTheConnection = JDBCStorage.conflictSummary(killedSession, MSSQL);
+ final String decidedOnTheConnection = conflictSummary(killedSession, MSSQL);
assertTrue(decidedOnTheConnection.contains("S0001"), decidedOnTheConnection);
assertFalse(decidedOnTheConnection.contains("25P02"), decidedOnTheConnection);
}
@@ -428,6 +604,112 @@
}
/**
+ * The class of a conflict is the one walk that budget must not bound, and it is the reason
+ * {@code failureScope()} does not bound its own either: truncation does not leave this verdict unanswered, it
+ * weakens it. A lock wait timeout past the budget, with a bare class 40 link inside it, would come back
+ * {@link Conflict#PROMPT} and be handed the one replay the class exists to refuse - a second full
+ * {@code innodb_lock_wait_timeout}. Truncation here grants a replay rather than losing one.
+ */
+ @Test
+ public void testTheConflictClassIsReadFromEveryLinkOfTheChain()
+ {
+ // a bare class 40 wrapper, then 64 links of a rejected statement, then the timeout that decided the class
+ final SQLException bareClass40 = sql(0, "40001");
+ SQLException tail = bareClass40;
+ for (int link = 0; link < 64; link++)
+ {
+ tail = chained(tail, sql(2627, "23000")).getNextException();
+ }
+ chained(tail, sql(1205, "40001"));
+
+ final JDBCStorage.ConflictVerdict verdict = JDBCStorage.conflictVerdict(bareClass40, MYSQL);
+ assertEquals(verdict.conflict, AFTER_LOCK_WAIT,
+ "a lock wait timeout past MAX_CHAIN_LINKS came back as a conflict the window does not bound");
+ assertFalse(JDBCStorage.replayableWithin(1, seconds(12), verdict.conflict),
+ "and was granted the replay past the window");
+ // the line reporting a replay names that same link, since one walk produced both
+ assertTrue(JDBCStorage.conflictSummary(verdict, bareClass40).contains("1205"));
+ }
+
+ /**
+ * The widening that reading every link brings, which is the one behaviour change of #903 outside the grant: a
+ * chain whose only conflict-bearing link sits past the budget was not a conflict at all on master - the
+ * truncating walk never reached it - so {@code replayReason()} answered null and the operation was failed.
+ * The case above pins the *class* of such a chain, since it puts a bare class 40 state at the head that the
+ * truncating walk already matched; this one pins that the conflict is found at all.
+ */
+ @Test
+ public void testAConflictOnlyPastTheBudgetIsFoundAtAll()
+ {
+ // 64 links of a rejected statement - the whole budget - and the conflict on the 65th
+ final SQLException head = sql(2627, "23000");
+ SQLException tail = head;
+ for (int link = 2; link <= 64; link++)
+ {
+ tail = chained(tail, sql(2627, "23000")).getNextException();
+ }
+ chained(tail, sql(0, "40001"));
+
+ assertEquals(conflictOf(head, POSTGRES), PROMPT,
+ "a conflict whose only link sits past MAX_CHAIN_LINKS was not found at all");
+ assertEquals(replayReason(head, POSTGRES, false, false, false), "a conflict",
+ "and the operation carrying it was not replayed");
+ }
+
+ /**
+ * What that walk stops at is the strongest class the engine of its driver can report, not the last constant of
+ * {@link Conflict}: only MySQL reports a lock wait timeout of its own, so a walk stopping at
+ * {@link Conflict#AFTER_LOCK_WAIT} never stops early on the other three engines and reads every link of every
+ * failed write on the driver whose chains are longest. Pinned from both sides - no failure of an engine is
+ * classed above its own ceiling, and every ceiling is reached by some failure - since a ceiling set too low
+ * would end the walk on a class weaker than the chain carries, and one set too high never ends it early.
+ */
+ @Test
+ public void testTheWalkStopsAtTheStrongestClassItsEngineCanReport()
+ {
+ // one shape per branch of the classification: a bare class 40, the two numbers the engines collide on, a
+ // deadlock of oracle, the state MySQL reports before its driver remaps it, and a failure that is no conflict
+ final SQLException[] shapes = {
+ sql(0, "40001"), sql(1205, "40001"), sql(1213, "40001"), sql(60, "61000"), sql(1205, "HY000"),
+ sql(2627, "23000") };
+
+ for (String driver : new String[] { POSTGRES, MYSQL, ORACLE, MSSQL, MARIADB, null })
+ {
+ final Conflict ceiling = JDBCStorage.ceilingOf(JDBCStorage.dialectOf(driver));
+ Conflict strongest = NONE;
+ for (SQLException shape : shapes)
+ {
+ final Conflict conflict = conflictOf(shape, driver);
+ assertTrue(conflict.compareTo(ceiling) <= 0,
+ driver + " classed " + shape.getSQLState() + "/" + shape.getErrorCode() + " as " + conflict
+ + ", above the ceiling its walk stops at");
+ strongest = conflict.compareTo(strongest) > 0 ? conflict : strongest;
+ }
+ assertEquals(strongest, ceiling, "the walk of " + driver + " stops at a class it can never reach");
+ }
+
+ // and the walk really stops there: a link behind a conflict already at its engine's ceiling is not looked
+ // at. This is what makes reading every link affordable - write() classifies before it knows the failure is
+ // replayable at all, so a plain 23000 from adding an entry that is already there reaches this walk too
+ final AtomicBoolean walkedPast = new AtomicBoolean();
+ final SQLException behindTheCeiling = new SQLException("synthetic failure", "23000", 2627)
+ {
+ @Override
+ public String getSQLState()
+ {
+ walkedPast.set(true);
+ return super.getSQLState();
+ }
+ };
+ assertEquals(conflictOf(sql(0, "40P01", behindTheCeiling), POSTGRES), PROMPT);
+ assertFalse(walkedPast.get(), "an engine reporting no lock wait timeout of its own walked past its ceiling");
+
+ // under mysql the same head is not the ceiling - a lock wait timeout could still be behind it - so it is
+ assertEquals(conflictOf(sql(0, "40001", behindTheCeiling), MYSQL), PROMPT);
+ assertTrue(walkedPast.get(), "mysql stopped before the link a lock wait timeout could have been on");
+ }
+
+ /**
* Opening a tree that is already there commits nothing, so the attempt stays replayable. The create table is
* guarded by a catalog read, and so is the create index on every engine but postgresql, so on an existing
* backend {@code openTree(name, true)} issues no statement at all - while
@@ -515,7 +797,7 @@
}
catch (StorageRuntimeException expected)
{
- assertTrue(JDBCStorage.isRetryableConflict(expected, POSTGRES), "the conflict was not the failure raised");
+ assertEquals(conflictOf(expected, POSTGRES), PROMPT, "the conflict was not the failure raised");
}
assertEquals(attempts.get(), 1, "a transaction that committed part of its work was replayed");
verify(statements).executeUpdate();
@@ -746,7 +1028,7 @@
}
catch (StorageRuntimeException expected)
{
- assertTrue(JDBCStorage.isRetryableConflict(expected, MYSQL), "the conflict was not the failure raised");
+ assertEquals(conflictOf(expected, MYSQL), PROMPT, "the conflict was not the failure raised");
}
assertEquals(attempts.get(), 1, "an attempt that committed part of its work was replayed");
verify(engineConnection).prepareStatement(startsWith("create index k_"));
@@ -844,6 +1126,22 @@
}
/**
+ * A mock of the given connection type, with the name every engine branch of {@code JDBCStorage} is keyed on
+ * asserted rather than assumed. The name of a mock is derived from the type it mocks, so a renamed fixture -
+ * or a Mockito that names its mocks differently - would move a case into another engine's branch with no test
+ * saying so, and there are cases no count of attempts would catch that in.
+ */
+ private static Connection mockOfEngine(Class<? extends Connection> engine)
+ {
+ final Connection con = mock(engine);
+ final String engineName = engine.getSimpleName().replace("Connection", "");
+ assertTrue(JDBCStorage.driverNameOf(con).contains(engineName),
+ "a mock of " + engine.getSimpleName() + " reaches no " + engineName + " branch: "
+ + JDBCStorage.driverNameOf(con));
+ return con;
+ }
+
+ /**
* The connection the tree catalog of a storage of this test is written on: it opens one straight through the
* driver, for the reason a stamp opens one of its own - the caller of openTree() is holding a pooled
* connection already.
@@ -872,11 +1170,7 @@
private JDBCStorage storageOverAnEngine(Class<? extends Connection> engine, boolean theIndex,
Connection... behind) throws Exception
{
- final Connection con = mock(engine);
- final String engineName = engine.getSimpleName().replace("Connection", "");
- assertTrue(JDBCStorage.driverNameOf(con).contains(engineName),
- "a mock of " + engine.getSimpleName() + " reaches no " + engineName + " branch: "
- + JDBCStorage.driverNameOf(con));
+ final Connection con = mockOfEngine(engine);
engineConnection = con;
// the connections behind it answer the connects the pool does not make: the tree catalog is read and
// written on one of its own, straight through the driver, since the caller of openTree() is holding a
@@ -999,11 +1293,155 @@
}
}
+ /**
+ * The two questions {@code write()} asks after a failed attempt, composed here the way it composes them: the
+ * conflict class is read off the failure once and handed to the reason, rather than being asked for again.
+ */
+ private static String replayReason(Throwable failure, String driver, boolean committing, boolean partlyCommitted,
+ boolean connectionClosed)
+ {
+ return JDBCStorage.replayReason(conflictOf(failure, driver), failure, committing, partlyCommitted,
+ connectionClosed);
+ }
+
+ /**
+ * The class of a failure, composed of the pieces {@code write()} composes it of. Forms of this and of
+ * {@link #conflictSummary} taking a failure and a driver used to live in {@code JDBCStorage} with no caller of
+ * their own in {@code src/main}, which left the classification javadoc hanging off methods production never
+ * called and made every test asking both questions walk the chains twice. The convenience is a test's, so it
+ * is written here.
+ */
+ private static Conflict conflictOf(Throwable failure, String driver)
+ {
+ return JDBCStorage.conflictVerdict(failure, driver).conflict;
+ }
+
+ /** The line reporting a replay, composed the way {@code write()} composes it: one walk, then the summary of it. */
+ private static String conflictSummary(Throwable failure, String driver)
+ {
+ return JDBCStorage.conflictSummary(JDBCStorage.conflictVerdict(failure, driver), failure);
+ }
+
private static SQLException sql(int errorCode, String sqlState)
{
return new SQLException("synthetic failure", sqlState, errorCode);
}
+ private static SQLException sql(int errorCode, String sqlState, Throwable cause)
+ {
+ return new SQLException("synthetic failure", sqlState, errorCode, cause);
+ }
+
+ private static long seconds(long seconds)
+ {
+ return TimeUnit.SECONDS.toNanos(seconds);
+ }
+
+ /**
+ * How {@link JDBCStorage#write} drives the two decisions above, which the cases before this one cannot see:
+ * they are handed an elapsed time and an attempt number rather than producing them. The clock is scripted and
+ * advances a fixed step per attempt - not per read of it - so that the timeline the loop sees depends on what
+ * it does rather than on how often it asks the time: a read added anywhere in {@code write()} leaves every row
+ * of this provider answering exactly as it does now.
+ * <p>
+ * Between them the rows pin the three lines the rest of the file would let a refactor take away. A single
+ * {@code startedAt} outside the retry loop is what makes the window bound the whole run rather than each
+ * attempt: moved inside, every attempt is measured against its own start, sees the step and nothing more, and
+ * replays to MAX_RETRIES. The grant of the first replay is what issue #903 is about: without it an attempt
+ * that alone outlasts the window leaves the loop with no replay at all. And the class the grant is asked of is
+ * read off the failure of this very run, rather than off a driver the loop does not carry: the last two rows
+ * fail as plainly as the first two and are replayed no times at all.
+ */
+ @DataProvider
+ public Object[][] writeRuns()
+ {
+ return new Object[][] {
+ // a step under the window, so the window is what ends the run: attempt 1 is granted its replay at 4 s,
+ // attempt 2 is inside the window at 8 s, attempt 3 is past it at 12 s. With startedAt inside the loop every
+ // attempt measures 4 s, never reaches the window, and the run goes to MAX_RETRIES instead
+ { "the window bounds the run, not the attempt", postgresConnection.class, sql(0, "40P01"), 4L, PROMPT, 3 },
+ // a step past the window, so only the grant can produce a second attempt: remove it and the run ends on
+ // the first. This is the row that pins the grant end to end, and the row above is the one that pins
+ // startedAt - at 12 s a per-attempt startedAt also stops at two attempts, and at 4 s the window alone
+ // already allows the replay of attempt 1. Neither row is redundant
+ { "the first replay is granted past the window", postgresConnection.class, sql(0, "40P01"), 12L, PROMPT, 2 },
+ // the same step, and the same class 40 state, for the one conflict the engine had already bounded: a
+ // single attempt. The predicate rows pin that decision, but only these rows pin that write() hands the
+ // predicate the class of its own failure - dropped on the way to replayableWithin() and the run above
+ // stays green while this one buys a second innodb_lock_wait_timeout
+ { "a lock wait timeout is granted no replay", mysqlConnection.class, sql(1205, "40001"), 12L,
+ AFTER_LOCK_WAIT, 1 },
+ // and the driver that reports that same timeout under a name this backend does not recognise: no grant
+ // there either, since what the grant rests on - the engine having bounded nothing - is unknown of it.
+ // No number of attempts separates this row from the one above: AFTER_LOCK_WAIT and UNKNOWN_ENGINE are
+ // refused the grant and bounded by the window identically, so the mysql row misread as unrecognised
+ // produces exactly the count expected of it. That is why each row names the class its engine gives its
+ // failure and the case asserts it of the mock it actually built
+ { "an unrecognised engine is granted no replay", mariadbConnection.class, sql(1205, "40001"), 12L,
+ UNKNOWN_ENGINE, 1 },
+ };
+ }
+
+ @Test(dataProvider = "writeRuns")
+ public void testWriteDrivesTheRetryLoop(String name, Class<? extends Connection> engine,
+ final SQLException conflict, final long stepSeconds, Conflict expectedClass, int expectedAttempts)
+ throws Exception
+ {
+ final AtomicInteger attempts = new AtomicInteger();
+ // the class name of the mock is what write() reads the engine off, asserted here the way
+ // storageOverAnEngine() asserts it
+ final Connection connection = mockOfEngine(engine);
+ // which branch of the classification the row reaches, asserted rather than inferred from the count: the
+ // count cannot tell AFTER_LOCK_WAIT from UNKNOWN_ENGINE, since both are refused the grant and bounded by
+ // the window alike, so a fixture renamed out of the mysql branch would leave that row green
+ assertEquals(conflictOf(conflict, JDBCStorage.driverNameOf(connection)), expectedClass,
+ name + ": the mock does not reach the branch the row names");
+
+ // a url of its own, as storageOver() gives every fixture of this file: getConnection() is overridden below,
+ // but distrustPool() is not, and a null one would reach ConcurrentHashMap.merge(null, ...) rather than the
+ // assertion under test the moment a row of this provider scripts a connection failure
+ final JDBCBackendCfg cfg = mock(JDBCBackendCfg.class);
+ when(cfg.getDBDirectory()).thenReturn(StubDriver.PREFIX + pools.incrementAndGet());
+
+ final JDBCStorage storage = new JDBCStorage(cfg, null)
+ {
+ @Override
+ Connection getConnection()
+ {
+ return connection;
+ }
+
+ @Override
+ long nanoTime()
+ {
+ // the attempts made are what moves this clock, so the run is the same however often write() reads it
+ return seconds(stepSeconds * attempts.get());
+ }
+ };
+ storage.accessMode = AccessMode.READ_WRITE;
+
+ StorageRuntimeException thrown = null;
+ try
+ {
+ storage.write(new WriteOperation()
+ {
+ @Override
+ public void run(WriteableTransaction txn)
+ {
+ attempts.incrementAndGet();
+ throw new StorageRuntimeException(conflict);
+ }
+ });
+ }
+ catch (StorageRuntimeException e)
+ {
+ thrown = e;
+ }
+
+ assertSame(thrown != null ? thrown.getCause() : null, conflict, name + ": the conflict reaches the caller");
+ assertEquals(attempts.get(), expectedAttempts, name + ": attempts made");
+ }
+
/** The second failure as the next exception of the first, the way a driver chains the errors of one message. */
private static SQLException chained(SQLException first, SQLException next)
{
--
Gitblit v1.10.0