diff --git a/bucket_test.go b/bucket_test.go index 21ddfca..7e959d0 100644 --- a/bucket_test.go +++ b/bucket_test.go @@ -190,9 +190,9 @@ func TestBucket_Delete_Large(t *testing.T) { }) } -// Ensure that deleting a large set of keys will work correctly. -// Reported by Jordan Sherer: https://github.com/boltdb/bolt/issues/184 -func TestBucket_Delete_Large2(t *testing.T) { +// Deleting a very large list of keys will overflow the freelist. +// https://github.com/boltdb/bolt/issues/192 +func TestBucket_Delete_ErrFreelistOverflow(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode.") } @@ -223,7 +223,7 @@ func TestBucket_Delete_Large2(t *testing.T) { } // Delete all of them in one large transaction - db.Update(func(tx *Tx) error { + err := db.Update(func(tx *Tx) error { b := tx.Bucket([]byte("0")) c := b.Cursor() for k, _ := c.First(); k != nil; k, _ = c.Next() { @@ -231,6 +231,9 @@ func TestBucket_Delete_Large2(t *testing.T) { } return nil }) + + // Check that a freelist overflow occurred. + assert.Equal(t, ErrFreelistOverflow, err) }) } diff --git a/freelist.go b/freelist.go index 407dc79..e27f80a 100644 --- a/freelist.go +++ b/freelist.go @@ -113,6 +113,11 @@ func (f *freelist) release(txid txid) { sort.Sort(pgids(f.ids)) } +// rollback removes the pages from a given pending tx. +func (f *freelist) rollback(txid txid) { + delete(f.pending, txid) +} + // isFree returns whether a given page is in the free list. func (f *freelist) isFree(pgid pgid) bool { for _, id := range f.ids { @@ -146,7 +151,7 @@ func (f *freelist) write(p *page) error { ids := f.all() // Make sure that the sum of all free pages is less than the max uint16 size. - if len(ids) > (1 << 16) { + if len(ids) >= 65565 { return ErrFreelistOverflow } @@ -157,3 +162,20 @@ func (f *freelist) write(p *page) error { return nil } + +// reload reads the freelist from a page and filters out pending items. +func (f *freelist) reload(p *page) { + f.read(p) + + // Filter out pending free pages. + ids := f.ids + f.ids = nil + + var a []pgid + for _, id := range ids { + if !f.isFree(id) { + a = append(a, id) + } + } + f.ids = a +} diff --git a/tx.go b/tx.go index 211085a..9ab1178 100644 --- a/tx.go +++ b/tx.go @@ -157,7 +157,7 @@ func (tx *Tx) Commit() error { // spill data onto dirty pages. startTime = time.Now() if err := tx.root.spill(); err != nil { - tx.close() + tx.rollback() return err } tx.stats.SpillTime += time.Since(startTime) @@ -170,10 +170,11 @@ func (tx *Tx) Commit() error { tx.db.freelist.free(tx.id(), tx.db.page(tx.meta.freelist)) p, err := tx.allocate((tx.db.freelist.size() / tx.db.pageSize) + 1) if err != nil { - tx.close() + tx.rollback() return err } if err := tx.db.freelist.write(p); err != nil { + tx.rollback() return err } tx.meta.freelist = p.id @@ -181,7 +182,7 @@ func (tx *Tx) Commit() error { // Write dirty pages to disk. startTime = time.Now() if err := tx.write(); err != nil { - tx.close() + tx.rollback() return err } @@ -195,7 +196,7 @@ func (tx *Tx) Commit() error { // Write meta to disk. if err := tx.writeMeta(); err != nil { - tx.close() + tx.rollback() return err } tx.stats.WriteTime += time.Since(startTime) @@ -217,10 +218,18 @@ func (tx *Tx) Rollback() error { if tx.db == nil { return ErrTxClosed } - tx.close() + tx.rollback() return nil } +func (tx *Tx) rollback() { + if tx.writable { + tx.db.freelist.rollback(tx.id()) + tx.db.freelist.reload(tx.db.page(tx.db.meta().freelist)) + } + tx.close() +} + func (tx *Tx) close() { if tx.writable { // Remove writer lock.