mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

maximthomas
2 days ago 52adad385c178dc231693e90b559d677219251eb
refs
author maximthomas <maxim.thomas@gmail.com>
Tuesday, September 1, 2026 10:13 +0200
committer maximthomas <maxim.thomas@gmail.com>
Tuesday, September 1, 2026 10:13 +0200
commit52adad385c178dc231693e90b559d677219251eb
tree 71f57a7a60bc2ef4799fce8d06f15ad1cfa985de tree | zip | gz
parent 4486d18e9fde441a438fc9d345675aabb33dface view | diff
[#903] Grant the free replay to the prompt conflicts alone, and keep one window

The first replay was granted to every conflict, which handed it to the one
class that does not need it. A MySQL lock wait timeout is reported only once
innodb_lock_wait_timeout has elapsed - the engine has already bounded that
wait - so a free replay buys a second wait of the same length, 100 s at the
50 s default where master released the worker after 50 s. Worse, the window
was never consulted for it: the grant returned first, so the 10 s bound could
not fire against the single most expensive replay it was introduced to stop.

The grant now goes to Conflict.PROMPT only, whose preceding wait SQL Server,
Oracle and PostgreSQL all leave unbounded - no window survives it, and
measuring one against it is what left issue #903 with zero replays.
AFTER_LOCK_WAIT is measured against the window from the first attempt, which
is what the class was introduced to do.

With the grant narrowed, the 60 s widening of the prompt window was
compensating for a defect rather than for anything real, and it multiplied by
six how long a conflicted write pins a worker thread - a deadlock storm hits
every worker at once. Reverted to 10 s. That leaves the two window constants
bound to the same literal, a name apiece for one value, so they are collapsed
into RETRY_WINDOW_NANOS and windowOf() is deleted. The Conflict enum survives
and is no longer inert at the documented default: it decides the grant.

Also:

- conflictOf() walks the whole cause chain and keeps the most specific class
found, so a wrapper carrying a bare class 40 state no longer downgrades the
AFTER_LOCK_WAIT of the SQLException it wraps.
- isRetryableConflict() is deleted; write() has called replayable() since the
previous commit and nothing in src/main called the wrapper. Its doc moves
onto conflictOf().
- The replay warning names the conflict class and both bounds, in ms - whole
seconds read "0 s" for most of a burst - since the effective cap is usually
the window and a log naming only MAX_RETRIES said nothing about why an
operation gave up at attempt 2 of a promised 10.

The line that held the original bug - startedAt read once outside the retry
loop - had no test: replayable() is handed an elapsed time rather than
measuring one, so moving that read inside the loop kept every case green
while restoring an unbounded retry. testWriteDrivesTheRetryLoop drives write()
through a scripted clock over a new nanoTime() seam and pins both lines. The
step size is load-bearing in each direction: at 4 s the run must stop on the
third attempt, which the relocated read turns into ten, and at 12 s only the
grant can produce a second attempt, so removing it stops the run on the first.

Claude-Session: https://claude.ai/code/session_0123YLkjSmyp15GynenKE9vP
2 files modified
332 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java 169 ●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java 163 ●●●● diff | view | raw | blame | history