Skip to content

Eliminate multiple connection leaks#1022

Merged
kvch merged 1 commit intomainfrom
fix-get-row-count-leak
Feb 16, 2026
Merged

Eliminate multiple connection leaks#1022
kvch merged 1 commit intomainfrom
fix-get-row-count-leak

Conversation

@kvch
Copy link
Copy Markdown
Contributor

@kvch kvch commented Feb 16, 2026

This PR eliminates multiple connections leaks during migrate:

  1. 2 leaked connections every time we backfilled a table
  2. 1 leaked connection when checking if an index was valid
  3. 1 leaked connection when looking up schema history

Closes #1020

* getting row count during backfill
* getting schema info
* getting history

Closes #1020
@github-actions github-actions bot temporarily deployed to Docs Preview February 16, 2026 15:30 Inactive
@github-actions
Copy link
Copy Markdown

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/xataio/pgroll/pkg/backfill 17.05% (+12.22%) 🎉
github.com/xataio/pgroll/pkg/migrations 77.61% (+0.02%) 👍
github.com/xataio/pgroll/pkg/state 74.90% (+0.10%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/xataio/pgroll/pkg/backfill/backfill.go 17.39% (+17.39%) 92 (+5) 16 (+16) 76 (-11) 🎉
github.com/xataio/pgroll/pkg/migrations/dbactions.go 89.72% (+0.10%) 214 (+2) 192 (+2) 22 👍
github.com/xataio/pgroll/pkg/state/history.go 75.76% (+0.76%) 33 (+1) 25 (+1) 8 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/xataio/pgroll/pkg/backfill/backfill_test.go
  • github.com/xataio/pgroll/pkg/backfill/export_test.go

@kvch kvch requested a review from tsg February 16, 2026 18:01
// In that case, we can safely return false.
return false, nil
}
defer rows.Close()
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.

In this file rows.Close were missing.

if err != nil {
return nil, err
}
defer rows.Close()
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.

Missing rows.Close

defer rows.Close()

if err := db.ScanFirstValue(rows, &currentSchema); err != nil {
rows.Close()
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.

rows.Close is called immediately so the connection is returned to the pool right away. All rows.Close are required because rows gets reassigned twice, and they must be returned to the pool as well to avoid leaks.

@kvch kvch merged commit 0a64836 into main Feb 16, 2026
67 of 68 checks passed
@kvch kvch deleted the fix-get-row-count-leak branch February 16, 2026 20:16
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.

migrate commands holds multiple connections

2 participants