[receiver/sqlserver] Use a shared database connection pool across scrapers#49606
[receiver/sqlserver] Use a shared database connection pool across scrapers#49606singhvibhanshu wants to merge 26 commits into
Conversation
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
github-actions
Bot
added
receiver/sqlserver
Run Windows
|
/rerun |
|
/rerun |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
Pull request dashboard statusStatus last refreshed: 2026-07-21 01:14:27 UTC.
This automated status or its linked feedback items may be incorrect. If something looks wrong, report it with the result you expected. |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
|
/rerun |
|
/rerun |
| if p == nil { | ||
| return nil | ||
| } | ||
| p.closeOnce.Do(func() { |
There was a problem hiding this comment.
nit: If close() is called before getDB() has ever been invoked (as happens on all errors.Join(err, provider.close()) error paths), closeOnce fires while p.db == nil. If anything later manages to call getDB(), openOnce will still run and open a pool that close() will now refuse to touch (its once has already fired), silently leaking the *sql.DB. Consider guarding: only call p.closeOnce.Do when p.db != nil, or gate getDB() with a closed flag so it returns an error after close(). Today the call graph doesn’t trigger this (controller-failure paths return before scrapers start), but the invariant is fragile enough to warrant defensive handling.
There was a problem hiding this comment.
Replaced the dual-sync.Once with a mutex + closed flag so getDB() refuses to open after close().
PTAL 🙏🏻
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
|
@splunk-shanu, |
|
/rerun |
cc: @crobert-1, @sincejune, @XSAM, @ebrdarSplunk, @sv-splunk, @akshays-19 |
2 participants
Description
This PR aims at making the SQL Server Receiver share a single database across all its scrapers. More info here.
Link to tracking issue
Fixes #47219
Testing
Added
Documentation
Added the chagelog file
Authorship