fleetd #512 follow-up: record why the drain-complete line must not move into a finally
The javadoc said the log.info fires "every time", which reads as an invitation to the exact edit that destroys it. The absence of the line is the signal that the drain died, so a finally would remove the signal and print partial counts in the same change. Raised by the fleet01 lead from their 2026-09-10 incident: that drain is known to have died only because it threw and left a stack trace. A drain that hung, or returned early on a condition, leaves no trace, no ERROR token and no priority -- only a missing line. Comment only. No behaviour change.
This commit is contained in:
@@ -1074,6 +1074,29 @@ public final class SessionManager implements TurnListener {
|
|||||||
* members were live) and must still produce the line. Both {@link #drainSnapshot} passes (the
|
* members were live) and must still produce the line. Both {@link #drainSnapshot} passes (the
|
||||||
* main snapshot and the straggler sweep) are folded into the one line: a caller reading two
|
* main snapshot and the straggler sweep) are folded into the one line: a caller reading two
|
||||||
* lines could not tell a two-pass drain from two separate drains.
|
* lines could not tell a two-pass drain from two separate drains.
|
||||||
|
*
|
||||||
|
* <p><strong>Non-goal: this line must never move into a {@code finally} block, and this method
|
||||||
|
* must never grow one around it.</strong> "Every time" above means every time the drain
|
||||||
|
* <em>finishes</em>, not every time this method exits. The absence of the line is the signal
|
||||||
|
* that the drain died, so a {@code finally} would destroy the signal and print confident
|
||||||
|
* partial counts in the same edit — the line would appear after a drain that threw, carrying
|
||||||
|
* whatever {@code tally} it had reached. Both halves of the value are lost at once. The line
|
||||||
|
* has to be the last statement of the successful path and reachable only from it.
|
||||||
|
*
|
||||||
|
* <p>This is written down because it is the obvious review comment ("shouldn't we always log
|
||||||
|
* the drain result?"), it sounds like thoroughness, and the paragraph above reads as an
|
||||||
|
* invitation to it. Raised by the fleet01 lead on 2026-09-12, from their 2026-09-10 incident:
|
||||||
|
* we only know that drain died on that host because it <em>threw</em>, and a
|
||||||
|
* {@code NoClassDefFoundError} reached the JVM's uncaught handler. A drain that hung on one
|
||||||
|
* session, or returned early on a condition rather than an exception, would leave no stack
|
||||||
|
* trace, no {@code ERROR} token and no priority — only a missing line. That makes the loud
|
||||||
|
* variant the one we have seen and the quiet variants the ones this line exists to catch.
|
||||||
|
*
|
||||||
|
* <p>Related: {@code released} and {@code abandoned} are counted incrementally inside {@link
|
||||||
|
* #drainSnapshot}'s loop and folded with {@link DrainTally#plus}, rather than derived from a
|
||||||
|
* collection read at the end, for the same reason. If a partial report is ever wanted it must
|
||||||
|
* be a different line with a different verb. One line must not serve both, or a reader cannot
|
||||||
|
* tell a finished drain from an interrupted one by its wording.
|
||||||
*/
|
*/
|
||||||
void drainAll(long timeoutNanos) {
|
void drainAll(long timeoutNanos) {
|
||||||
long deadline = System.nanoTime() + timeoutNanos;
|
long deadline = System.nanoTime() + timeoutNanos;
|
||||||
|
|||||||
Reference in New Issue
Block a user