Repository navigation
Conversation
Keep OpenFGA from closing the shared SQLite pool and close it after registry services stop. Release resources on initialization failures and make shutdown idempotent. Add regression coverage for shared pool ownership, service shutdown ordering, alertmanager snapshots, and opaque token persistence. Refs ob-labs/ai-vision-ob#110
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.
Summary
OpenFGA currently closes the shared SQLite connection pool before other services finish shutting down. This can cause Alertmanager snapshot writes or Tokenizer persistence to fail and may produce a nonzero exit code after SIGTERM.
This change keeps the shared pool owned by SigNoz, which closes it after all services stop. It also releases resources on initialization failure and makes repeated or concurrent shutdown calls idempotent while preserving error reporting.
Refs ob-labs/ai-vision-ob#110
Validation evidence
Validated on 2026-10-23 using Linux amd64 and Go 1.25.7. The recorded source files match the code committed in
f80a8647f.Regression tests
go test -mod=readonly -race \ ./pkg/authz/openfgaserver \ ./pkg/signoz \ ./pkg/alertmanager/alertmanagerserver \ ./pkg/tokenizer/opaquetokenizer \ -count=1 -timeout=5mActual output:
As a control, replacing only
sqlstore.gowith its version from base commitd560a6ad5reproducedsql: database is closedin all three regression tests:TestSQLStoreClosePreservesSharedPoolTestShutdownSnapshotsAfterOpenFGAStopsTestShutdownFlushAfterOpenFGAStopsEvidence:
tests/full-race.log,tests/baseline.log, andtests/summary.jsonin the evidence bundle.Container shutdown and recovery
Built using the production Dockerfile:
docker buildx build --load --platform linux/amd64 \ -f cmd/community/Dockerfile.ob \ -t local/signoz-ob:issue110-acceptance .Local image ID:
Health, login, and authenticated API checks returned HTTP 200. Nonempty silence and notification-log fixtures survived restart, with snapshot timestamps advancing. No closed-database or snapshot-write errors were observed.
Evidence:
runtime/summary.json, per-round container inspections, logs, and snapshot records.Deployment and browser checks
upcompleted successfully. Both SigNoz shutdowns exited 0; all six services were healthy afterward. Existing and newly ingested traces contained the expected three spans.recastruns exited 0. All six previous containers exited 0 in each round. The historical trace retained its 20 expected spans.Evidence:
deployment/summary.json,deployment/negative-summary.json,cli/summary.json,browser-checks.json, and the browser screenshots.Scope
Validation used locally built images and seekdb. Compose replaced the frontend image while restarting the same fixed SigNoz image; CLI validation also used the same images across recasts. Published images, cross-version SigNoz upgrades, and OceanBase CE remain unverified.
Snapshot checks establish fixture persistence and restoration; they do not cover concurrent snapshot updates or actual notification delivery.