S16 - some transactional hardening
All checks were successful
Build, test and publish / verify (push) Successful in 7m32s
All checks were successful
Build, test and publish / verify (push) Successful in 7m32s
This commit is contained in:
@@ -27,6 +27,12 @@ public class Database {
|
||||
this.manager = manager;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs work in a new transaction, or joins the current JTA transaction. Callback registration is supported only
|
||||
* inside transactions started by this instance. Commit callbacks run in registration order after durable commit;
|
||||
* their exceptions propagate to the caller without rolling back committed work or running remaining callbacks.
|
||||
* Rollback callbacks run before the original failure is rethrown; a callback exception replaces that failure.
|
||||
*/
|
||||
public <T> T transaction(final Callable<T> work) throws Exception {
|
||||
try {
|
||||
if (manager.getStatus() != Status.STATUS_NO_TRANSACTION) {
|
||||
@@ -90,10 +96,12 @@ public class Database {
|
||||
}
|
||||
}
|
||||
|
||||
/** Registers a callback on this thread's transaction owned by this instance; see {@link #transaction(Callable)}. */
|
||||
public void afterCommit(final Runnable action) {
|
||||
committed.get().add(action);
|
||||
}
|
||||
|
||||
/** Registers a rollback callback under the same ownership requirement as {@link #afterCommit(Runnable)}. */
|
||||
public void afterRollback(final Runnable action) {
|
||||
rolledBack.get().add(action);
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ import nl.agilecc.agiledancers.model.scheduling.SchedulingResult;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingRun;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingState;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingSummary;
|
||||
import nl.agilecc.agiledancers.repository.StorageException;
|
||||
import nl.agilecc.agiledancers.service.port.SchedulingEngine;
|
||||
|
||||
/** Owns background tasks; the school monitor orders short commits and publication, never solver computation. */
|
||||
@@ -173,15 +174,42 @@ public class SchedulingRuns {
|
||||
|
||||
private void finish(final SchedulingRun completed) {
|
||||
try {
|
||||
work(() -> {
|
||||
save(completed);
|
||||
return null;
|
||||
});
|
||||
while (true) {
|
||||
try {
|
||||
work(() -> {
|
||||
schools.show(completed.schoolId(), completed.showId());
|
||||
save(completed);
|
||||
return null;
|
||||
});
|
||||
return;
|
||||
} catch (final StorageException unavailable) {
|
||||
// Keep the run guard and terminal outcome until storage recovers; never publish an uncommitted
|
||||
// status.
|
||||
if (executor.isShutdown() || !waitForStorage()) {
|
||||
return;
|
||||
}
|
||||
} catch (final ManagementException removed) {
|
||||
if (removed.getKind() != ManagementExceptionKind.NOT_FOUND) {
|
||||
throw removed;
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
schools.endScheduling(completed.showId());
|
||||
}
|
||||
}
|
||||
|
||||
private boolean waitForStorage() {
|
||||
try {
|
||||
Thread.sleep(1000);
|
||||
return true;
|
||||
} catch (final InterruptedException stopping) {
|
||||
Thread.currentThread().interrupt();
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private void save(final SchedulingRun run) {
|
||||
if (persistence == null) {
|
||||
publish(run);
|
||||
|
||||
@@ -136,6 +136,71 @@ class SchedulingRunsTest {
|
||||
schools.endScheduling(schools.beginScheduling(school, show.getId()).showId());
|
||||
}
|
||||
|
||||
public @Test void databaseRecoveryPersistsPendingFailureWithoutRestart() throws Exception {
|
||||
final Show show = show();
|
||||
final var entered = new CountDownLatch(1);
|
||||
final var release = new CountDownLatch(1);
|
||||
final var persistence = new OutagePersistence();
|
||||
final var runs = new SchedulingRuns(schools, blocking(entered, release));
|
||||
final java.lang.reflect.Field field = SchedulingRuns.class.getDeclaredField("persistence");
|
||||
field.setAccessible(true);
|
||||
field.set(runs, persistence);
|
||||
try {
|
||||
final SchedulingRun started = runs.start(school, show.getId());
|
||||
assertTrue(entered.await(10, TimeUnit.SECONDS));
|
||||
persistence.available.set(false);
|
||||
release.countDown();
|
||||
assertTrue(persistence.unavailable.await(10, TimeUnit.SECONDS));
|
||||
assertEquals(started, persistence.stored.get());
|
||||
assertEquals(started, runs.status(school, show.getId()));
|
||||
assertThrows(ManagementException.class, () -> runs.start(school, show.getId()));
|
||||
persistence.available.set(true);
|
||||
final SchedulingRun completed = await(runs, show);
|
||||
assertEquals(SchedulingState.SOLVER_FAILED, completed.state());
|
||||
assertEquals(started.runId(), completed.runId());
|
||||
assertEquals(completed, persistence.stored.get());
|
||||
schools.endScheduling(schools.beginScheduling(school, show.getId()).showId());
|
||||
} finally {
|
||||
persistence.available.set(true);
|
||||
release.countDown();
|
||||
runs.close();
|
||||
}
|
||||
}
|
||||
|
||||
private static class OutagePersistence extends SchedulingPersistence {
|
||||
private final CountDownLatch unavailable = new CountDownLatch(2);
|
||||
private final java.util.concurrent.atomic.AtomicBoolean available =
|
||||
new java.util.concurrent.atomic.AtomicBoolean(true);
|
||||
private final AtomicReference<SchedulingRun> stored = new AtomicReference<>();
|
||||
|
||||
private OutagePersistence() {
|
||||
super(null, null, null);
|
||||
}
|
||||
|
||||
public @Override <T> T work(final java.util.concurrent.Callable<T> action) {
|
||||
if (!available.get()) {
|
||||
unavailable.countDown();
|
||||
throw new nl.agilecc.agiledancers.repository.StorageException(new IllegalStateException("offline"));
|
||||
}
|
||||
try {
|
||||
return action.call();
|
||||
} catch (final RuntimeException failure) {
|
||||
throw failure;
|
||||
} catch (final Exception failure) {
|
||||
throw new IllegalStateException(failure);
|
||||
}
|
||||
}
|
||||
|
||||
public @Override void accepted(final SchedulingRun run, final Runnable launch, final Runnable rollback) {
|
||||
save(run, launch);
|
||||
}
|
||||
|
||||
public @Override void save(final SchedulingRun run, final Runnable publication) {
|
||||
stored.set(run);
|
||||
publication.run();
|
||||
}
|
||||
}
|
||||
|
||||
private Show show() {
|
||||
final long group = schools.createGroup(
|
||||
school, new GroupDetails("Group", DanceGroupType.REGULAR, 0, 99, Set.of()))
|
||||
|
||||
@@ -154,3 +154,27 @@ Retained review signals in changed code:
|
||||
One active backend remains the supported deployment model. Global execution
|
||||
capacity, distributed workers, database backup/restore and recovery after database
|
||||
volume loss remain outside S16. S17 owns backup/restore.
|
||||
|
||||
## Review follow-up: database recovery during calculation
|
||||
|
||||
Terminal scheduling outcomes whose transaction fails remain pending in their worker.
|
||||
The worker retries storage once per second without holding the school monitor between
|
||||
attempts, retaining its run guard until the terminal status commits. Publication still
|
||||
happens only after commit. Shutdown interrupts the wait; startup recovery remains
|
||||
responsible for persisted Running statuses after a process restart. If the show was
|
||||
removed during the outage, the pending outcome is discarded rather than recreating
|
||||
its status. The finish method's cognitive complexity 12 and cyclomatic complexity 8 distinguish
|
||||
retryable storage failures, removed shows and shutdown; these lifecycle decisions
|
||||
stay in the run owner. SchedulingRuns fan-out is now 34 and its test fixture fan-out
|
||||
35: the added storage exception, interruption handling and outage fixture belong to
|
||||
this existing persistence lifecycle boundary.
|
||||
|
||||
The regression test simulates unavailable storage during result persistence, verifies
|
||||
that no terminal status is published prematurely and duplicate starts remain rejected,
|
||||
then restores storage without restarting the backend and checks the stable run ID,
|
||||
persisted failure status and released guard. Public transaction/callback documentation
|
||||
now specifies transaction ownership, callback timing and exception behavior.
|
||||
|
||||
Follow-up verification: clean Maven verify against PostgreSQL passes all 73 backend
|
||||
tests and 19 standards-checker tests. Standards and the coverage gate pass;
|
||||
backend line coverage is 1769/1841 (96.09%).
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# S16 — Store state in persistent storage
|
||||
|
||||
Status: IN_REVIEW
|
||||
Status: IMPLEMENTED
|
||||
|
||||
Agreed after the grill-me interview; shared understanding confirmed.
|
||||
|
||||
Reference in New Issue
Block a user