Skip to content

Commit 0bed70c

Browse files
committed
tx_check: pass read tx into freelist load to avoid nested beginTx
Check already runs on a read transaction (often from View on another goroutine). Rebuilding an unsynced freelist called beginTx from freepages, which could block when combined with the outer View transaction. Pass the active Tx into loadFreelist so reconstruction uses freepagesWithTx and skips opening a nested read transaction (fixes #877). Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
1 parent fd2e4cc commit 0bed70c

2 files changed

Lines changed: 25 additions & 6 deletions

File tree

db.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ func Open(path string, mode os.FileMode, options *Options) (db *DB, err error) {
301301
}
302302

303303
if db.PreLoadFreelist {
304-
db.loadFreelist()
304+
db.loadFreelist(nil)
305305
}
306306

307307
if db.readOnly {
@@ -419,12 +419,23 @@ func (db *DB) getPageSizeFromSecondMeta() (int, bool, error) {
419419
// loadFreelist reads the freelist if it is synced, or reconstructs it
420420
// by scanning the DB if it is not synced. It assumes there are no
421421
// concurrent accesses being made to the freelist.
422-
func (db *DB) loadFreelist() {
422+
//
423+
// When sharedReadTx is non-nil, an unsynced freelist is reconstructed by
424+
// scanning using that transaction instead of opening a nested read-only
425+
// transaction. Tx.check passes the active read transaction so freelist
426+
// reconstruction does not call beginTx while another goroutine may still
427+
// hold the outer read transaction from View, which previously could block
428+
// indefinitely (see https://github.com/etcd-io/bbolt/issues/877).
429+
func (db *DB) loadFreelist(sharedReadTx *Tx) {
423430
db.freelistLoad.Do(func() {
424431
db.freelist = newFreelist(db.FreelistType)
425432
if !db.hasSyncedFreelist() {
426433
// Reconstruct free list by scanning the DB.
427-
db.freelist.Init(db.freepages())
434+
if sharedReadTx != nil {
435+
db.freelist.Init(db.freepagesWithTx(sharedReadTx))
436+
} else {
437+
db.freelist.Init(db.freepages())
438+
}
428439
} else {
429440
// Read free list from freelist page.
430441
db.freelist.Read(db.page(db.meta().Freelist()))
@@ -1250,6 +1261,13 @@ func (db *DB) freepages() []common.Pgid {
12501261
panic("freepages: failed to open read only tx")
12511262
}
12521263

1264+
return db.freepagesWithTx(tx)
1265+
}
1266+
1267+
// freepagesWithTx lists page IDs that are not reachable from the bucket tree
1268+
// for the given read-only transaction. The transaction must not be used
1269+
// concurrently while this function runs.
1270+
func (db *DB) freepagesWithTx(tx *Tx) []common.Pgid {
12531271
reachable := make(map[common.Pgid]*common.Page)
12541272
nofreed := make(map[common.Pgid]bool)
12551273
ech := make(chan error)
@@ -1267,7 +1285,7 @@ func (db *DB) freepages() []common.Pgid {
12671285
// TODO: If check bucket reported any corruptions (ech) we shouldn't proceed to freeing the pages.
12681286

12691287
var fids []common.Pgid
1270-
for i := common.Pgid(2); i < db.meta().Pgid(); i++ {
1288+
for i := common.Pgid(2); i < tx.meta.Pgid(); i++ {
12711289
if _, ok := reachable[i]; !ok {
12721290
fids = append(fids, i)
12731291
}

tx_check.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ func (tx *Tx) check(cfg checkConfig, ch chan error) {
4141
ch <- panicked{r}
4242
}
4343
}()
44-
// Force loading free list if opened in ReadOnly mode.
45-
tx.db.loadFreelist()
44+
// Force loading free list if opened in ReadOnly mode. Pass this tx so
45+
// freelist reconstruction does not open a nested read transaction.
46+
tx.db.loadFreelist(tx)
4647

4748
// Check if any pages are double freed.
4849
freed := make(map[common.Pgid]bool)

0 commit comments

Comments
 (0)