Skip to content

[#1064] Replay a JE transaction ended by a lock conflict instead of giving it up at once - #1065

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1064-je-write-replay
Open

vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1064-je-write-replay

Conversation

@vharseko

Copy link
Copy Markdown
Member

Fixes #1064.

What JE does, and what JEStorage.write did with it

With the shipped configuration je.lock.timeout is 0 (ConfigurableEnvironment.defaultConfig), so a writer waiting for a record lock waits rather than times out, and the one conflict JE raises is the deadlock: it detects the cycle as soon as a wait would close it - LockManager.waitForLock runs checkAndHandleDeadlock before every wait, the detection delay being 0 - and ends one transaction of the cycle, chosen at random, with a DeadlockException; the other is granted its lock once the victim has aborted. JEStorage.write made one attempt: the transaction wraps the conflict in a StorageRuntimeException, write() unwrapped it and threw the bare DeadlockException, and BackendImpl.createDirectoryException answered the LDAP client with the server error result code and JE's account of its lock table as the diagnostic message. Reproduced at the storage level before the change: two writers locking two records in opposite order, the victim's write() throwing after one attempt within ~8 ms while the other commits.

PDBStorage.write replays a rolled back transaction under an attempt cap and a retry window (#937); JDBCStorage.write under MAX_RETRIES and MAX_RETRY_WINDOW_NANOS. Now JEStorage.write does the same for a LockConflictException - the base class JE documents as "abort and retry" - under the same two bounds, and gives it up the same way: a StorageRuntimeException naming the backend, the attempts, the time and which bound was spent, the conflict suppressed rather than made the cause, since every caller strips a cause off (write() itself and EntryContainer.throwAllowedExceptionTypes).

Where the JE loop differs from the PDB one

  • The conflict is raised inside the operation, and again by commit(). JE raises it from a record read or write, never from the commit, which takes no lock; the operation may catch it there - DN2URI.targetEntryReferrals swallows a StorageRuntimeException, and VLVIndex.applyConfigurationChange did until [#991] Decide and report an index configuration change outside the write which is replayed #997 - and the transaction is then abort-only, so commit() raises the conflict again, bare. The loop matches both forms, and an attempt which swallowed its conflict commits nothing and is replayed, where on PersistIt it "committed an attempt which had done nothing" ([#991] Decide and report an index configuration change outside the write which is replayed #997).
  • The backoff is needed, for a reason of JE's own. JE locks a record by the LSN of its current version. The victim's abort hands the waiting survivor the lock it asked for, but undoes the version that lock belongs to, so the survivor has to lock the version the undo put back - and a replay which comes back at once wins that race, holds the record when the survivor asks again, and the same deadlock forms with the roles drawn afresh. Without the sleep the deadlock of two writers was seen to form three times in a row (attempts 2 and 4, then 2 and 3); JE's own retry example sleeps before retrying for this reason. The ladder is PDB's, 50 ms doubling to 1 s.
  • The interrupt flag is not restored. The sleep is the one place the loop can be interrupted, and JE invalidates the whole environment when a thread carrying the interrupt flag makes any call - TxnManager.registerTxn/unRegisterTxn acquire their latch interruptibly, and EntryContainer.writeTrustState, on the caller's own failure road, is such a call. The interrupt is reported instead, next to the conflict, as the suppressed exceptions of the StorageRuntimeException thrown; the flag stays as the sleep cleared it.

ConfigurableEnvironment's comment above setLockTimeout(0) said a deadlocked operation blocks indefinitely; it does not, and the comment now says what JE does with it.

Reach

The ordinary LDAP write path does not reach the deadlock: IndexBuffer flushes keys in TreeMap order, EntryContainer keeps one tree order across add, delete, modify and modDN ("Ensure same access ordering as deleteEntry", "Ensure that all index updates are done in the correct order to avoid deadlocks" - the ordering the 2.6 JE backend already had, OPENDJ-1375), the counters are sharded by thread, and the core holds DN locks. 3992 concurrent adds, modifies, deletes and renames over 8 threads on shared index keys produced none. So this is parity with the other two engines and a guard for an ordering nothing checks: a new write path breaks it silently, and an operator who sets je.lock.timeout through ds-cfg-je-property turns plain contention into a LockTimeoutException, which the loop replays as well.

Tests

JEStorageTest, new, after PDBStorageTest. The conflicts are JE's own - a DeadlockException cannot be built by a test, its constructor needs the internal locker it invalidates - a deadlock made by two writers locking two records in opposite order, and a conflict on every attempt made by a transaction which keeps a record locked while the storage runs with a je.lock.timeout, set the way an operator sets it:

  • testDeadlockVictimIsReplayed and testConflictSwallowedInsideTheOperationIsRaisedAgainByCommitAndReplayed: both writers commit, at least one was replayed, the records agree; red at the base with the bare DeadlockException - the swallowed variant with the one commit() raises;
  • testWriteGivesUpAfterTheAttemptCap, testWriteGivesUpOnTheWindowWhenAttemptsAreSlow: the message names the bound, the cause is null, the conflict is suppressed, the record is untouched; testWriteIsReplayedUntilTheConflictClears, testWriteIsReplayedOnceWhenTheFirstAttemptOutlastsTheWindow: the attempt after the holder's commit is the one which applies;
  • testInterruptedWriteReportsTheConflictItWasReplaying: the flag is clear afterwards and the exception carries both. To deliver the interrupt to the sleep and to nothing of JE, it runs on the storage's import environment, whose writes open no transaction, with an operation raising a conflict JE raised earlier: a transaction aborted with the flag set takes the environment down before the sleep is reached, which is what the first version of this case did to every test after it;
  • testRetryDelayGrowsAndStaysBounded, the PDB pin for the JE copy.

The four holder cases and the delay pin need the bounds and the constructor, so they are red at the base by construction rather than by assertion. Green here: JEStorageTest 8/8 five times in a row, JETestCase, EncryptedJETestCase, PDBStorageTest, ReplayedConfigChangeTest.

Not in this change

  • The loop is the third copy of one shape (PDB, JDBC, JE), each with an engine-specific conflict and give-up. A shared helper is a refactoring of its own.
  • JEStorage.read makes one attempt as well. A non-transactional reader can only be a deadlock victim inside a cycle, and with forward cursor walks against writers which lock in one order there is none to close; left alone.
  • The one lock-order inversion found by reading: EntryContainer.renameEntry updates the old superior's count before the new one's, so two opposite renames between the same parents on threads whose ids agree modulo 256 (the counter shard) can deadlock. Reachable, rare, and now replayed rather than reported; not otherwise addressed.

…onflict instead of giving it up at once

JEStorage.write made one attempt: the DeadlockException JE throws at the
victim of a deadlock - the one conflict left with je.lock.timeout at 0 -
reached the caller bare after that attempt, and the LDAP client was answered
with a server error carrying JE's account of its lock table. PDBStorage and
JDBCStorage replay under an attempt cap and a retry window; JEStorage now
does the same for a LockConflictException, and gives it up the way they do.

Three points are JE's own. The conflict is raised inside the operation and,
should the operation swallow it, again by commit(), so both forms are
matched and a swallowed conflict commits nothing. The replay backs off
first: a record is locked by the LSN of its current version, the victim's
abort undoes the version the survivor was granted the lock on, and a replay
which comes back at once wins the race for the version put back and forms
the deadlock again. And the interrupt flag is not restored when the backoff
is interrupted, since JE invalidates the whole environment when a thread
carrying the flag makes any call - which the caller's failure road does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug concurrency Thread-safety / race-condition bugs java Changes to Java sources tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JEStorage.write makes one attempt: a transaction JE ends as the victim of a deadlock is given up rather than replayed

1 participant