From d8e4cffa12444da138a4fd0f67dfd665844c9912 Mon Sep 17 00:00:00 2001
From: Ben Johnson <benbjohnson@yahoo.com>
Date: Tue, 25 Mar 2014 07:25:00 -0600
Subject: [PATCH] Fix bucket reclamation.

The bucket page is allocated separately from the rest of the pages but the old bucket pages were
not being added to the freelist. This change fixes that and adds a simple check for database
consistency. More advanced consistency checks can be added in the future.

Fixes #82.
---
 db_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 tx.go      |  7 ++++---
 tx_test.go |  2 +-
 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/db_test.go b/db_test.go
index df17f94..a2df9d3 100644
--- a/db_test.go
+++ b/db_test.go
@@ -244,11 +244,11 @@ func TestDBStat(t *testing.T) {
 		// Obtain stats.
 		stat, err := db.Stat()
 		assert.NoError(t, err)
-		assert.Equal(t, stat.PageCount, 128)
-		assert.Equal(t, stat.FreePageCount, 2)
-		assert.Equal(t, stat.PageSize, 4096)
-		assert.Equal(t, stat.MmapSize, 4194304)
-		assert.Equal(t, stat.TxCount, 2)
+		assert.Equal(t, 126, stat.PageCount)
+		assert.Equal(t, 3, stat.FreePageCount)
+		assert.Equal(t, 4096, stat.PageSize)
+		assert.Equal(t, 4194304, stat.MmapSize)
+		assert.Equal(t, 2, stat.TxCount)
 
 		// Close readers.
 		t0.Rollback()
@@ -282,6 +282,48 @@ func TestDBMmapSize(t *testing.T) {
 	assert.Equal(t, db.mmapSize(1<<30), 1<<31)
 }
 
+// Ensure that database pages are in expected order and type.
+func TestDBConsistency(t *testing.T) {
+	withOpenDB(func(db *DB, path string) {
+		db.Update(func(tx *Tx) error {
+			return tx.CreateBucket("widgets")
+		})
+
+		for i := 0; i < 10; i++ {
+			db.Update(func(tx *Tx) error {
+				assert.NoError(t, tx.Bucket("widgets").Put([]byte("foo"), []byte("bar")))
+				return nil
+			})
+		}
+		db.Update(func(tx *Tx) error {
+			if p, _ := tx.Page(0); assert.NotNil(t, p) {
+				assert.Equal(t, "meta", p.Type)
+			}
+			if p, _ := tx.Page(1); assert.NotNil(t, p) {
+				assert.Equal(t, "meta", p.Type)
+			}
+			if p, _ := tx.Page(2); assert.NotNil(t, p) {
+				assert.Equal(t, "freelist", p.Type)
+			}
+			if p, _ := tx.Page(3); assert.NotNil(t, p) {
+				assert.Equal(t, "free", p.Type)
+			}
+			if p, _ := tx.Page(4); assert.NotNil(t, p) {
+				assert.Equal(t, "buckets", p.Type)
+			}
+			if p, _ := tx.Page(5); assert.NotNil(t, p) {
+				assert.Equal(t, "leaf", p.Type)
+			}
+			if p, _ := tx.Page(6); assert.NotNil(t, p) {
+				assert.Equal(t, "free", p.Type)
+			}
+			p, _ := tx.Page(7)
+			assert.Nil(t, p)
+			return nil
+		})
+	})
+}
+
 // Ensure that a database can return a string representation of itself.
 func TestDBString(t *testing.T) {
 	db := &DB{path: "/tmp/foo"}
diff --git a/tx.go b/tx.go
index a3ca825..3859de2 100644
--- a/tx.go
+++ b/tx.go
@@ -195,14 +195,15 @@ func (t *Tx) Commit() error {
 	}
 	t.buckets.write(p)
 
+	// Free previous bucket page and update meta.
+	t.db.freelist.free(t.id(), t.page(t.meta.buckets))
+	t.meta.buckets = p.id
+
 	// Write dirty pages to disk.
 	if err := t.write(); err != nil {
 		return err
 	}
 
-	// Update the meta.
-	t.meta.buckets = p.id
-
 	// Write meta to disk.
 	if err := t.writeMeta(); err != nil {
 		return err
diff --git a/tx_test.go b/tx_test.go
index 58cfc95..0694b3b 100644
--- a/tx_test.go
+++ b/tx_test.go
@@ -230,7 +230,7 @@ func TestTxDeleteBucket(t *testing.T) {
 
 		db.Update(func(tx *Tx) error {
 			// Verify that the bucket's page is free.
-			assert.Equal(t, []pgid{root}, db.freelist.all())
+			assert.Equal(t, []pgid{6, root, 3}, db.freelist.all())
 
 			// Create the bucket again and make sure there's not a phantom value.
 			assert.NoError(t, tx.CreateBucket("widgets"))