[#1061] Ask for the session restart under an owner as well, and leave it to the state checkpointer

[vharseko]

Fixes #1061.

On master. #985's [#954] commit went into master with the squash of #988 (80481f756d), so the branch is that master and the one [#1061] commit, 25f1034981, which is the diff to review. The third of the three roads this closes, giveBackParkedChanges(), is #954's, and it is on master now - with the #986 paragraph of its javadoc, on the thread which is stopping, next to this change's paragraph on the owner. On this head: ReplayDuringImportTest 9/9, UpdateOperationTest 41/41, ParkedChangeGiveBackTest 3/3, Skipped: 0.

What was wrong

recoverFromReplayFailure(), abandonReplay() and - with #954 - giveBackParkedChanges() release the change they hand back and, while the session has an owner (sessionHasAnOwner(): the domain shutting down or disabled, or a total update into this replica from the moment it is asked for), asked for no session restart. Two of the three owners forget the pending changes and every request with them - disable() / enable() and the import's finally clear both - so nothing was lost there. The third does not: a total update which is asked for and never begins - initializeFromRemote() refused at publish, an ErrorMsg from the exporter, the two-minute watchdog of InitializeTask (abortStalledInitializeFromRemote()), or the second attempt initialize() makes after a failed import, which re-publishes the request and keeps the context - ends in releaseIEContext() alone, and a change released under it stayed listed, uncommitted and owned by nobody until the next failed replay of this domain restarted the session. On a domain which then goes quiet that is for good: commit() moves the ServerState no further than the oldest uncommitted change, so the state in memory, and the one persisted from it, stop at the change - the shape of #954, one road over.

#968 left it that way on purpose: the request was a flag which only the thread of a failed replay ran, so a flag set under an owner was served by the next failed replay, which would have set it itself. #981 changed that premise - the state checkpointer runs a request nobody ran, and holds it for as long as ieRunning() - and the last resort of replay() relies on it already: its request is made under an owner as well, and only its run is gated. Found in the fourth review of #985, filed as #1061.

What changed

• The three roads ask on every road, as one policy. The request is made once the change is released and before the owner is read - remotePendingChanges.replayFailed(csn) / releaseParkedChangesOwnedByCurrentThread(), then sessionRestarts.request(…), then sessionHasAnOwner() - and the owner arm returns with it standing. What is asked for is what was asked for before: NOW on a thread which is stopping or which an OutOfMemoryError is ending, AFTER_BACKOFF otherwise; abandonReplay() asks NOW as it did. The owners which forget the pending changes forget the request with them (disable() :5221, enable() :5401, the import's finally :5916); the one which does not leaves it to the checkpointer, which runs it within its tick of releaseIEContext() and has the replication server send the change again.
• The owner roads stay silent, and the give-back's count stays off them. WARN_REPLAY_RETRYING_CHANGE (through the throttle of Replication: the replay retry warning is logged once per delivery, with nothing bounding it once the give-up budget is raised #942), NOTE_REPLAY_ABANDONED_CHANGE and NOTE_REPLAY_PARKED_CHANGE_GIVEN_BACK are not logged under an owner, and the parked deliveries are not counted as processed, as before: a server which is shutting down abandons every change in flight, and none of them is delivered again before it is started back, so a line which says the replication server sends the change again would not hold on every owner road. On the never-begun road the change is now asked for again without a line saying so, and a parked delivery given back there is counted by the delivery which replays it; said here rather than papered over with a line of its own.
• The gates on the two runs of replay() become load-bearing - the parked road (:2826) and the last resort (:2910): a request now stands under every owner, and a run there would spend it on restartSession()'s refusal, where the request left standing is what the checkpointer runs. The javadoc of sessionHasAnOwner(), the comments of the three arms, of the parked road, of enable() (whose account of the disabled = false ordering rested on abandonReplay() asking for nothing) and of the test hook requestSessionRestart() say what holds now.

What this does not close. A no-owner thread which read no owner, made its request and reached runRequestedSessionRestarts() while the owner arrived spends the merged flag - its own and any request made under the owner in between - on restartSession()'s refusal. That window is the few statements between the read and the lock, the same class as the last resort's today, and it is not a regression (before this, nothing was asked for under the owner at all); closing it for good means restartSession() saying it refused and the caller giving the request back, which is a change to the loop of runRequestedSessionRestarts() and belongs to a follow-up. #1049 closes the import direction of it in passing - its runRequestedSessionRestarts() leaves the requests standing while ieRunning() - and #1045 turns the owner read of restartSession() into a claim. With #1049 in, the import part of the parked road's gate is no longer load-bearing (the request is not taken), and the mutant below which pins it would survive; whichever lands second re-checks.

Tests

All in ReplayDuringImportTest, on the one owner a test can hold open and then take away: initializeFromRemote() with the exporter holding the InitializeRequestMsg, and in the place of the answer abortStalledInitializeFromRemote(0) - the watchdog's road, and the one road out of an unanswered request a test can take at a time of its choosing (an ErrorMsg releases the context only through the task, which the test does not have; production always has one). Each change travels the replication server - published by the exporter's broker, taken off the domain's synchronous queue by the test - which is what has it to deliver again; a message handed to the domain, as the class's other cases do, is one the replication server never had.

case	road	result
aChangeReleasedUnderARequestWhichIsNeverAnsweredIsDeliveredAgain (new)	recoverFromReplayFailure() - the attempts in place spent on an entryUUID search which does not run	green; no retry warning, session up, !cover(csn); after the abort the change is delivered again within the checkpointer's tick + the 1 s backoff, replayed, applied, covered
aChangeAbandonedUnderARequestWhichIsNeverAnsweredIsDeliveredAgain (new)	abandonReplay() - replayed with the stopping flag set, abandoned at the top of its first attempt	green; not applied, no NOTE 309, session up; delivered again after the abort and covered
aParkedChangeGivenBackUnderARequestWhichIsNeverAnsweredIsDeliveredAgain (new)	giveBackParkedChanges() - the parked change waits for one another thread of the test holds before its operation is built (ModifyMsgWhoseOperationWaitsToBeBuilt, new), so nothing has failed and the give-back's request is the only one standing; the replay unwound is the OOME-on-ack fixture of #954	green; dependent-changes-size 1 → 0, held change committed when let go, parked change delivered again after the abort and covered
aParkedChangeGivenBackWhileTheRequestIsOnItsWayLeavesTheSessionToTheOwner (renamed from …IsNotAskedForAgain, assertions unchanged)	the give-back's owner arm: not counted, not reported, session left to the import which then runs to its end	green
the class's other five cases		green, 9/9

The three new cases were run first against the class as it stands on #985's head (git show HEAD:…LDAPReplicationDomain.java compiled ahead of target/classes): 3/3 red, each on the wait for the redelivery - "was not delivered again once the request gave up … within 30000 ms".

Mutants, each compiled in place of the class and run against the case meant to catch it:

mutant	result
recoverFromReplayFailure() asks only when the session has no owner (the request moved back below the owner arm)	red on the released case: "was not delivered again once the request gave up"
abandonReplay() asks only when the session has no owner	red on the abandoned case: "was not delivered again once the request gave up"
giveBackParkedChanges() asks only when the session has no owner	red on the parked case
the parked road's gate forgets the import: !shutdown.get() && !disabled in place of !sessionHasAnOwner() (:2826)	red on the parked case: the run under the owner spends the give-back's request on the refusal, nothing comes back

TestSynchronousReplayQueue is backed by a ConcurrentLinkedDeque rather than a LinkedList - the redelivery is offered by the listener thread of the domain and taken by the thread of the test - byte for byte the change #1049 makes to the same file, so the two merge in either order.

Run on this head, one JVM per class: ReplayDuringImportTest 9/9, ParkedChangeGiveBackTest 3/3 (as of #985's fifth round), SessionRestartBackoffTest 3/3, SessionRestartTest 2/2, DisabledDomainServerStateTest 2/2, RemotePendingChangesTest 24/24, SessionRestartRequestsTest 7/7, ServerStateFlushTest 4/4, LDAPReplicationDomainConfigChangeTest 9/9, DependencyTest 3/3, UpdateOperationTest 40/40, InitOnLineTest 10/10, GenerationIdTest 4/4, ReSyncTest 2/2, NamingConflictTest 21/21, AssuredReplicationPluginTest 14/14, ReplicationDomainTest 12/12 - the classes which restart a session, drive a total update or read the requests, 169 tests, Skipped: 0.

[vharseko]

@maximthomas restacked on master at 80481f756d: the squash of #988 took #985's [#954] commit in with it, so the [#954] this branch carried is gone and the branch is the one [#1061] commit, 25f1034981 - the diff to review is what it was. Nothing new is proposed here; this only says what the conflict was and how it was taken.

The conflict was in one place, and it was prose: the javadoc of giveBackParkedChanges(), where this change rewrote the paragraph on a domain whose session has an owner - the changes are released and asked for, and the request is left to the state checkpointer - and #986 added a paragraph on the thread which is stopping right under it. Both are there now, this change's first. The body of the method, ReplayDuringImportTest, ModifyMsgWhoseOperationWaitsToBeBuilt and TestSynchronousReplayQueue merged on their own. git range-diff against d047625c57 shows the code byte for byte the same.

Re-run after the restack, one JVM per class: ReplayDuringImportTest 9/9, UpdateOperationTest 41/41 - the cases of #954 and #986 included - ParkedChangeGiveBackTest 3/3. The four mutants are not re-measured on this head. The description says "on master" where it said "stacked on #985".