[#925] Keep asking for a session restart until it has run by vharseko · Pull Request #981 · OpenIdentityPlatform/OpenDJ

vharseko · 2026-09-09T06:19:04Z

Fixes #925.

Was stacked on #958, which has landed (776339a). The branch now sits on master directly - at 600df92, which is #950 (#978) - and the diff is four commits: 03e9cd3 ([#925] Keep asking for a session restart until it has run), df61f75, which moves its message ordinal, 82eda0e, review round 1, and c659a42, review round 2 (the last section). The rounds #958 went through while this was open reached into the same code, so the rebases along the way were a merge of decisions rather than of hunks - the last section says which, and it still describes how this change meets what #958 put on master.

What was wrong

runRequestedSessionRestarts() takes the request before it runs the restart, and restartSession() stops the session in its first synchronized block and starts it again in the second. Anything thrown in between left the request cleared and the session stopped, and nothing asked for it again: no change is delivered over a session which is down, so no replay fails and no thread comes back to the recovery. The domain sat out of the topology - the change it could not replay still owned by the replication server, its ServerState stopped behind it - until the server was restarted or a configuration change happened to call restartService().

Worth being precise about the trigger, because the issue text names one which does not happen: broker.start() on an unreachable replication server does not throw. connectAsDataServer() swallows every connection failure - performPhaseOneHandshake() catches ConnectException, SocketTimeoutException and Exception, connectToReplicationServer() catches Exception - and leaves the domain in degraded mode, where the listener thread reconnects on its own. What makes the throw reachable is narrower and, in one respect, damning: enableService() is the one call on that road nothing guards, while the reconnect loop of the broker wraps the same connectAsDataServer() in a catch (Exception). Every failure that path is written to survive is fatal to this one, and what is fatal about it is the missing listener thread: it is the reconnection engine, so a domain which loses it has nothing left to bring the session back.

What changed

SessionRestartRequests (new) holds what the domain has been asked for - NONE < NOW < AFTER_BACKOFF, merged by taking the strongest. The request is still taken before the restart runs, which it must be: a change released while a restart is under way is not one that restart asks for, its delivery would be turned down as a duplicate of a change a replay thread still owns. Clearing the flag after the restart instead - the other half of the suggestion in the issue - would swallow exactly those requests.
A restart which could not run gives its request back, with the backoff whatever it was asked for with: a session which can not be started is what that wait exists for, and a NOW request given back bare would have the retry hammer once a second. [#922] Give a change back when the replay which owns it is unwound #958 has since put the request back on its own (round 2, blocking 4) - a finally which sets the flag again - and this replaces it: the give-back carries the backoff, and, the point of this PR, something runs what was given back. Under [#922] Give a change back when the replay which owns it is unwound #958 alone the restored request is run by "the next failed or abandoned replay of this domain", and on a session which is down there is none.
The wait belongs to the request rather than to the thread which runs it (the first asymmetry in the issue). A replay thread on its way out is owed no backoff - the backend is not what is going away - and a failing replay is owed one; either thread can end up running the other's request. Round 3 of [#889] Keep a change the replay could not apply out of the ServerState #892 traced this as unreachable ("a stopping thread's zero-delay restart cannot coexist with a requester owed a backoff") and it very nearly is: stopReplayThreads() sets the shutdown flag of each thread in a loop, so the window is the one between the first flag and the last. It costs nothing to close it here, since the request had to carry its own state anyway. The thread an OutOfMemoryError is ending, which [#922] Give a change back when the replay which owns it is unwound #958 exempts from the backoff for the same reason, asks for NOW as well.
The state checkpointer runs a request nobody else will, once a second. It is the one thread a domain has for as long as it is up, whatever its session is doing. It runs the restart outside its own monitor - shutdown() takes that monitor to wake it - and swallows and reports a failure rather than end on it: a domain whose checkpointer is gone saves no ServerState, and shutdown() waits for that thread. The report itself is guarded the way the report of a give-back which failed is: on the road out of a JVM which has refused an allocation, building the line is a throw of its own, and it must not end the thread either.
The checkpointer holds its restart back while a total update is being processed, in either direction (ieRunning()). A restart stops the session the total update runs over. An import into this replica reads its entries from that session and would end on the ones which had arrived - and disabled does not say such an import is running, since preBackendImport() keeps this domain's own backend events from disabling it, so ownsItsSession() does not see it. An export from this replica publishes its entries over it, and exportLDIFEntry() gives the export up as ERR_INIT_RS_DISCONNECTION_DURING_EXPORT once the broker has been stopped under it, which leaves the replica it was initializing to be initialized again. The checkpointer is the thread which can afford to wait: the request stays standing, it ticks once a second, and the restart runs as soon as the context is released; the change waits for as long as the total update takes, and the ServerState with it, while the replay keeps running. The two roads which restart synchronously - the replay thread's own restart and the last resort of replay() - still restart mid-export, as they do on master: gating them the same way is only safe once something runs what they leave standing, which is this change, and is left to Replication: a session restart run by the thread which released a change cuts the export the session carries #1048. The import direction of those two roads is A failed entryUUID search reads as a deleted entry, and conflict resolution records the change as replayed #956 ([#956] Tell a failed entryUUID search apart from an entry which is not there #968).
abandonReplay() asks for the restart rather than running it (the second asymmetry). The threads of the pool are stopped one after the other and joined, so a live ds-cfg-num-update-replay-threads change cost one stop and reconnect per thread which was replaying a change - none of them waiting, each one a listener join plus a full handshake, and the configuration change waited for all of them. One restart is all the replication server needs to send back every change which was handed back.
The last resort of replay() keeps running its restart itself, as [#922] Give a change back when the replay which owns it is unwound #958 decided in its round 1 (non-blocking 4): on that road the thread is likely ending and the report is the line an operator acts on. What this branch adds there is the same as everywhere else - a restart which throws leaves its request standing, given back with the backoff - except that on this road the last resort itself runs what is standing, on the same thread and backoff and all, so the first run of the restart is the only one without the wait; only a restart which throws there as well is left to the checkpointer, where before the request went out with the thread.
The backoff is waited on a monitor rather than slept through, and disable() / shutdown() wake it. This is not in the issue; it is what the checkpointer needs in order not to hold a domain going down for up to ten seconds, and it removes the same stall for a replay thread stopReplayThreads() is joining. The wake is counted as well as flagged: disabled does not stay set, so a checkpointer descheduled between disable()'s notifyAll() and its own re-check for as long as enable() takes to clear the flag would find nothing to wake for and wait out the rest of the backoff, for a session the restart has nothing left to start. The count is taken where the session is stopped, under serviceStateLock, so a disable() which has yet to take that lock is one the wait sees whether it is under way yet or not.
enable() clears the holder, as disable() does. A request made in the window between a replay thread's read of disabled and disable()'s own clear - abandonReplay() reads the flag, logs, then asks - would survive the disabled span, and the checkpointer would restart the session enable() just started, within the second, for a change which is gone with the pending changes. Every request standing at enable() is that one.

Tests


`SessionRestartRequestsTest` (new)	7/7 - take/give-back, the merge, that a request given back does not undo the one made meanwhile, and that a `NOW` take given back as `AFTER_BACKOFF` is owed the backoff
`UpdateOperationTest.aSessionRestartWhichCouldNotRunIsRunAgain` (new)	green, 8 s. Two restarts fail: the replay thread's, and the checkpointer's for the request it gave back. The checkpointer reports the one which threw on it once - or twice, when its tick takes the replay thread's request as well - and runs it again, which is what delivers the change; before the fix the entry is still there 60 s after the delivery which failed
`UpdateOperationTest.aRestartAskedForWithoutTheBackoffIsGivenBackWithIt` (new)	green. A replay thread stopped while it holds a change - #941's fixture - hands it back and asks for the restart without the backoff; the checkpointer's restart for it fails, and 1.5 s after the throw the session is still down: the request was given back with the backoff, not as it was made
`SessionRestartBackoffTest` (new)	3/3 - a domain of its own whose checkpointer sits in a three-second backoff: `shutdown()` and `disable()` each return in a fraction of it, and after `disable()` / `enable()` the checkpointer saves a change of the replica's own within its next tick; and a request left standing across a `disable()` / `enable()` is not run against the session `enable()` started
`ReplayDuringImportTest.aRequestWhichStoodWhileTheImportRanIsNotRunOnceItIsOver` (new, in #968's class)	green. A request left standing while the import streams is not run against the session started back at its end
`UpdateOperationTest` / `ReplayDuringImportTest`	33/33, 4/4 - the 31 and 3 of `master` plus the ones above
`RemotePendingChangesTest` / `AssuredReplicationPluginTest` / `DependencyTest`	21/21, 14/14, 3/3
`DisabledDomainServerStateTest` / `SessionRestartTest` / `LDAPReplicationDomainConfigChangeTest`	2/2, 2/2, 9/9 - the classes which disable, enable or restart a domain, run for the `enable()` change

The tests were checked against the mutations they exist for, each mutant compiled in place of the class and run by itself:

mutant	result
the checkpointer's call to `runPendingSessionRestart()` removed	`aSessionRestartWhichCouldNotRunIsRunAgain` red: the change is never delivered
the give-back after a restart threw removed	red: the change is never delivered
the checkpointer's `catch` rethrows	red: the change is never delivered, and the checkpointer is gone with it: on `master` since #977 its `shutdown()` no longer waits for a thread which has ended, so what is lost is the checkpointing of the domain and the restart nobody else runs, not its shutdown - the test JVM ends by itself, in 72 s
`Thread.sleep()` in place of the monitor wait, inside the monitor - the checkpointer holds it through the sleep	the `disable()` case red on `disable()` (2804 ms), the `shutdown()` case red on `shutdown()` (2764 ms): each is held by the wake it gives, which takes the monitor
`Thread.sleep()` in place of the monitor wait, outside the monitor - the whole synchronized loop replaced	the `shutdown()` case red on `shutdown()` (2786 ms), the `disable()` case red on the checkpoint, which did not come within 2500 ms: the wake reaches nothing
the wake removed from `disable()`	the `disable()` case red on the checkpoint (2500 ms), the `shutdown()` case green
the wake removed from `shutdown()`	the `shutdown()` case red (2806 ms), the `disable()` case green
the wake count dropped from the wait's condition - a wake checked for by its flag only	green under every case: the window is the checkpointer being descheduled between `notifyAll()` and its re-check for as long as `enable()` takes, which no test can hit on purpose. Closed by construction, unpinned
the clear in `enable()` removed	`aRequestWhichStoodWhileTheDomainWasDisabledIsNotRunOnceItIsEnabledBack` red: the failure was spent, the request was run against the session `enable()` started
the clear at the end of `importBackend()` removed	`aRequestWhichStoodWhileTheImportRanIsNotRunOnceItIsOver` red: the failure was spent, the request was run against the session started back at the end of the import
`giveBack(restart)` in place of `giveBack(AFTER_BACKOFF)` - the request given back as taken	`aRestartAskedForWithoutTheBackoffIsGivenBackWithIt` red: the session was started back within 1500 ms of the restart which threw. `aSessionRestartWhichCouldNotRunIsRunAgain` and the three cases of `SessionRestartBackoffTest` green, as they take `AFTER_BACKOFF` already

Every row was run at this head, one JVM per run, with the mutant compiled in place of the class.

The backoff test acts on the domain by the clock: the checkpointer takes the request back one second after it reported the failure - its wait(1000) - and holds the backoff for three, and the test acts a fifth of a second into those three. A machine slow enough to push the checkpointer's tick past that delay has the test act before the backoff begins, which a slept-through wait survives: the test then proves less, but reports nothing false. The give-back case in UpdateOperationTest is of the same kind: it looks at the session 1.5 s after the throw, halfway between the second the checkpointer's next tick is away and the further second the backoff adds, and a tick pushed past that delay has it look before the session could be back under either level.

Two things to say plainly rather than bury.

failNextSessionRestarts(int) / getSessionRestartFailuresLeft() / requestSessionRestart() are @VisibleForTesting and there is a branch in restartSession() which production never takes; the tests of the checkpointer stand on the first two, and the two which pin the clears in enable() and importBackend() on the third, since a request which stands across a disabled span or an import is made in a window no test can hit on purpose. Nothing else reaches enableService(): connection failures are swallowed inside it, and the machinery #958 added throws from a replay, not from a restart. The precedent this PR first named, setReplayGiveUpDelay(), is gone - #901 replaced it with the configured replay-give-up-delay - so the one left in the class is setReplayDrainTimeout() from #945. If you would rather have no injection point in the class, the alternative I can see is dropping the integration test and keeping only the unit test of the holder, which leaves the give-back and the checkpointer unpinned.

The collapsed restarts of a num-update-replay-threads change are half pinned. aRestartAskedForWithoutTheBackoffIsGivenBackWithIt stops one replay thread while it holds a change and reads the restart the checkpointer runs for it, so "the thread asks rather than runs, and the checkpointer runs it" is pinned; "one restart rather than one per thread" is not, since that needs as many parked threads as changes, and the fixture parks one.

#941 has since landed (d0422c6), and aChangeAStoppedReplayThreadHeldIsGivenBackAndDeliveredAgain sits in this class next to the change it was claimed to survive, and it is green. It is worth saying why it survives rather than only that it does. That test stops a replay thread while it holds a change and reads the assured ack the abandoned delivery published; under this change abandonReplay() no longer restarts the session itself, so the ack now goes out over a session which is still up, where before it raced the teardown. The test is not merely still valid - it is the less brittle for this change. Its comment on reading the ack rather than its ordering ("an ack published after the hand-back reaches it all the same") describes a race this change removes.

Rebased onto #958 as it went, and onto `master` once it landed

Two rounds of #958 landed on the same code between the opening of this PR and now, and git reported six hunks in LDAPReplicationDomain. Three were mechanical - #901 removed the replayGiveUpDelayInMs field and its setters this branch had sat next to, so they go; the outOfMemory parameter #958 gave recoverFromReplayFailure() becomes the request being NOW. Three were decisions:

[#922] Give a change back when the replay which owns it is unwound #958 round 2 put the request back itself, with a finally around restartSession() which sets the flag again. Replaced by giveBack(AFTER_BACKOFF), for the two reasons above: the backoff travels with the request, and the checkpointer runs it. The concrete trigger that round named - the operating system refusing the listener thread - is kept in the comment.
[#922] Give a change back when the replay which owns it is unwound #958 round 1 has the last resort of replay() run the restart rather than leave a request. Kept as decided there; the request is made NOW first, so a restart which throws is given back and run by the checkpointer instead of going out with the thread. The earlier text of this PR, which said that road "asked for a restart nothing was going to run", described the [#922] Give a change back when the replay which owns it is unwound #958 of the time and is gone.
The ordinal. With this branch on the current [#922] Give a change back when the replay which owns it is unwound #958, ERR_ACK_NOT_PUBLISHED_316 and the 316 this branch first took met in one hunk, and git did report it this time. 0e91a8f resolves it to ERR_REPLAY_SESSION_RESTART_FAILED_325; the file has no ordinal twice. The open PRs which add to replication.properties still hold 310-325 with nothing claimed twice.

The diff of the two commits against master is 506 insertions and 67 deletions over the same five files as before.

Rebased once more onto #958 round 4 (9618f12, on master at 13d57e0). Round 4 only grew the comment inside the finally this branch replaces, so the resolution above stands and those lines go with the block; the twelve of them are the whole difference from the previous version of the two commits. What put both PRs back into conflict was #928 (#973), which removed the NPE #958's test scaffolding relied on to fail a replay before the CSN is read - #958's own rebase moved that scaffolding to ModifyMsgWhoseOperationRefusesAControl, and this branch sits on it as it is.

#958 has since landed as 776339a, a squash of the 9618f12 this branch sat on, with the same tree. Rebased onto master there: the five commits it carried went, the two of this PR moved without a hunk to resolve, and the tree of the new head is the tree of the old one - git diff ca9ba89 823980c is empty.

Rebased once more onto master at 9ff409b, where #964 landed. It took WARN_CHANGELOG_READ_AGAIN_FOR_MISSING_CHANGES_321 at the end of replication.properties, next to the message this branch adds, and that was the whole conflict: one hunk, both sides kept, 321 above 325. Nothing in the Java met #964 - it changes the replication server's catch-up, this branch changes the domain's restart - and the two commits are what they were, 506 insertions and 67 deletions over the same five files. 325 is claimed by nothing on master, which holds 310-317, 319-321 and 326-327; the file has no ordinal twice.

Rebased once more onto master at cebef54, where #926 (#974) landed. It moved serviceStateLock and sessionGeneration up from LDAPReplicationDomain into ReplicationDomain, and this branch adds sessionRestartFailuresToInject right where they used to be - one hunk, the field block, resolved by taking master's side and keeping only the new field. Everything else merged without a word, and a range-diff against the previous version of the two commits shows context only: the moved declarations, and the sessionGeneration++ #974 took out of restartSession() next to failSessionRestartIfATestAskedFor(). #974 reached into the same restart, so the meeting point is worth a line: it counts the generation inside enableService(), and only once the session is up, so a start which threw leaves the generation where the stop put it. The injected failure of this branch sits between the generation guard and enableService(), which is the same case - the thread which stopped the session still owns it, and the retry the state checkpointer runs goes through a fresh disableService() as any other restart does. Nothing in this branch had to move for it. The two commits are still 506 insertions and 67 deletions over the same five files; 325 is still claimed by nothing on master, and the file has no ordinal twice.

Review round 1

Blocking 1, the checkpointer's catch pinned by no test. aSessionRestartWhichCouldNotRunIsRunAgain now fails two restarts rather than one - the replay thread's and the checkpointer's - captures the error log over the delivery with errorLogRecordsOf(), and asserts that ERR_REPLAY_SESSION_RESTART_FAILED is reported exactly once before the change is delivered. The catch → rethrow mutant is red, and is the third row of the table above. The count of one assumes the replay thread runs its own request, which it does unless the checkpointer's tick lands in the instants between the request being made and being taken; the assertion says so.
Blocking 2, the ieRunning() gate. Deliberate, and now said where it can be read: the javadoc of runPendingSessionRestart() and the bullet above. Both directions are in it - the import because ownsItsSession() does not see an online total update into this replica at all, the export because exportLDIFEntry() gives the export up on the broker stop. The two synchronous roads during an export are Replication: a session restart run by the thread which released a change cuts the export the session carries #1048; during an import they are A failed entryUUID search reads as a deleted entry, and conflict resolution records the change as replayed #956 ([#956] Tell a failed entryUUID search apart from an entry which is not there #968).
Non-blocking, the unguarded report. Guarded, in the shape of the give-back's report in replay().
Non-blocking, the holder cleared in enable(). Done, under the lock, first thing in enable(), with the window it closes in the comment.
Non-blocking, the backoff wait and its wakes. SessionRestartBackoffTest (new, two cases, a replication server and a domain of its own on o=test after the pattern of SessionRestartTest) pins both wakes, and each wake is pinned by its own case: the mutant table shows which case catches which.
Non-blocking, the give-back level. giveBack() takes what the caller asks for again, and its javadoc says what that is and why it is not what take() returned; the class javadoc says which thread is owed the wait and which is not. aRestartGivenBackAfterItThrewIsOwedTheBackoff pins the merge for the production constant.

The round is one commit on top of the two, c2455f8: 381 insertions and 15 deletions over five files, SessionRestartBackoffTest the new one. The three commits against master are 874 insertions and 68 deletions over six files.

Rebased onto `master` at `eef0757`, where #968 (#956) and #977 (#952) landed

Git reported nothing, and the merge did not compile. git merge-tree of the branch and master had no conflicting hunk, GitHub said mergeable, and every CI leg of the previous head failed in Build with Maven on the merge commit it builds: #968 resets the restart request at the end of importBackend(), next to the pending changes and the backoff - sessionRestartRequested.set(false) - and the first commit of this branch replaces that field with the holder. Different parts of the file, so git merged both sides as they were. Fixed in that first commit, where the field goes: sessionRestarts.clear(), which is what disable() and enable() do on the same road. A range-diff against the previous version of the three commits shows that line, the context #968 and #977 moved, and one comment: the checkpointer's runPendingSessionRestart() was "run outside the block above", and #977 moved the save it referred to out of that block too, so it now says "outside the monitor above, as the save is".

#977 met this change at the checkpointer, and the two agree. It takes state.save() out of the checkpointer's monitor into saveState(), which keeps a failed write to itself, and replaces the while (!done) of shutdown() with a bounded join; this branch runs runPendingSessionRestart() right after that save, outside the same monitor, and guards its own report. saveState() catches RuntimeException, so an Error out of the report would still end the thread there - the guard on the report stays. What #977 changes about the mutant table is the catch → rethrow row, re-run on this head: the checkpointer which ends on the rethrow no longer hangs the test JVM's cleanup, since #977's join() returns at once for a thread which has ended - the case is red as before, and the JVM ends by itself, where before it spun on done. What that mutant costs is the checkpointing of the domain, and the restart nobody else runs; not its shutdown.

#968 met it at the roads which ask for the restart. sessionHasAnOwner() - ownsItsSession() || importInProgress() - is what recoverFromReplayFailure(), abandonReplay() and restartSession() now refuse the restart on, and the request this branch makes sits after that refusal on each of them, as the flag did. The checkpointer's gate is ieRunning(), which covers importInProgress() and the export besides, so it did not have to move. The import-end reset above is the one place the two touched the same state.

Ordinals: #968 took 322, #977 took 323 and 324, this branch keeps 325; the file has no ordinal twice.

Review round 2

Blocking, the disable case does not time disable(). As measured. The sleep the table's row was run with was the one outside the monitor - the whole synchronized loop replaced by a sleep - which the row gave away without saying so: "the checkpoint did not come within 2500 ms" is a checkpointer which sleeps through enable(), where one which sleeps inside the monitor holds disable() instead and then saves inside the bound. The case times disable() now, the way the shutdown case times shutdown(), against the one bound both wakes share - WAKE_BOUND_IN_MS - and both shapes are rows of the table: inside the monitor, red on disable() (2804 ms) and on shutdown() (2764 ms); outside it, red on the checkpoint and on shutdown() (2786 ms).
Non-blocking, the count of 325. reported == 1 || reported == 2, and the comment says which road gives two - the checkpointer's tick between the replay thread's request() and the CAS spends both failures - and why none has no road.
Non-blocking, the wake is not sticky. Counted, with the count read where the session is stopped rather than where the wait begins: restartSession() takes it in its first block, under serviceStateLock, before it lets go of the session it stopped. Read at the entry of the wait, the same window would sit between that block and the wait - a disable() and an enable() which both land there leave the count bumped already when it is read. disable() takes serviceStateLock before it wakes, so a wake not yet given when the block lets go is one the wait sees, whether it is under way by then or not, and one given before it is a disable() the block saw as the owner of the session. The session generation would have said the same and is not read: [#926] Restart the session of a replication domain in one place, under the lock and the generation #974 keeps it under the lock, and the wait is deliberately outside it. shutdown() wakes before it takes the lock and needs no count, its flag never comes back. Unpinned, and the table says so: the window is the checkpointer being descheduled for the length of enable(), which no test can hit on purpose.
Non-blocking, the two holder clears. Both pinned, on requestSessionRestart() - the third @VisibleForTesting in the class, which the "say plainly" paragraph now lists. The enable() case is the third of SessionRestartBackoffTest; the importBackend() case sits in [#956] Tell a failed entryUUID search apart from an entry which is not there #968's ReplayDuringImportTest, which has the import fixture: a request made while the import streams, a failure left to inject, and at the end of the import the failure is still there to spend and the session is up. And a correction to the rebase note: [#956] Tell a failed entryUUID search apart from an entry which is not there #968's first case runs through the import-end reset, it does not pin it - during an import abandonReplay() refuses on sessionHasAnOwner(), so nothing stands there to be cleared, and deleting the line survives that class. It does not survive the new case.
Non-blocking, the give-back level. Pinned on the road named: aRestartAskedForWithoutTheBackoffIsGivenBackWithIt parks a replayed delete with [#909] Cover the change a stopped replay thread hands back to the replication server #941's fixture, changes the number of replay threads so that the thread which holds it hands it back on its way out and asks for the restart without the backoff, and fails the restart the checkpointer runs for it. 1.5 s after the throw the session is still down: given back with the backoff, the request is taken on the checkpointer's next tick and waited a second, so the session is back two seconds after the throw; given back as taken, it is back on that tick. The giveBack(restart) mutant is red on it and green on every other case, as the round said it would be. It is also the first half of a pin the description said this PR had none of: the collapsed restarts of a thread-count change.
Suggestion, startDomain() and the finally. Assigned before the assert, as in SessionRestartTest, and the cleanup is one release() whose three steps are chained in nested finally blocks.
Question, the wake-removal rows. Not run at c2455f8: they were run on 916afd5, the head of round 1 before the rebase, whose round-1 commit the rebase left byte for byte - so those rows measured this code, but the description did not say when. Every row is re-run at this head, and the table says so.
Nitpick, the OOME road. As read: the first run of the restart on that road is without the backoff, one which throws is given back with it, and the last resort of replay() re-runs what is standing on the same thread, backoff and all; only one which throws there as well is left to the checkpointer. The comment at the call says that now, the bullet above has its clause, and 317 says a restart is asked for rather than under way.

The round is one commit on top of the three, c659a42: 374 insertions and 39 deletions over five files, ReplayDuringImportTest the one this PR had not touched before. The four commits against master are 1214 insertions and 73 deletions over seven files.

Rebased onto master at 600df92 in the same push, where #950 (#978), #913 (#979), #1046 (#1047) and #1031 (#1033) landed. Nothing met this branch: #978 is the one which touches LDAPReplicationDomain, in the constructor and in publishReplicaOfflineMsg(), and range-diff shows all four commits moved as they were. 325 is still claimed by nothing on master - the four landings added to core.properties, not to replication.properties - and the file has no ordinal twice.

vharseko · 2026-09-09T09:42:00Z

Force-pushed: rebased on master at 2a7bb9d (was 92d88ca). Nothing in the change itself was touched - the diff against master is byte for byte the one that was here before, 1369 insertions and 75 deletions over the same nine files.

The conflict was one hunk, the import block of UpdateOperationTest: master had gained TimeoutException and AtomicReference from #941, this branch adds Supplier in the same place. Resolved by taking both.

What the rebase actually brought is worth a look, because it settles something this PR had only asserted. The last section of the description said the collapsed restarts of a num-update-replay-threads change have no test of their own here, that writing one would need the replay parking ShortCircuitPlugin gets in #941, and that #941's own test "stays valid under this change". #941 has now landed, so aChangeAStoppedReplayThreadHeldIsGivenBackAndDeliveredAgain sits in this class next to the change it was claimed to survive. It does: 22/22 green.

It survives for a reason worth stating rather than leaving as a passed test. That test stops a replay thread while it holds a change and then reads the assured ack the abandoned delivery published. Under this change abandonReplay() no longer restarts the session itself - it asks, and the state checkpointer runs it - so the ack now goes out over a session which is still up, where before it raced the teardown. Its own comment covers that race from the other side ("an ack published after the hand-back reaches it all the same"); this change removes it. The test is not merely still valid, it is the less brittle for this branch.

Run after the rebase, all green:


`UpdateOperationTest`	22/22
`AssuredReplicationPluginTest`	14/14
`IsServerFailureTest` (from `master`, #939)	23/23
`RemotePendingChangesTest`	18/18
`SessionRestartRequestsTest`	6/6
`DependencyTest` / `PendingChangesTest` (from `master`, #918)	3/3, 3/3

The two master classes are in the list because they are the ones which touch the files this branch changes: #939 opened isServerFailure() up for its test and #918 changed publishReplicaOfflineMsg(), both in LDAPReplicationDomain. Neither meets this change; they were run to say so rather than to assume it.

The description has been updated: the SHA to review is now 8f9a7fd, and the closing section says what became of the conflict it predicted.

One thing unchanged by the rebase: this is still stacked on #958, which is itself conflicting with master right now. The first two commits here are #958 rebased with that same import resolution, so it can be lifted straight over when you get to it.

vharseko · 2026-09-09T11:01:57Z

Ordinal moved: ERR_REPLAY_SESSION_RESTART_FAILED 316 → 325 (31d7dfa).

Nothing catches this on the way in. The additions land in different parts of the file, so git merges
every pair of them without reporting a conflict - I merged all ten onto master to check, and the
result carried 310, 311, 315 and 316 twice each. The generator does not check either: it loads the
file into a Properties and keys on name and ordinal (MessagePropertyKey.compareTo), so both
sides compile. What comes out is two unrelated messages carrying one support ID, found by whoever
reads a log rather than by CI.

The open PRs which add to the file now hold 310-325 with nothing claimed twice:

310-313 #935 · 314 #959 · 315-317 #958, #985 · 318 #985, #988 · 319-320 #945 · 321 #964 ·
322 #968 · 323-324 #977 · 325 #981

No Java moved with it: the generated constant is the key name without its ordinal, so the rename is
confined to replication.properties. #935, #958 and #985 keep what they had.

This branch is not one of the six: 316 was free where it sits, on the earlier [#922] commits, from
before ERR_ACK_NOT_PUBLISHED existed. #958 gives that name 316, and #985 carries it, so the two
meet on master. ERR_ERROR_REPLAYING_CHANGE_315 is the [#922] message itself and stays where the
rest of that stack has it.

vharseko · 2026-09-11T05:35:36Z

Force-pushed: rebased onto the current #958 at 31e633d (was 00fead7, the #958 of two review rounds ago), which sits on master at 5d176c6. The commits to review are 5323358 and 0e91a8f.

This time the conflict was not one of hunks. #958 went through two rounds since this was opened, and both reached into the code this PR changes, so what git reported in LDAPReplicationDomain - six hunks - had three decisions inside it, resolved as follows.

[#922] Give a change back when the replay which owns it is unwound #958 round 2 put the request back itself (its blocking 4): a finally around restartSession() which sets the flag again when the restart ends abruptly. This branch replaces it with giveBack(AFTER_BACKOFF), and it is worth saying why the replacement is not a preference. The [#922] Give a change back when the replay which owns it is unwound #958 version restores the request for "the next failed or abandoned replay of this domain" to run - and on a session which is down there is none, which is what Replication: a session restart request is consumed before the restart runs #925 is. The give-back here also carries the backoff, so a session which can not be started is not asked for again once a second. The trigger that round named - the OS refusing the listener thread - is kept in the comment.
[#922] Give a change back when the replay which owns it is unwound #958 round 1 has the last resort of replay() run the restart itself (its non-blocking 4) rather than leave a request standing. Kept as decided there: the request is made NOW first, runRequestedSessionRestarts() follows, and a restart which throws on that road is now given back and run by the checkpointer instead of going out with the thread. The earlier text of this PR, which said that road "asked for a restart nothing was going to run", described the [#922] Give a change back when the replay which owns it is unwound #958 of the time and is gone from the description.
The outOfMemory road [#922] Give a change back when the replay which owns it is unwound #958 added to recoverFromReplayFailure() skipped the backoff by parameter; here the wait belongs to the request, so a thread an OutOfMemoryError is ending asks for NOW, the way a stopping one does.

The mechanical three: #901 removed replayGiveUpDelayInMs and its two setters, which this branch had sat next to - gone, and the new test never used them; and the ordinal, which this time git did report, since ERR_ACK_NOT_PUBLISHED_316 and the 316 this branch first took landed in one hunk. 0e91a8f resolves it to 325, as before; no ordinal is in the file twice.

One consequence for the description's "say plainly" paragraph: the precedent it named for the test hook, setReplayGiveUpDelay(), no longer exists - #901 replaced it with the configured property - so the one left in the class is setReplayDrainTimeout() from #945. Rewritten to say so.

Run after the rebase, all green:


`UpdateOperationTest`	30/30 - the 29 of #958 plus `aSessionRestartWhichCouldNotRunIsRunAgain`
`SessionRestartRequestsTest`	6/6
`RemotePendingChangesTest`	21/21
`AssuredReplicationPluginTest`	14/14
`DependencyTest`	3/3

The description is updated: the SHAs, the bullets the two #958 rounds touched, and a closing section which says what was merged and how.

vharseko · 2026-09-11T18:37:01Z

Force-pushed: rebased onto the current #958 at 9618f12 (was 31e633d), which sits on master at 13d57e0. The commits to review are 31f46f7 and ca9ba89.

The two commits of this PR are what they were: 506 insertions and 67 deletions over the same five files. The one difference from the previous version is twelve lines of a comment #958 round 4 grew inside the finally this branch replaces with giveBack(AFTER_BACKOFF) - they go with the rest of that block, for the reasons given in the previous rebase note. The other hunk git reported was replication.properties, where #959's 326 and 327 landed next to the message this branch adds: both sides are kept, and 325 is still claimed by nothing else among the open PRs.

What put both PRs back into conflict is worth a line, because it was not textual. #928 (#973) removed the NPE in getEntryDN().equals(SET_PERMISSIVE_MODIFY_FOR_DN) that #958's test scaffolding was built on: ModifyMsgWithAnUnparseableOperationDN no longer fails the replay before OperationContext.getCSN(op) is read - the operation runs, reports the syntax of its DN, and the change is stepped over. Git reported one hunk of that and merged the rest without a word; taken at face value, four of #958's cases would have compiled and asserted a give-back of a change the replay had just recorded as applied. #958's rebase moves them to ModifyMsgWhoseOperationRefusesAControl, the message #928 gave the #889 test in its place - addRequestControl(ManageDsaIT) throws on the empty control list, the same road one line earlier - and this branch sits on that rebase as it is. Nothing in LDAPReplicationDomain moved for it.

Run after the rebase, all green:


`UpdateOperationTest`	32/32 - the 30 of before, plus `aModifyWhoseEntryDNDoesNotParseIsReportedRatherThanThrownOn` from #928 and `aChangeHandedOutAsADependencyIsGivenBackWhenItsReplayIsUnwound` from #958 round 4
`SessionRestartRequestsTest`	6/6
`RemotePendingChangesTest`	21/21
`AssuredReplicationPluginTest`	14/14
`DependencyTest`	3/3

The description is updated: the SHAs and the base.

vharseko · 2026-09-12T13:22:19Z

Force-pushed: rebased onto master at 776339a (was on #958 at 9618f12). #958 has landed, so this is no longer stacked on anything: the commits to review are 6d6b379 and 823980c, and the diff against master is 506 insertions and 67 deletions over the same five files.

The conflict GitHub reported was the squash, not the code. 776339a is #958's 9618f12 squashed onto master, with the same tree, while this branch still carried the five commits of it. Moving the two commits over them left no hunk to resolve, and the tree of the new head is the tree of the old one - git diff ca9ba89 823980c is empty.

Ordinal 325 is still claimed by nothing on master, which holds 310-317, 319-320 and 326-327; the file has no ordinal twice.

Run on the new head: SessionRestartRequestsTest 6/6 - and that is the only class re-run locally. The tree being the one of the previous note, its runs (UpdateOperationTest 32/32 among them) were runs of this code; CI runs the rest on this head.

The description is updated: the stacking paragraph is gone, the SHAs and the base, and the closing section says what the squash did to the branch.

vharseko · 2026-09-14T07:41:50Z

Force-pushed: rebased onto master at 9ff409b (was 776339a). The commits to review are 1dc39e1 and 3592ab5, and the diff against master is 506 insertions and 67 deletions over the same five files.

The conflict was one hunk, the tail of replication.properties: #964, which is 9ff409b, took WARN_CHANGELOG_READ_AGAIN_FOR_MISSING_CHANGES_321 where this branch adds its message. Resolved by keeping both, 321 above 325 - in both commits, since the first adds the message as 316 and the second renames it. master now holds 310-317, 319-321 and 326-327; 325 is still claimed by nothing, and the file has no ordinal twice.

Nothing in the Java met #964: it changes MessageHandler and DataServerHandler on the replication server, this branch changes the domain's restart, and git merged the rest without a word. A range-diff of the two commits against their previous version shows the properties context and nothing else.

About the red leg on the previous head (JDK 26): it was aChangeHandedOutAsADependencyIsGivenBackWhenItsReplayIsUnwound from #958 asserting which replay thread picks a parked change up - #1036, which #1037 addresses - and the seventeen failures behind it are its cascade, the class not replaying anything once that test has failed. The other four ubuntu legs of the same run passed the class. Not this change.

Run on the new head: SessionRestartRequestsTest 6/6, and a test-compile of the module - ReplicationMessages generates with both 321 and 325 in it. The tree of the two commits being what it was, the runs of the previous notes (UpdateOperationTest 32/32 among them) were runs of this code; CI runs the rest on this head.

The description is updated: the SHAs, the base, and the closing section says what #964 put in the file.

vharseko · 2026-09-14T09:55:50Z

Force-pushed: rebased onto master at cebef54 (was 9ff409b). The commits to review are 0b35ffd and 2a2766f, and the diff against master is 506 insertions and 67 deletions over the same five files.

The conflict was one hunk, the field block of LDAPReplicationDomain: #926 (#974), which is cebef54, moved serviceStateLock and sessionGeneration up into ReplicationDomain, and this branch adds sessionRestartFailuresToInject right where they used to be. Resolved by taking master's side and keeping only the new field - the two moved declarations are not brought back. Everything else merged without a word, and a range-diff against the previous version shows context only: the moved declarations, and the sessionGeneration++ #974 took out of restartSession() next to failSessionRestartIfATestAskedFor().

Worth a line, since #974 reached into the same restart: it counts the generation inside enableService(), and only once the session is up - a start which threw leaves the generation where the stop put it. The injected failure of this branch sits between the generation guard and enableService(), so a restart which fails is that same case: the thread which stopped the session still owns it, and the retry the state checkpointer runs goes through a fresh disableService() as any other restart does. Nothing in this branch had to move for it.

Ordinal 325 is still claimed by nothing on master, which holds 310-317, 319-321 and 326-327; the file has no ordinal twice.

Run on the new head, all green:


`UpdateOperationTest`	32/32 - `aSessionRestartWhichCouldNotRunIsRunAgain` among them
`LDAPReplicationDomainConfigChangeTest` (touched by #974)	9/9
`SessionRestartTest` (added by #974)	2/2
`SessionRestartRequestsTest`	6/6

The description is updated: the SHAs, the base, and the closing section says what #974 did to the file.

maximthomas

praise: SessionRestartRequests is the right shape for this: the request outlives the thread which made it, the merge keeps the strongest level, and take-before-run is kept and argued — the duplicate-delivery reason is the one that matters. Giving the request back after a throw and letting the state checkpointer run what a leaving thread cannot is a clean choice: one thread per domain, already waited for by shutdown(), no new thread to leak. The backoff is waited on a monitor outside serviceStateLock, and disable()/shutdown() wake it, so a domain going away never sits out a backoff it is not owed. shutdown() waits for the checkpointer outside the lock, so a restart on that thread cannot deadlock it. The description is precise where the issue text was not (enableService() unguarded vs the broker's own reconnect loop), the mutant table is the kind of evidence a reviewer can re-run, and the #958 "merge of decisions" section made the rebase auditable. SessionRestartRequestsTest reads as a spec.

issue (blocking): the checkpointer's own failure handling is pinned by no test

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:2407
opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3673-3680

failNextSessionRestarts(1) is spent by the replay thread's own restart; the give-back is pinned, but the checkpointer's catch (Throwable) and ERR_REPLAY_SESSION_RESTART_FAILED_325 are entered by no test. Measured: catch → rethrow in runPendingSessionRestart() leaves aSessionRestartWhichCouldNotRunIsRunAgain green (1 run / 0 failures). Under that mutant a second refusal ends the checkpointer — no more ServerState saves, and shutdown() spins in while (!done).

Two failures reach the catch once; the report comes out exactly once:

domain.failNextSessionRestarts(2);

final CSN csn = gen.newCSN();
final List<String> records = errorLogRecordsOf(() -> {
  broker.publish(new DeleteMsg(tmp.getName(), csn, uuid));
  assertNull(getEntry(tmp.getName(), 60000, false),
      "the change was not delivered again after the session restarts which failed");
  return null;
});
assertEquals(countRecordsOf(records,
    "Could not restart the replication session of domain \"" + baseDN + "\""), 1,
    "the checkpointer reports the restart which threw on it, once, and runs it again");
assertEquals(domain.getSessionRestartFailuresLeft(), 0, "...");

Plus a third row in the description's mutant table: runPendingSessionRestart() catch → rethrow: red (the change is never delivered; the checkpointer is gone).

question (blocking): is the ieRunning() gate on the checkpointer road deliberate?

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3664

ieRunning() covers exports; ownsItsSession() (shutdown || disabled) covers only a total update into this replica. At base, abandonReplay() ran the restart at once, export or not. Now a change abandoned by a thread-count change during an export is redelivered only once the export ends (minutes on a large backend), with the ServerState stopped behind it — while the replay thread's own restart (:3596) and the last resort in replay() (:2629) still restart mid-export. Neither the description nor the javadoc names the gate.

If deliberate (a restart mid-export unwinds the export the session carries): one line in the description and in the javadoc, and the two synchronous roads as a follow-up. If not: drop it, or gate all three.

issue (non-blocking): the report inside the checkpointer's catch is unguarded

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3679

The catch says "It must not end on it", then formats a stack trace — on the OOME road the catch exists for. A throw out of the report escapes, the checkpointer ends, shutdown() spins in while (!done) (:2524; done is set at :668 with no finally). replay() guards its own report at :2662-2672 for exactly this reason; same shape here:

catch (Throwable t)
{
  try
  {
    logger.error(ERR_REPLAY_SESSION_RESTART_FAILED, getBaseDN(), stackTraceToSingleLineString(t));
  }
  catch (Throwable reportFailure)
  {
    // The restart is asked for again already; the report must not end this thread.
  }
}

(Overlaps #977; the guard belongs here whichever lands first.)

suggestion (non-blocking): clear the holder in enable() too

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:4519, :4646

disable() clears the holder after the drain; enable() never does. A request landing after the clear (a queued change of this domain dequeued after awaitReplayDrained() and abandoned) survives the disabled span, and the checkpointer restarts the fresh session ≤ 1 s after enable(). Harmless — the generation guard keeps it to one restart — but one reconnect nobody asked for. A sessionRestarts.clear() under the lock at the top of enable() closes it.

suggestion (non-blocking): the backoff wait and its two wakes are reached by no test

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3784-3792, :4524, :2504

No case disables or shuts a domain down while a restart sits in its backoff, so Thread.sleep(left) in place of the monitor wait survives by construction. A case which asks for a restart with failNextSessionRestarts(1), calls domain.disable() during the backoff and bounds its duration well under MAX_REPLAY_RETRY_DELAY_IN_MS would pin both the wait and the wake. If that is out of scope, say so in the mutant table.

suggestion (non-blocking): pin the give-back level, and say what giveBack() takes

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/SessionRestartRequests.java:88-92, LDAPReplicationDomain.java:3637

@param taken what take() returned to the caller which could not run it — the one caller passes the constant AFTER_BACKOFF, and the class javadoc's "must not spend the wait another thread's request was made with" does not say which thread is which. The unit test gives back the level it took, so a NOW take given back as AFTER_BACKOFF is pinned nowhere:

@Test
public void aRestartGivenBackAfterItThrewIsOwedTheBackoff()
{
  final SessionRestartRequests requests = new SessionRestartRequests();
  requests.request(NOW);
  assertEquals(requests.take(), NOW);
  requests.giveBack(AFTER_BACKOFF);
  assertEquals(requests.take(), AFTER_BACKOFF,
      "a session which could not be started is what the wait exists for");
}

This pins the merge; the production constant stays pinned only through the backoff case above.

…y its restart waits out a total update, and wake its backoff Review round 1 of OpenIdentityPlatform#981. The catch of the state checkpointer was entered by no test: the one injected failure was spent by the replay thread's restart, and the checkpointer's ran. aSessionRestartWhichCouldNotRunIsRunAgain now fails two restarts, and asserts that the checkpointer reports the one which threw on it exactly once before the change is delivered; a catch which rethrows is red. The report inside that catch is guarded the way the give-back's report in replay() is: on the road out of a JVM which has refused an allocation, building the line is a throw of its own, and it would end the thread shutdown() waits for. The ieRunning() gate of runPendingSessionRestart() is said out loud. A restart stops the session a total update runs over, in either direction: an import into this replica ends on the entries which had arrived - and ownsItsSession() does not see an online total update at all, since preBackendImport() keeps this domain's own backend events from disabling it - and an export is given up by exportLDIFEntry() on the broker stop. The checkpointer can wait: the request stands until the context is released. enable() clears the holder as disable() does. A request made between a replay thread's read of the flag and disable()'s clear survived the disabled span, and the checkpointer restarted the session enable() had just started, within the second, for a change gone with the pending changes. giveBack() takes what the caller asks for again, not what take() returned, and says so; a NOW take given back as AFTER_BACKOFF is pinned. SessionRestartBackoffTest pins the monitor wait and both its wakes on a domain of its own: shutdown() during the backoff returns in a fraction of it, and after disable() / enable() the checkpointer saves a change within its next tick. Each wake is caught by its own case.

vharseko · 2026-09-15T07:32:58Z

Round 1 is 916afd5, one commit on top of the two: 381 insertions and 15 deletions over five files, SessionRestartBackoffTest the new one. The description is updated - the two new bullets in "What changed", the tests and mutant tables, and a closing section per item.

Blocking 1 - the checkpointer's catch pinned by no test. As measured: the one injected failure was the replay thread's, and the checkpointer's restart ran. aSessionRestartWhichCouldNotRunIsRunAgain now fails two - failNextSessionRestarts(2) - captures the error log over the delivery with errorLogRecordsOf(), and asserts that ERR_REPLAY_SESSION_RESTART_FAILED is reported exactly once before the change is delivered; the redelivery itself says the checkpointer ran the request again after reporting. The catch → rethrow mutant is red - the change is never delivered, and the test JVM then hangs in its cleanup exactly the way you described, shutdown() spinning on done for a checkpointer which is gone. The two mutants of the original table were re-run on the changed test and are still red. One thing said in the assertion's comment rather than left implicit: the count of one assumes the replay thread runs its own request, which it does unless the checkpointer's tick lands in the instants between request() and the CAS; a count of two there would be that, not a double report.

Blocking 2 - the ieRunning() gate. Deliberate, and it now says so in the javadoc of runPendingSessionRestart() and in the description. Both directions are in it, and the import one is worth stating precisely, since ownsItsSession() covers less than a total update into this replica: it is shutdown || disabled, and disabled is set by the disable() road - an offline import, a restore, a backend going away - not by an online total update, which preBackendImport() keeps from disabling the domain (ignoreBackendInitializationEvent). So on the checkpointer road ieRunning() is the only thing which sees an import coming in, and without it the checkpointer would stop the session the import reads its entries from. The export direction is what you named: exportLDIFEntry() sees the broker stop as ERR_INIT_RS_DISCONNECTION_DURING_EXPORT and gives the export up, and the replica being initialized is left to be initialized again. The checkpointer is the one road which can afford to wait - the request stands, it ticks once a second, and the restart runs as soon as the context is released - while the two synchronous roads cannot leave a request standing on their own, since nothing but the checkpointer would run it; gating them is only safe on top of this change. That follow-up is #1048. The import direction of the same two roads is #956 (#968), which refuses the restart through sessionHasAnOwner().

The unguarded report. Guarded, in the shape of the give-back's report in replay(). #977 catches only RuntimeException around state.save() and bounds the join, so an Error out of the report would still end the checkpointer there; the guard belongs here either way, as you said.

The holder cleared in enable(). Done, under the lock, first thing in enable(), with the window it closes in the comment: abandonReplay() reads the flag, logs, then asks. enable() is always paired with a disable() - import and restore begin/end, backend finalization and initialization - so a request standing there is always for a change gone with the pending changes.

The backoff wait and its wakes. Pinned rather than declared out of scope: SessionRestartBackoffTest, two cases on a replication server and a domain of their own on o=test, after the pattern of SessionRestartTest. Both put the checkpointer into the three-second backoff of a third restart in a row (two injected failures), which is the wait a test can see: shutdown() waits for that thread, and it is what saves the ServerState. The shutdown() case times shutdown() during the backoff (bound 1.5 s); the disable() case disables and enables the domain during it, makes a change of the replica's own, and waits for the persisted ds-sync-state to cover its CSN (bound 2.5 s) - nothing but the checkpointer writes that state while the domain is up. Under Thread.sleep() in place of the wait both are red, shutdown() at 2812 ms; with the wake removed from disable() alone only the disable() case is red, with it removed from shutdown() alone only the shutdown() case (2776 ms). The test acts by the clock - a fifth of a second into the backoff, after the checkpointer's wait(1000) - and the failure mode of a slow machine is stated in the class: it acts before the backoff begins, proves less, and reports nothing false.

The give-back level. giveBack() takes what the caller asks for again, and its javadoc says why that is not what take() returned; the class javadoc now says which thread is owed the wait and which is not, rather than "another thread's request". aRestartGivenBackAfterItThrewIsOwedTheBackoff pins the merge for the production constant.

Run on the new head, one JVM per class: UpdateOperationTest 32/32, RemotePendingChangesTest 21/21, AssuredReplicationPluginTest 14/14, DependencyTest 3/3, DisabledDomainServerStateTest 2/2, SessionRestartTest 2/2, LDAPReplicationDomainConfigChangeTest 9/9, SessionRestartBackoffTest 2/2, SessionRestartRequestsTest 7/7 - the last three classes of master are the ones which disable, enable or restart a domain, run for the enable() change.

…y its restart waits out a total update, and wake its backoff Review round 1 of OpenIdentityPlatform#981. The catch of the state checkpointer was entered by no test: the one injected failure was spent by the replay thread's restart, and the checkpointer's ran. aSessionRestartWhichCouldNotRunIsRunAgain now fails two restarts, and asserts that the checkpointer reports the one which threw on it exactly once before the change is delivered; a catch which rethrows is red. The report inside that catch is guarded the way the give-back's report in replay() is: on the road out of a JVM which has refused an allocation, building the line is a throw of its own, and it would end the thread shutdown() waits for. The ieRunning() gate of runPendingSessionRestart() is said out loud. A restart stops the session a total update runs over, in either direction: an import into this replica ends on the entries which had arrived - and ownsItsSession() does not see an online total update at all, since preBackendImport() keeps this domain's own backend events from disabling it - and an export is given up by exportLDIFEntry() on the broker stop. The checkpointer can wait: the request stands until the context is released. enable() clears the holder as disable() does. A request made between a replay thread's read of the flag and disable()'s clear survived the disabled span, and the checkpointer restarted the session enable() had just started, within the second, for a change gone with the pending changes. giveBack() takes what the caller asks for again, not what take() returned, and says so; a NOW take given back as AFTER_BACKOFF is pinned. SessionRestartBackoffTest pins the monitor wait and both its wakes on a domain of its own: shutdown() during the backoff returns in a fraction of it, and after disable() / enable() the checkpointer saves a change within its next tick. Each wake is caught by its own case.

vharseko · 2026-09-15T07:54:04Z

Force-pushed: rebased onto master at eef0757 (was cebef54), where #968 (#956) and #977 (#952) landed. The commits to review are 1be75f6, aea9b4e and c2455f8 - the round-1 commit is byte for byte what it was; range-diff shows a change in the first commit only.

Git reported nothing, and the merge did not compile - which is what every CI leg of the previous head failed on, in Build with Maven, since a leg builds refs/pull/981/merge:

LDAPReplicationDomain.java:[5425,8] error: cannot find symbol
  symbol:   variable sessionRestartRequested

#968 resets the restart request at the end of importBackend(), next to the pending changes and the backoff, and the first commit of this branch replaces that field with the holder; different parts of the file, so git kept both sides. Fixed where the field goes, in that first commit: sessionRestarts.clear(), as disable() and enable() do on the same road. The one other line the range-diff shows is a comment: runPendingSessionRestart() was "run outside the block above", and #977 moved the save it referred to out of that block too, so it now says "outside the monitor above, as the save is".

The two landings met this branch elsewhere as well, without a hunk between them:

[#952] Keep a failed state write from killing the checkpointer and hanging the shutdown #977 takes state.save() out of the checkpointer's monitor into saveState() and bounds the join of shutdown(); this branch runs runPendingSessionRestart() right after that save, outside the same monitor. saveState() catches RuntimeException, so the guard on the report stays. The catch → rethrow mutant was re-run on this head: red as before, and the test JVM now ends by itself - [#952] Keep a failed state write from killing the checkpointer and hanging the shutdown #977's join() returns at once for a thread which has ended - where before it spun on done; the row of the mutant table says so.
[#956] Tell a failed entryUUID search apart from an entry which is not there #968's sessionHasAnOwner() is what the three roads refuse the restart on, and the request this branch makes sits after that refusal on each, as the flag did. The checkpointer's gate is ieRunning(), which covers importInProgress() and the export besides, so it did not move.

Ordinals: #968 took 322, #977 took 323 and 324, this branch keeps 325; the file has no ordinal twice.

Run on the new head, one JVM per class, all green: UpdateOperationTest 32/32, ReplayDuringImportTest 3/3 and NamingConflictTest 18/18 (#968's, the first of them covering the import-end reset), ServerStateFlushTest 4/4 (#977's), RemotePendingChangesTest 21/21, AssuredReplicationPluginTest 14/14, DependencyTest 3/3, DisabledDomainServerStateTest 2/2, SessionRestartTest 2/2, LDAPReplicationDomainConfigChangeTest 9/9, SessionRestartBackoffTest 2/2, SessionRestartRequestsTest 7/7.

The description is updated: the SHAs, the base, the mutant row, and a closing section on the two landings.

maximthomas

praise: The rebase and the round-1 fixes hold up to measurement, not just to reading.

The rebase is byte for byte: the -U0 line sets of the three commits match the previous head, the only residue being the importBackend() clear (sessionRestarts.clear(), LDAPReplicationDomain.java:5474) and one comment.
The round-1 blocking issue is closed and pinned: with failNextSessionRestarts(2) the second failure is the checkpointer's, and the catch→rethrow mutant in runPendingSessionRestart() dies at the 60 s delivery assert (UpdateOperationTest.java:2421; the checkpointer dead with msgID 140, the JVM exiting through #977's join).
SessionRestartBackoffTest's shutdown case pins the in-monitor sleep mutant: "shutdown() waited 2783 ms" here, 2812 ms in your run.
The guarded 325 report, the clear in enable() under the lock, the wakes ordered after the flags in both disable() and shutdown(), and the wait outside serviceStateLock all read correct; the owned == null routing at :3718 makes the count of one in UpdateOperationTest the only road for the injected failures.
Ordinal 325 is unique against origin/master; all five Linux cells and CodeQL are green.

issue (blocking): The disable case does not time disable(), so a checkpointer which sits out the backoff while holding the monitor leaves it green.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/SessionRestartBackoffTest.java:147-148, :166-174; opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:4824, :4830-4832

The wake is the last thing disable() does, and it is a synchronized (sessionRestartBackoff) block. With Thread.sleep(left) in place of sessionRestartBackoff.wait(left) at :3990 the sleeping checkpointer holds that monitor, disable() blocks at :4830 for the rest of the backoff and returns only once the checkpointer is out of it; enable(), the modify and the 2.5 s clock all start after the stall, the save lands a tick later, and the case is green — measured 1/1 (9.4 s against 8.3 s at head), while the shutdown case goes red on the same mutant. The table's "Thread.sleep() in place of the wait: both are red" holds only for a sleep outside the monitor; the description does not say which shape was run. Time disable() the way the shutdown case times shutdown():

final long started = System.nanoTime();
domain.disable();
final long tookMs = NANOSECONDS.toMillis(System.nanoTime() - started);
assertTrue(tookMs < SHUTDOWN_BOUND_IN_MS, "disable() waited " + tookMs
    + " ms: it sat out the backoff of the session restart the state checkpointer"
    + " was waiting through, for a session disable() was cutting anyway");
domain.enable();

(SHUTDOWN_BOUND_IN_MS then bounds both wakes; rename it.) With this the case is red under either sleep shape. Please also name the shape in the table.

issue (non-blocking): UpdateOperationTest asserts the 325 report exactly once while its own comment names the outcome of two as legitimate.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:2427-2436

If the checkpointer's tick lands between the replay thread's request() (LDAPReplicationDomain.java:3763) and its CAS (:3780), the checkpointer spends both injected failures and logs 325 twice; the case is then red on the road the comment describes. The window is sub-microsecond, so this is a rare flake, not a wrong pin — a count of 0 has no road (replayFailed(csn) at :3718 nulls the owner, so replay()'s catch never reports), and the count still pins the report.

final int reported = countRecordsOf(records,
    "Could not restart the replication session of domain \"" + baseDN + "\"");
assertTrue(reported == 1 || reported == 2, "the state checkpointer reports the restart"
    + " which threw on it once, or twice when its tick took the replay thread's request"
    + " as well: " + reported + " in " + records);

issue (non-blocking): The wake in disable() is not sticky, and the test's back-to-back enable() can put the checkpointer back into the backoff.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3984-3991, :4800, :4824, :4987; opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/SessionRestartBackoffTest.java:147-148

The wake is flag-then-notify. A checkpointer descheduled between notifyAll() and its loop re-check for longer than enable() takes to reach disabled = false (:4987, after the clear and loadDataState()) re-evaluates !disabled as true and waits the rest of the backoff (~2.6 s in the test); the save then lands ~3.6 s after disable(), past CHECKPOINT_BOUND_IN_MS — the false red the class javadoc says the case never reports. In production the cost is latency only (block 2 returns on the generation). Not seen in 1/1 here or 2/2 in your runs; the mechanism is certain from the code. A wake generation makes it sticky:

/** Bumped under {@link #sessionRestartBackoff} on every wake, so that a wake is never missed. */
private long sessionRestartBackoffWakes;

private void wakeSessionRestartBackoff()
{
  synchronized (sessionRestartBackoff)
  {
    sessionRestartBackoffWakes++;
    sessionRestartBackoff.notifyAll();
  }
}

// in waitBeforeSessionRestart()
synchronized (sessionRestartBackoff)
{
  final long wakes = sessionRestartBackoffWakes;
  for (long left = until - monotonicNowInMs();
       left > 0 && !shutdown.get() && !disabled && wakes == sessionRestartBackoffWakes;
       left = until - monotonicNowInMs())
  {
    sessionRestartBackoff.wait(left);
  }
}

issue (non-blocking): The two holder clears outside the checkpointer road — enable() and importBackend() — are pinned by nothing.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:4960, :5474

The disable case runs disable() (which clears at :4819) before enable(), and UpdateOperationTest never disables, so deleting :4960 survives by construction, as the description says; :5474 is reached by no test of this PR and has no table row. A pin needs a request standing while the domain is disabled, which the existing hooks can observe once a request hook exists (the precedent of failNextSessionRestarts):

domain.disable();
domain.requestSessionRestart();   // @VisibleForTesting: sessionRestarts.request(SessionRestart.NOW)
domain.failNextSessionRestarts(1);
domain.enable();
Thread.sleep(2000);               // two checkpointer ticks
assertEquals(domain.getSessionRestartFailuresLeft(), 1,
    "the request which stood while the domain was disabled was run against the session enable() started");
assertTrue(domain.isConnected());

With :4960 deleted the checkpointer runs the leftover request within the second, spends the injected failure and leaves the session down. Or: record both clears as unpinned in the table — and say whether #968's tests reach :5474.

issue (non-blocking): The giveBack(AFTER_BACKOFF) call site is pinned by the holder's unit case only; giveBack(restart) survives every case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3804; opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/SessionRestartRequestsTest.java:86-96

aRestartGivenBackAfterItThrewIsOwedTheBackoff pins the merge inside the holder; on the road both integration cases take the level taken is already AFTER_BACKOFF, so giving the taken level back is the same thing. Only a NOW take whose restart throws — a replay abandoned past its give-up budget, abandonReplay() — tells the two apart, and no case reaches it (the description says so). Either a case on that road asserting isConnected() is still false 1.2 s after the throw, as leaveTheCheckpointerInTheBackoff does, or record the call site as unpinned in the table.

suggestion (non-blocking): startDomain() asserts before the caller's assignment, and the finally runs its three steps unchained — a failed setup or a throwing release() leaks the domain or the RS into the next case.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/SessionRestartBackoffTest.java:105, :141, :178-184, :268-277

createNewDomain() + start() + assertTrue(isConnected()) all run before domain = is assigned; a failed assert leaves a started, registered domain that finally never deletes (domain == null; createNewDomain() replaces it silently in the map). Assign before the assert, as SessionRestartTest does, and chain the cleanup:

domain = startDomain(baseDN, rsPort);   // startDomain(): create + start(), no assert
assertTrue(domain.isConnected(), "the domain did not connect to its replication server");
...
finally
{
  try
  {
    release(domain, broker);
  }
  finally
  {
    try
    {
      if (domain != null)
      {
        MultimasterReplication.deleteDomain(baseDN);
      }
    }
    finally
    {
      remove(replicationServer);
    }
  }
}

question (non-blocking): Were the wake-removal rows of the mutant table run at this head?

The two rows ("removed from disable() alone: the disable case red"; "from shutdown() alone: the shutdown case red") were not run this round; the three runs went to the head, the rethrow mutant and the sleep mutant. If they were run, a word on it in the description is enough.

nitpick (non-blocking): "A restart which throws is given back and run by the checkpointer instead of going out with the thread" is not true on the OOME/last-resort road of replay()'s catch, and the comment there is now stale.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2731-2735; the PR description

On that road (owned != null, recoverFromReplayFailure(owned, replayThreadShutdown, true)) the thread still re-runs the merged request itself after 317, as at base — what changed is that the re-run now carries the backoff. So the comment "the session is restarted without sitting through the backoff, since the thread which is doing it is on its way out" is false at this head, and the description's sentence needs one clause: "except the OOME road of replay(), which still restarts in-thread, now with the backoff". 317's text ("the session is being restarted so that the change is delivered again") names a restart the give-back's success does not imply; ride-along if touched.

… has run The recovery from a failed replay takes the request before it runs the restart, and restartSession() stops the session in its first synchronized block and starts it again in the second. Anything thrown in between left the request cleared and the session stopped, and nothing asked for it again: no change is delivered over a session which is down, so no replay fails and no thread comes back to the recovery. The domain sat out of the topology - the change it could not replay still owned by the replication server, its ServerState stopped behind it - until the server was restarted or a configuration change happened to restart the service. The throw is narrow but not theoretical: enableService() is the one call on that road nothing guards, while the reconnect loop of the broker wraps the same connectAsDataServer() in a catch, so every failure that path is written to survive is fatal to this one. The request is a SessionRestartRequests now. It is still taken before the restart runs, as it must be - a change released while a restart is under way is not one that restart asks for - and it is given back when the restart could not run, with the backoff whatever it was asked for with, since a session which can not be started is what that wait exists for. The wait belongs to the request rather than to the thread which runs it: a request made by a replay thread on its way out is owed none, one made by a failing replay is owed one, and either thread can end up running the other's request. Nothing else runs a request which is left standing, so the state checkpointer of the domain does, once a second: it is the one thread a domain has for as long as it is up, whatever its session is doing. That also lets abandonReplay() ask for the restart rather than run it - the threads of the pool are stopped one after the other and joined, so a live num-update-replay-threads change cost one stop and reconnect per thread which was replaying a change, none of them waiting, while the configuration change waited for all of them - and it runs the bare hand-back of issue OpenIdentityPlatform#922, which asked for a restart nothing was going to run. The backoff is waited on a monitor rather than slept through, and a domain which is going away or is being disabled wakes it: the thread which holds it is a replay thread the shutdown of the pool joins, or the checkpointer the shutdown of the domain waits for.

…nothing else claims

…y its restart waits out a total update, and wake its backoff Review round 1 of OpenIdentityPlatform#981. The catch of the state checkpointer was entered by no test: the one injected failure was spent by the replay thread's restart, and the checkpointer's ran. aSessionRestartWhichCouldNotRunIsRunAgain now fails two restarts, and asserts that the checkpointer reports the one which threw on it exactly once before the change is delivered; a catch which rethrows is red. The report inside that catch is guarded the way the give-back's report in replay() is: on the road out of a JVM which has refused an allocation, building the line is a throw of its own, and it would end the thread shutdown() waits for. The ieRunning() gate of runPendingSessionRestart() is said out loud. A restart stops the session a total update runs over, in either direction: an import into this replica ends on the entries which had arrived - and ownsItsSession() does not see an online total update at all, since preBackendImport() keeps this domain's own backend events from disabling it - and an export is given up by exportLDIFEntry() on the broker stop. The checkpointer can wait: the request stands until the context is released. enable() clears the holder as disable() does. A request made between a replay thread's read of the flag and disable()'s clear survived the disabled span, and the checkpointer restarted the session enable() had just started, within the second, for a change gone with the pending changes. giveBack() takes what the caller asks for again, not what take() returned, and says so; a NOW take given back as AFTER_BACKOFF is pinned. SessionRestartBackoffTest pins the monitor wait and both its wakes on a domain of its own: shutdown() during the backoff returns in a fraction of it, and after disable() / enable() the checkpointer saves a change within its next tick. Each wake is caught by its own case.

@VisibleForTesting

…count the wakes, and pin the clears and the give-back level Review round 2. - SessionRestartBackoffTest times disable() as it times shutdown(), against the one bound both wakes share: a checkpointer which sleeps the backoff through while it holds the monitor holds disable() for the rest of it, and the disable case was green under that shape of the sleep mutant. Both shapes are in the mutant table now. - The wake of the backoff is counted as well as flagged, and the count is read where the session is stopped, under serviceStateLock: `disabled` does not stay set, so a checkpointer descheduled between disable()'s notifyAll() and its own re-check for as long as enable() takes would wait out the rest of the backoff for a session it has nothing left to start. - aSessionRestartWhichCouldNotRunIsRunAgain accepts the checkpointer's report twice, on the road its own comment names: its tick landing between the replay thread's request and the CAS. - requestSessionRestart() (@VisibleForTesting) leaves a request standing, and two cases pin the clears in enable() and importBackend() on it - the second in OpenIdentityPlatform#968's ReplayDuringImportTest, which has the import fixture. - aRestartAskedForWithoutTheBackoffIsGivenBackWithIt pins the level a restart which could not run is given back with, on the abandonReplay() road OpenIdentityPlatform#941's fixture reaches: a NOW request given back as taken has the session back a second early. - startDomain() asserts nothing, so that a case which fails on its way up still holds the domain to delete, and the cleanup chains its three steps. - The comment on the OOME road of replay() and ERR_REPLAY_GIVE_BACK_FAILED say what happens: the restart is asked for, and the last resort re-runs a given-back request on the same thread, backoff and all.

vharseko · 2026-09-15T18:07:26Z

Round 2 is c659a42, one commit on top of the three: 374 insertions and 39 deletions over five files, ReplayDuringImportTest the one this PR had not touched before. The four commits against master are 1214 insertions and 73 deletions over seven files. The description is updated - the tests and mutant tables, the two bullets the round touched, the "say plainly" paragraphs, and a closing section per item.

Blocking - the disable case does not time disable(). As measured. The row of the table said "both cases red" for a sleep in place of the wait, and the shape it was run in was the sleep outside the monitor - the whole synchronized loop replaced - which the row itself gives away: "the checkpoint did not come within 2500 ms" is the outcome of a checkpointer which sleeps through enable(), and a checkpointer which sleeps inside the monitor holds disable() instead and then saves inside the bound, as you found. The case now times disable() the way the shutdown case times shutdown(), against the one bound both wakes share (WAKE_BOUND_IN_MS), and both shapes are in the table as rows of their own: inside the monitor, the disable case is red on disable() (2804 ms) and the shutdown case on shutdown() (2764 ms); outside it, the disable case is red on the checkpoint and the shutdown case on shutdown() (2786 ms).

The count of 325. reported == 1 || reported == 2, with the comment saying which road gives two and why none has no road.

The wake is not sticky. Counted, as you sketched, with one difference in where the count is read: restartSession() takes it in its first block, under serviceStateLock and before it lets go of the session it stopped, rather than at the entry of the wait. Read at the entry, the same window sits between that block and the wait: a disable() and an enable() which both land there leave the count already bumped when it is read, and the wait goes on for the whole backoff as before. disable() takes serviceStateLock before it wakes, so a wake which has yet to be given when the block lets go is one the wait sees, and one given before it is a disable() the block saw as the owner of the session. The session generation would have said the same and is not read: #974 keeps it under the lock, and the wait is deliberately outside it. shutdown() wakes before it takes the lock, and needs no count - its flag never comes back. This has no pin: the window is the checkpointer being descheduled for the length of enable() between notifyAll() and its re-check, which no test can hit on purpose; the row in the table says so, and the mutant which drops the count from the condition is green under every case, as expected.

The two holder clears. Pinned, both, on a requestSessionRestart() hook - the third @VisibleForTesting in the class, and said so in the "say plainly" paragraph. The enable() case is the third of SessionRestartBackoffTest, as you sketched it; the importBackend() case sits in #968's ReplayDuringImportTest, which has the import fixture: a request made while the import streams, and a failure to inject, and at the end of the import the failure is still there to spend and the session is up. Under the mutant which deletes the clear the checkpointer runs the request on its first tick after the import lets the session go, spends the failure and leaves the session down. And a correction to what I wrote in the rebase note: #968's first case runs through the import-end reset, it does not pin it - during an import abandonReplay() refuses on sessionHasAnOwner(), so nothing stands there to be cleared, and deleting the line survives that class. It does not survive the new case.

The give-back level. Pinned on the road you named: aRestartAskedForWithoutTheBackoffIsGivenBackWithIt parks a replayed delete with #941's fixture, changes the number of replay threads so that the thread which holds it hands it back on its way out and asks for the restart without the backoff, and fails the restart the checkpointer runs for it. Then it looks at the session 1.5 s after the throw: the request given back with the backoff has the checkpointer take it on its next tick and wait a second, so the session is back two seconds after the throw; given back as taken, it is back on that tick, one second after. The giveBack(restart) mutant is red on it ("the session was started back within 1500 ms of the restart which threw"), and green on aSessionRestartWhichCouldNotRunIsRunAgain and on SessionRestartBackoffTest, as you said it would be. It is also the first pin of the collapsed restarts of a thread-count change, which the description said had none: one abandoned change, one restart, run by the checkpointer rather than by the thread which handed the change back. "One rather than one per thread" is still unpinned - that needs as many parked threads as changes - and the paragraph now says which half is pinned.

startDomain() and the finally. Assigned before the assert, as in SessionRestartTest, and the cleanup is one release() with the three steps chained in nested finally blocks.

The wake-removal rows. They were not run at c2455f8: they were run on 916afd5, the head of round 1 before the rebase, whose round-1 commit the rebase left byte for byte and whose first commit it changed by the importBackend() clear and one comment - so what those rows measured is this code, but the description did not say when they were measured. Every row of the table is re-run at this head, and the table says so.

The OOME road. As you read it: the first run of the restart on that road is without the backoff, and one which throws is given back with it and re-run by the last resort of replay() on the same thread, backoff and all - only one which throws there as well is left to the checkpointer. The comment at the call says that now, the description's sentence has its clause, and 317 says a restart is asked for rather than under way.

Run on c659a42, one JVM per class, all green: the four classes this round touches - UpdateOperationTest 33/33, SessionRestartBackoffTest 3/3, ReplayDuringImportTest 4/4, SessionRestartRequestsTest 7/7 - after the rebase, and on bc6835e, the same four commits before it: those four again, RemotePendingChangesTest 21/21, AssuredReplicationPluginTest 14/14, DependencyTest 3/3, DisabledDomainServerStateTest 2/2, SessionRestartTest 2/2, LDAPReplicationDomainConfigChangeTest 9/9, ServerStateFlushTest 4/4, NamingConflictTest 18/18, and every row of the mutant table, twelve runs.

Rebased onto master at 600df92 in the same push, where #978, #979, #1047 and #1033 landed. Nothing met this branch - #978 is the one which touches LDAPReplicationDomain, in the constructor and in publishReplicaOfflineMsg() - and range-diff shows all four commits moved as they were. 325 is still claimed by nothing on master; the file has no ordinal twice.

vharseko requested a review from maximthomas September 9, 2026 06:19

vharseko added bug replication concurrency Thread-safety / race-condition bugs java tests Test suites: fixing, enabling, un-disabling and removed java labels Sep 9, 2026

vharseko force-pushed the issues/925-session-restart-request branch from 58df624 to 8f9a7fd Compare September 9, 2026 09:39

vharseko requested review from maximthomas and removed request for maximthomas September 9, 2026 09:42

vharseko mentioned this pull request Sep 9, 2026

[#986] Give back the changes a replay thread the pool stopped had parked #988

Open

vharseko requested review from maximthomas and removed request for maximthomas September 9, 2026 11:02

vharseko force-pushed the issues/925-session-restart-request branch from 31d7dfa to 0e91a8f Compare September 11, 2026 05:35

vharseko requested review from maximthomas and removed request for maximthomas September 11, 2026 05:35

vharseko mentioned this pull request Sep 11, 2026

[#1014] Report a ReplicaOfflineMsg as forwarded only to a peer which can decode it #1019

Open

vharseko force-pushed the issues/925-session-restart-request branch from 0e91a8f to ca9ba89 Compare September 11, 2026 18:35

vharseko force-pushed the issues/925-session-restart-request branch from ca9ba89 to 823980c Compare September 12, 2026 13:19

This was referenced Sep 13, 2026

The #958 hand-out tests assert which thread replays a parked change, which getNextUpdate() does not guarantee #1036

Open

[#1036] Assert that a parked change is handed out, not which thread hands it out #1037

Open

vharseko force-pushed the issues/925-session-restart-request branch from 823980c to 3592ab5 Compare September 14, 2026 07:36

vharseko requested review from maximthomas and removed request for maximthomas September 14, 2026 07:42

vharseko force-pushed the issues/925-session-restart-request branch from 3592ab5 to 2a2766f Compare September 14, 2026 09:54

maximthomas requested changes Sep 15, 2026

View reviewed changes

vharseko mentioned this pull request Sep 15, 2026

Replication: a session restart run by the thread which released a change cuts the export the session carries #1048

Open

vharseko requested a review from maximthomas September 15, 2026 07:33

vharseko force-pushed the issues/925-session-restart-request branch from 916afd5 to c2455f8 Compare September 15, 2026 07:53

maximthomas requested changes Sep 15, 2026

View reviewed changes

vharseko added 4 commits September 15, 2026 20:59

[OpenIdentityPlatform#925] Take an ordinal in replication.properties …

df61f75

…nothing else claims

vharseko force-pushed the issues/925-session-restart-request branch from c2455f8 to c659a42 Compare September 15, 2026 18:06

vharseko requested a review from maximthomas September 15, 2026 18:07

Navigation Menu

Uh oh!

Uh oh!

[#925] Keep asking for a session restart until it has run - #981

[#925] Keep asking for a session restart until it has run#981
vharseko wants to merge 4 commits into
OpenIdentityPlatform:masterfrom
vharseko:issues/925-session-restart-request

vharseko commented Sep 9, 2026 •

edited

Loading

Uh oh!

vharseko commented Sep 9, 2026

Uh oh!

vharseko commented Sep 9, 2026

Uh oh!

vharseko commented Sep 11, 2026

Uh oh!

vharseko commented Sep 11, 2026

Uh oh!

vharseko commented Sep 12, 2026

Uh oh!

vharseko commented Sep 14, 2026

Uh oh!

vharseko commented Sep 14, 2026

Uh oh!

maximthomas left a comment

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

maximthomas left a comment

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

vharseko commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

What changed

Tests

Rebased onto #958 as it went, and onto master once it landed

Review round 1

Rebased onto master at eef0757, where #968 (#956) and #977 (#952) landed

Review round 2

Uh oh!

vharseko commented Sep 9, 2026

Uh oh!

vharseko commented Sep 9, 2026

Uh oh!

vharseko commented Sep 11, 2026

Uh oh!

vharseko commented Sep 11, 2026

Uh oh!

vharseko commented Sep 12, 2026

Uh oh!

vharseko commented Sep 14, 2026

Uh oh!

vharseko commented Sep 14, 2026

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

Uh oh!

vharseko commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vharseko commented Sep 9, 2026 •

edited

Loading

Rebased onto #958 as it went, and onto `master` once it landed

Rebased onto `master` at `eef0757`, where #968 (#956) and #977 (#952) landed