Conversation
The lock guarded against two processes running AutoMigrate at once. That cannot happen here. db.Connect has one non-test caller, main.go:48. docker-compose gives the server container_name: quotient_server, a fixed name that pins the service to one instance, and sets no replica count. The five replicas belong to the runner, which works off Redis and never calls db.Connect. restart: always restarts sequentially. So the condition was created only by go test running two package binaries against one database. -p 1 on the integration job already fixes that: 8 of 8 runs clean with the lock reverted, while the same two binaries started concurrently still collide 3 of 6 times. The lock also made startup able to block forever, since pg_advisory_lock has no timeout and nothing logged while waiting, and it implied that concurrent migration is supported when AutoMigrate at startup would be the wrong mechanism for it anyway. engine/db/db.go returns byte for byte to its state before b1a6335. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NqKNZiXr2BPn1ztnmMmiXS
The -p 1 comment listed the Redis queues and the truncated rows as the reasons the two packages cannot run together. It did not mention AutoMigrate. That was true while the advisory lock existed; after reverting it, -p 1 is the only thing keeping two AutoMigrate runs off one catalog, so a reader who isolated the queues and the rows would drop the flag and get log.Fatalln at db.Connect. Name AutoMigrate, name the two packages rather than "these packages" (the glob matches five, and engine/checks, engine/config and engine/db hold no shared state), and point at the Makefile, which has passed -p 1 since eb53301 for the same reason. Connect gains a doc comment for the property that makes the flag necessary. It states what the code does rather than that one process happens to call it, which is a deployment fact that would rot. Verified: -p 1 clean 5 of 5 against a fresh database per run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NqKNZiXr2BPn1ztnmMmiXS
The merge interleaved two versions of Connect instead of picking one. It kept dbaseqp#150's DB struct and its d.createCumulativeScoresView() call, but took the old signature and the old AutoMigrate line, so the result had a function with no receiver calling d, an assignment to the package-level db that dbaseqp#150 deleted, and an unused gormDB. Three compile errors, and Connect no longer returned *DB, which engine.go:67 and testutil.go:69 both need. Rebuild db.go from upstream and remove only the lock. dbaseqp#150 had already moved migrate() onto the struct, so that method stays and loses its advisory lock mechanics; Connect keeps upstream's shape and its *DB return. The doc comment moves to Connect, the exported entry point. Against upstream this is now 4 insertions and 39 deletions in one file. Verified: build, vet and golangci-lint clean, and the integration suite 4 of 4 with a fresh database per run against the service configuration docker-compose.base.yml specifies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NqKNZiXr2BPn1ztnmMmiXS
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
2 participants
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.
Removed some unnecessary test changes.