Skip to content

meta/sql: expose connection pool metrics - #7722

Open
CodeRusher wants to merge 4 commits into
juicedata:mainfrom
CodeRusher:feat/sql-pool-metrics
Open

CodeRusher wants to merge 4 commits into
juicedata:mainfrom
CodeRusher:feat/sql-pool-metrics

Conversation

@CodeRusher

@CodeRusher CodeRusher commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #7721

SQL metadata operations can slow down while waiting for a client-side connection, but existing transaction/operation metrics do not expose pool occupancy or waiting. For example, holding the only connection in a bounded pool makes subsequent acquisitions wait before their SQL executes.

Register the existing Prometheus collectors.NewDBStatsCollector in dbMeta.InitMetrics, exporting nine juicefs_go_sql_* metrics with db_name="metadata". Common metadata metrics and nil-registerer behavior are preserved; Redis/KV registration is unchanged. Collection reads local DB.Stats() without SQL queries, background collection loops, dependency updates, or pool configuration changes. English and Chinese metric references are included.

Registration errors, including conflicts with a custom db_name label, are logged as warnings. The SQL collector is omitted when registration fails, allowing initialization to continue with common metadata metrics.

Local validation on Go 1.25.10:

  • Standalone SQLite reproduction: ordinary custom labels export all nine SQL metric families; db_name=app and db_name=metadata log warnings without panicking, while common metrics still gather successfully; a nil registerer remains supported
  • Existing go test -mod=readonly -count=1 -run '^TestSQLiteClient$' ./pkg/meta
  • go build -mod=readonly ./pkg/meta, also with nosqlite and nosqlite,nomysql,nopg
  • go vet -mod=readonly ./pkg/meta; golangci-lint v2.6.0 reports no new issues

No new unit test files are included.

Register the existing Prometheus DBStats collector for SQL metadata clients so connection occupancy, waiting and churn are observable. Preserve common metadata metrics and keep non-SQL backends unchanged.

Add pool lifecycle, contention, concurrent collection and backend isolation tests, plus English and Chinese metric references.

Signed-off-by: CodeRusher <coderushing@gamil.com>
Keep one table-driven test for SQL collector registration, common metadata metrics, nil registerers and non-SQL backend isolation. Leave connection pool behavior and collector field mapping to the upstream dependencies.

Signed-off-by: CodeRusher <coderushing@gamil.com>
@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 22.06%. Comparing base (b3bc265) to head (e9f5850).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
pkg/meta/sql.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7722       +/-   ##
===========================================
- Coverage   57.86%   22.06%   -35.81%     
===========================================
  Files         180       31      -149     
  Lines       60562    25446    -35116     
===========================================
- Hits        35046     5615    -29431     
+ Misses      21907    19231     -2676     
+ Partials     3609      600     -3009     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: CodeRusher <coderushing@gamil.com>
Comment thread pkg/meta/sql.go Outdated
func (m *dbMeta) InitMetrics(reg prometheus.Registerer) {
m.baseMeta.InitMetrics(reg)
if reg != nil {
reg.MustRegister(collectors.NewDBStatsCollector(m.db.DB().DB, "metadata"))

@jiefenghuang jiefenghuang Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential label conflicts need to be handled here to prevent panics.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cb1511a. Replaced MustRegister with Register and a warning. A custom db_name label (even db_name=metadata) now causes the SQL collector to be skipped without a panic; common metadata metrics remain registered.

Validated with a standalone local SQLite reproduction covering ordinary labels, conflicting db_name=app / db_name=metadata, and a nil registerer. Normal registration exports all nine SQL metric families; conflicting registration logs a warning and other metrics still gather successfully. The existing TestSQLiteClient, default / nosqlite / SQL-disabled builds, go vet, and golangci-lint also passed.

Use Register and log failures so conflicting custom labels do not panic during SQL pool metric initialization. Keep existing metadata metrics and omit the SQL collector when registration fails.

Signed-off-by: CodeRusher <coderushing@gamil.com>

This branch has not been deployed

No deployments
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.

Expose client-side SQL metadata connection pool metrics

2 participants