fix(database): make a failed database open recoverable, and report it on /readyz

[vchaindz]

Fixes #2155
Closes #2156

Problem

allocDB caches a placeholder &dbRef{count: 1} before openDB runs. When the open fails, Get calls Release, which only decrements the refcount — a dbRef{db: nil, count: 0} stays in dbCache for the life of the process. That single entry poisons every reader that consults the cache:

Reader	Broken behaviour
GetState	returns unable to get state forever, and never reaches the m.Get fallback below it that would retry the open
IsActive	reports a database that never opened as active
GetOptionsByIndex	returns a nil *Options, which lazyDB.Path() and lazyDB.MaxResultSize() dereference
CloseAll	needed a nil guard (bfa78b3, #2108) to avoid panicking on shutdown

In the reported incident a transient S3 403 during the first open of two databases was therefore terminal for 3 h 20 min, until a restart.

Why nothing retried: the client state path short-circuits before Get.

ImmuServer.CurrentState → lazyDB.CurrentState → DBManager.GetState   ← returns here

m.Get is never called, so openDB is never retried — matching the reporter's observation that no S3 traffic appeared during the outage. UseDatabase passes too, because it checks db.IsClosed() and a failed open never sets dbInfo.closed.

Meanwhile Health returns Status: true unconditionally, so the pod stayed 1/1 Running and the outage surfaced as a crashloop in the client services instead.

Changes

Release pops the ref when the open failed and nobody picked it up, so a dbRef with a nil db is never observable. The pop lives in Release rather than a parallel cleanup path so the pendingClose hand-off still runs when Close races a failed open — a separate function orphans the parked ref and its database is never closed.

Get releases db.mtx before Release acquires m.mtx. This fixes a lock-order inversion reachable on master: allocDB takes m.mtx then db.mtx and returns holding db.mtx, so the old ordering let a second caller inside allocDB for the same index hold m.mtx and block on db.mtx while the failing open held db.mtx and waited on m.mtx. Since allocDB takes m.mtx first, that wedged Get for every database. The refcount always reaches zero on this path, so the inversion was reached on every failed open — only a concurrent caller was needed to close the cycle.

This is not what caused #2155: a deadlocked opener holds db.mtx, which would have blocked GetState on dbInfo.mtx and silenced the metrics goroutine, and the reporter saw its log line every minute throughout. It is a separate latent bug on the same path.

GetState falls through to m.Get instead of returning the sentinel, surfacing the real storage error and re-driving the open, rate-limited by openRetryInterval. A recorded failure outranks any state cached by a previous successful open, otherwise a database that fails to reopen serves a stale state and never retries. Net effect: the once-a-minute metrics loop becomes a self-healing retry driver.

GetOptionsByIndex collapses to dbInfo.getOptions() — both branches returned the same options; the only distinct effect of the cache branch was the nil.

OpenDB and NewDB now close the store on the sql.NewEngine, document.NewEngine and NewTxHolderPool error paths, where only InitIndexing did. A leaked store would make the retries this PR enables fail on the next attempt.

/readyz reports real state. The metrics server already served /initz, /readyz and /livez as unconditional 200 stubs. /readyz now returns 503 and names each database whose most recent open attempt failed and which has not opened successfully since. /livez and /initz stay unconditional — liveness should not depend on storage reachability. A database that was never accessed has never attempted an open and is not reported, so lazy startup registration does not fail readiness. The failure record is read atomically rather than under the dbInfo mutex, which is held for the whole duration of an open and so is precisely unavailable when a probe needs an answer.

IsActive is corrected in passing to require a ref that actually holds an open database.

Not included

DatabaseListV2's Loaded field is unchanged. Making it reflect real open state alters an existing field's meaning for current clients, and distinguishing "explicitly unloaded" from "failed to open" properly wants a new field and a proto change. /readyz provides the orchestrator signal without that.

Separately worth filing: allocDB holds m.mtx across db.mtx.Lock() while Get holds db.mtx across openDB, so one slow open of one database stalls allocDB for every database. Fixing that structurally (have allocDB return without db.mtx) makes the inversion above impossible rather than merely avoided, but it has a much larger blast radius than this fix.

Tests

TestDBManagerCloseAllAfterFailedOpen is updated — it asserted the ref remains cached, encoding the bug.

Added: a deadlock regression test (hangs against the old lock ordering — verified by reverting just that hunk), failed-open-then-recover through GetState, concurrent fail-then-succeed on one index, Close racing a failed open (guards the pendingClose drain), GetOptionsByIndex after a failed open, the OpenFailures accessor, and /readyz in both states.

go build ./... and go vet ./... clean. go test -race ./pkg/database/... and go test ./pkg/server/... pass. The first commit was verified green standing alone.

[coveralls]

coverage: 84.86% (+0.02%) from 84.841% — fix/2155-failed-open-db-ref into master