Skip to content

CBG-5472: Make the stats logger independent of the database lock#8431

Open
bbrks wants to merge 1 commit into
CBG-3658from
CBG-5472
Open

CBG-5472: Make the stats logger independent of the database lock#8431
bbrks wants to merge 1 commit into
CBG-3658from
CBG-5472

Conversation

@bbrks

@bbrks bbrks commented Jul 6, 2026

Copy link
Copy Markdown
Member

CBG-5472

The stats logger could stall, leaving gaps in sg_stats.log, whenever a database config update held a lock the stats path also needed. Two independent locks were involved:

  • _databasesLock: updateCalculatedStats took RLock, but a config update holds the write lock across an infinite index-readiness wait, blocking the reader for the whole (unbounded) window.
  • base.SgwStats.dbStatsMapMutex: String() held it for the entire marshal, and NewDBStats/ClearDBStats held it for all Prometheus (un)registration under _databasesLock.

Decouple the reader from both:

  • ServerContext now keeps a lock-free atomic snapshot of the database contexts, refreshed under _databasesLock at each _databases mutation. updateCalculatedStats reads the snapshot, never the lock.
  • dbStatsMapMutex is now held only for the map operation: String() marshals a shallow copy, and NewDBStats/ClearDBStats do their Prometheus work outside the lock. The unregister* calls only touch Prometheus (not the DbStats maps), so this stays race-free.

The stats ticker now keeps emitting (stale-but-present) while databases are added, removed or reloaded.

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api
  • Self-review @bbrks

Dependencies (if applicable)

Integration Tests

The stats logger could stall, leaving gaps in sg_stats.log, whenever a
database config update held a lock the stats path also needed. Two
independent locks were involved:

- _databasesLock: updateCalculatedStats took RLock, but a config update
  holds the write lock across an infinite index-readiness wait, blocking
  the reader for the whole (unbounded) window.
- base.SgwStats.dbStatsMapMutex: String() held it for the entire marshal,
  and NewDBStats/ClearDBStats held it for all Prometheus (un)registration
  under _databasesLock.

Decouple the reader from both:

- ServerContext now keeps a lock-free atomic snapshot of the database
  contexts, refreshed under _databasesLock at each _databases mutation.
  updateCalculatedStats reads the snapshot, never the lock.
- dbStatsMapMutex is now held only for the map operation: String()
  marshals a shallow copy, and NewDBStats/ClearDBStats do their Prometheus
  work outside the lock. The unregister* calls only touch Prometheus (not
  the DbStats maps), so this stays race-free.

The stats ticker now keeps emitting (stale-but-present) while databases
are added, removed or reloaded.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bbrks bbrks self-assigned this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant