Conversation
…named locks Every backend server runs every @scheduled job. The pipeline scans were safe only because each transition is a conditional UPDATE, and the restart and alert scans were not safe at all: two servers fired the same scheduled restart and sent the same alert in the same minute. Add NamedLockRegistry over MySQL user-level locks (GET_LOCK / RELEASE_LOCK) on one dedicated connection per server, opened outside the Hikari pool and kept for the life of the process. A lock lives as long as its session, so a dead server's locks are freed the moment its connection drops, with no lease to renew and no clock involved. The registry is fail-closed: with its connection down a server drives nothing until it reconnects. The pipeline scans take one lock per pipeline (oops:pipeline:{id}), so a pipeline is driven by one server from build through rollout while other servers drive other pipelines. The rollout scan sweeps locks whose pipeline was finished elsewhere, snapshotting the held locks before it queries so a lock the build scan takes concurrently is never swept. The restart and alert scans take one sticky lock each on their first tick and keep it, so one server leads until it goes away. The lock is not a fence - a server whose session MySQL cut keeps believing in its locks until it next talks to the registry - which is why every pipeline transition still goes through updateStatusIfMatch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pipelines Four gaps either let a stale read overwrite a transition made elsewhere or left a pipeline in a state nothing would ever move it out of, which blocks the application from being deployed again: - stopPipeline saved the whole row unconditionally. With the scan job carrying a RUNNING pipeline into DEPLOYING on any server in between, the stop overwrote the deploy in progress, which then ran to completion under a pipeline that said it was stopped. It now transitions from the status the caller read and fails with "state changed concurrently" when that read is stale. - completeDeployPhase (scan job and service) ignored the result of the DEPLOYING -> ROLLING_OUT claim, so a pipeline stopped while its artifact was applied was announced as rolling out anyway. A lost claim is now logged and nothing further is reported; the deploy and rollback failure paths honour their claims the same way. - A RUNNING pipeline whose build Job the cluster no longer had (deleted by hand, work namespace cleaned, TTL-reaped) mapped to UNKNOWN and matched no branch, so it stayed RUNNING for good. A missing Job now fails the build at once; a Job that exists without a status is still left for the next tick. Stopping such a pipeline no longer errors on the missing Job either. - Nothing looked at DEPLOYING, so a server dying mid-deploy left the pipeline there forever. The rollout scan now fails a pipeline that has stayed DEPLOYING for ten minutes, measured from when the scanning server first saw it deploying, since no column records status-change time and the conditional updates bypass entity timestamps. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The build log moved from a WebSocket to two server-sent event streams (steps/watch and log?container=) in #99, but the integration suite was not brought along: routes.json still lacked the two routes, which fails the workflow's inventory check on every branch, and test_streams.py still opened the removed pipeline log socket and asserted its frame types and ping/pong. Regenerate routes.json, give OopsClient an SSE reader that records the call for the coverage check, and replace the two socket tests with the contract of the streams: the watch opens with the step list, reports the finished build with every step SUCCEEDED, and ends by itself; a finished step's log replays in stamped batches, ends, and a reconnect carrying Last-Event-ID replays nothing already seen. The ping/pong keepalive contract stays covered on the pod log socket, which still has it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…c dispatch Every server-sent event stream (pipeline steps and logs, pod statuses) ended with the authorization filter denying it. The SseEmitter completes on an ASYNC dispatch, JwtAuthFilter is a OncePerRequestFilter that does not run again on that dispatch and nothing carries the authentication over, so the dispatch arrived anonymous and was refused after the whole stream had already been sent. In the log that is "Unable to handle the Spring Security Exception because the response is already committed" for every stream; on the wire the chunked response is cut short of its terminator. A browser's EventSource shrugs, which is why the UI never showed it; any other client, the integration suite included, reports "Response ended prematurely". Permit the ASYNC dispatch type in the authorization rules: the request it belongs to was authorized when it came in, and the dispatch is the same request finishing. The initial dispatch stays guarded, which the new test pins alongside the async one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the pod in The pod log and terminal socket tests opened their sockets without the environment query parameter both handlers resolve the pod through, so the server closed each socket at once with "Environment not found". The tests passed anyway: a loop over zero frames asserts nothing, and a terminal that answers nothing satisfies "does not answer ping". The new ping/pong test for the pod log socket was the first to look for a frame and see there never was one. Pass the environment, and require the log socket to deliver at least one line before it closes so the transport assertions have something to check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
read_until_closed relied on the socket timeout to give up on a pod that writes nothing more, but the server pings every socket every ten seconds as a keepalive and websocket-client's recv() answers those inside its own loop, each ping restarting the socket timeout. On the fixture pod, which logs one line at startup and then nothing, the read after that line blocked for as long as the backend stayed up: two local runs sat on the pod log test for hours. It never showed before because the sockets were closed at once for want of the environment parameter. Read frame by frame with control frames included, so the deadline is checked after every ping, and re-arm the socket timeout from what is left of it before each read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Integration result, The CI run on 98ff624 failed on exactly the things the three commits since then fix: the two SSE tests (the async-dispatch denial truncating every stream), the pod log ping test (socket closed for want of |
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every backend server runs every
@Scheduledjob. The pipeline scans were safe only because each transition is a conditional UPDATE, and the scheduled-restart and resource-alert scans were not safe at all: two servers fired the same restart and sent the same alert in the same minute.NamedLockRegistry (infrastructure/lock) puts MySQL user-level locks (
GET_LOCK/RELEASE_LOCK) on one dedicated connection per server, opened with DriverManager from the datasource properties and kept outside the Hikari pool for the life of the process. A lock lives exactly as long as its session, so a dead server's locks are freed the moment its connection drops — no lease to renew, no clock involved. The registry is fail-closed: with its connection down a server drives nothing until it reconnects, which is recoverable, whereas two servers driving the same thing is what the lock exists to prevent. It validates the idle session every 30s sowait_timeoutnever reaps it unnoticed.The pipeline scans take one lock per pipeline (
oops:pipeline:{id}), so one server drives a pipeline from build through rollout while other servers drive other pipelines. The rollout scan sweeps locks whose pipeline was finished elsewhere (a user stopping it from any server); it snapshots the held locks before it queries the active pipelines, so a lock the build scan takes concurrently is never swept as no longer active. The restart and alert scans take one sticky lock each on their first tick and keep it, so one server leads until it goes away — releasing after each tick would let a second server's tick in the same minute fire the same restart again.The lock is not a fence. A server whose session MySQL cut keeps believing in its locks until it next talks to the registry, which is why every pipeline transition still goes through
updateStatusIfMatch: the lock makes collisions rare, the CAS makes them harmless. That is the same split client-go's leader election documents for itself.Reviewing the state machine for that turned up four gaps, fixed in the second commit.
stopPipelinesaved the whole row unconditionally, so a stop could overwrite a deploy the scan job had just claimed, which then ran to completion under a pipeline that said it was stopped; it now transitions from the status the caller read and fails with "state changed concurrently" when that read is stale.completeDeployPhaseignored the result of its DEPLOYING → ROLLING_OUT claim, so a pipeline stopped while its artifact was applied was announced as rolling out anyway. A RUNNING pipeline whose build Job the cluster no longer had (deleted by hand, work namespace cleaned, TTL-reaped) mapped to UNKNOWN and matched no branch, so it stayed RUNNING for good and the application could never be deployed again; a missing Job now fails the build at once, and a Job that exists without a status is still left for the next tick. Nothing looked at DEPLOYING, so a server dying mid-deploy left the pipeline there forever with the same effect; the rollout scan now fails a pipeline that has stayed DEPLOYING for ten minutes. That timeout is measured from when the scanning server first saw the pipeline deploying, because no column records status-change time and the conditional updates bypass entity timestamps — a restart only starts the clock again. If a precise timestamp is preferred, the alternative is astatus_changed_timecolumn written by the conditional updates.Rebased onto main after #101, which carries the SSE and integration-suite fixes this branch had first needed (the async-dispatch rule, the route inventory, the stream tests and the socket reader); those commits are dropped here in favour of main's. No schema change. AGENTS.md documents the lock under "Multi-server coordination". The backend suite (300 tests) passes against a throwaway mysql:8.4.