Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recursive checker implementation. #225

Merged
merged 7 commits into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,8 @@ func (b *Bucket) forEachPageNode(fn func(*page, *node, int)) {
b._forEachPageNode(b.root, 0, fn)
}

func (b *Bucket) _forEachPageNode(pgid pgid, depth int, fn func(*page, *node, int)) {
var p, n = b.pageNode(pgid)
func (b *Bucket) _forEachPageNode(pgId pgid, depth int, fn func(*page, *node, int)) {
var p, n = b.pageNode(pgId)

// Execute function.
fn(p, n, depth)
Expand Down Expand Up @@ -654,11 +654,11 @@ func (b *Bucket) rebalance() {
}

// node creates a node from a page and associates it with a given parent.
func (b *Bucket) node(pgid pgid, parent *node) *node {
func (b *Bucket) node(pgId pgid, parent *node) *node {
_assert(b.nodes != nil, "nodes map expected")

// Retrieve node if it's already been created.
if n := b.nodes[pgid]; n != nil {
if n := b.nodes[pgId]; n != nil {
return n
}

Expand All @@ -673,12 +673,12 @@ func (b *Bucket) node(pgid pgid, parent *node) *node {
// Use the inline page if this is an inline bucket.
var p = b.page
if p == nil {
p = b.tx.page(pgid)
p = b.tx.page(pgId)
}

// Read the page into the node and cache it.
n.read(p)
b.nodes[pgid] = n
b.nodes[pgId] = n

// Update statistics.
b.tx.stats.IncNodeCount(1)
Expand Down
31 changes: 25 additions & 6 deletions cmd/bbolt/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (cmd *CheckCommand) Run(args ...string) error {
// Perform consistency check.
return db.View(func(tx *bolt.Tx) error {
var count int
for err := range tx.Check() {
for err := range tx.Check(CmdKvStringer()) {
fmt.Fprintln(cmd.Stdout, err)
count++
}
Expand Down Expand Up @@ -540,11 +540,7 @@ func formatBytes(b []byte, format string) (string, error) {
case "bytes":
return string(b), nil
case "auto":
if isPrintable(string(b)) {
return string(b), nil
} else {
return fmt.Sprintf("%x", b), nil
}
return bytesToAsciiOrHex(b), nil
case "redacted":
return fmt.Sprintf("<redacted len:%d>", len(b)), nil
default:
Expand Down Expand Up @@ -1573,6 +1569,15 @@ func isPrintable(s string) bool {
return true
}

func bytesToAsciiOrHex(b []byte) string {
sb := string(b)
if isPrintable(sb) {
return sb
} else {
return hex.EncodeToString(b)
}
}

func stringToPage(str string) (uint64, error) {
return strconv.ParseUint(str, 10, 64)
}
Expand Down Expand Up @@ -1689,3 +1694,17 @@ Additional options include:
Defaults to 64KB.
`, "\n")
}

type cmdKvStringer struct{}

func (_ cmdKvStringer) KeyToString(key []byte) string {
return bytesToAsciiOrHex(key)
}

func (_ cmdKvStringer) ValueToString(value []byte) string {
return bytesToAsciiOrHex(value)
}

func CmdKvStringer() bolt.KVStringer {
return cmdKvStringer{}
}
20 changes: 10 additions & 10 deletions cursor.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,13 @@ func (c *Cursor) goToFirstElementOnTheStack() {
}

// Keep adding pages pointing to the first element to the stack.
var pgid pgid
var pgId pgid
if ref.node != nil {
pgid = ref.node.inodes[ref.index].pgid
pgId = ref.node.inodes[ref.index].pgid
} else {
pgid = ref.page.branchPageElement(uint16(ref.index)).pgid
pgId = ref.page.branchPageElement(uint16(ref.index)).pgid
}
p, n := c.bucket.pageNode(pgid)
p, n := c.bucket.pageNode(pgId)
c.stack = append(c.stack, elemRef{page: p, node: n, index: 0})
}
}
Expand All @@ -193,13 +193,13 @@ func (c *Cursor) last() {
}

// Keep adding pages pointing to the last element in the stack.
var pgid pgid
var pgId pgid
if ref.node != nil {
pgid = ref.node.inodes[ref.index].pgid
pgId = ref.node.inodes[ref.index].pgid
} else {
pgid = ref.page.branchPageElement(uint16(ref.index)).pgid
pgId = ref.page.branchPageElement(uint16(ref.index)).pgid
}
p, n := c.bucket.pageNode(pgid)
p, n := c.bucket.pageNode(pgId)

var nextRef = elemRef{page: p, node: n}
nextRef.index = nextRef.count() - 1
Expand Down Expand Up @@ -268,8 +268,8 @@ func (c *Cursor) prev() (key []byte, value []byte, flags uint32) {
}

// search recursively performs a binary search against a given page/node until it finds a given key.
func (c *Cursor) search(key []byte, pgid pgid) {
p, n := c.bucket.pageNode(pgid)
func (c *Cursor) search(key []byte, pgId pgid) {
p, n := c.bucket.pageNode(pgId)
if p != nil && (p.flags&(branchPageFlag|leafPageFlag)) == 0 {
panic(fmt.Sprintf("invalid page type: %d: %x", p.id, p.flags))
}
Expand Down
4 changes: 3 additions & 1 deletion db.go
Original file line number Diff line number Diff line change
Expand Up @@ -1148,9 +1148,11 @@ func (db *DB) freepages() []pgid {
panic(fmt.Sprintf("freepages: failed to get all reachable pages (%v)", e))
}
}()
tx.checkBucket(&tx.root, reachable, nofreed, ech)
tx.checkBucket(&tx.root, reachable, nofreed, HexKVStringer(), ech)
close(ech)

// TODO: If check bucket reported any corruptions (ech) we shouldn't proceed to freeing the pages.

var fids []pgid
for i := pgid(2); i < db.meta().pgid; i++ {
if _, ok := reachable[i]; !ok {
Expand Down
4 changes: 2 additions & 2 deletions db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ func TestOpen_Check(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if err = db.View(func(tx *bolt.Tx) error { return <-tx.Check() }); err != nil {
if err = db.View(func(tx *bolt.Tx) error { return <-tx.Check(bolt.HexKVStringer()) }); err != nil {
t.Fatal(err)
}
if err = db.Close(); err != nil {
Expand All @@ -407,7 +407,7 @@ func TestOpen_Check(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if err := db.View(func(tx *bolt.Tx) error { return <-tx.Check() }); err != nil {
if err := db.View(func(tx *bolt.Tx) error { return <-tx.Check(bolt.HexKVStringer()) }); err != nil {
t.Fatal(err)
}
if err := db.Close(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions freelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ func (f *freelist) rollback(txid txid) {
}

// freed returns whether a given page is in the free list.
func (f *freelist) freed(pgid pgid) bool {
_, ok := f.cache[pgid]
func (f *freelist) freed(pgId pgid) bool {
_, ok := f.cache[pgId]
return ok
}

Expand Down
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,5 @@ require (
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/mod v0.7.0 // indirect
golang.org/x/tools v0.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=
golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/tools v0.4.0 h1:7mTAgkunk3fr4GAloyyCasadO6h9zSsQZbwvcaIciV4=
golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand Down
2 changes: 1 addition & 1 deletion internal/btesting/btesting.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (db *DB) MustCheck() {
err := db.Update(func(tx *bolt.Tx) error {
// Collect all the errors.
var errors []error
for err := range tx.Check() {
for err := range tx.Check(bolt.HexKVStringer()) {
errors = append(errors, err)
if len(errors) > 10 {
break
Expand Down
87 changes: 87 additions & 0 deletions internal/tests/tx_check_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package tests_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"

bolt "go.etcd.io/bbolt"
"go.etcd.io/bbolt/internal/btesting"
"go.etcd.io/bbolt/internal/guts_cli"
"go.etcd.io/bbolt/internal/surgeon"
)

func TestTx_RecursivelyCheckPages_MisplacedPage(t *testing.T) {
db := btesting.MustCreateDB(t)
require.NoError(t,
db.Fill([]byte("data"), 1, 10000,
func(tx int, k int) []byte { return []byte(fmt.Sprintf("%04d", k)) },
func(tx int, k int) []byte { return make([]byte, 100) },
))
require.NoError(t, db.Close())

xRay := surgeon.NewXRay(db.Path())

path1, err := xRay.FindPathsToKey([]byte("0451"))
require.NoError(t, err, "cannot find page that contains key:'0451'")
require.Len(t, path1, 1, "Expected only one page that contains key:'0451'")

path2, err := xRay.FindPathsToKey([]byte("7563"))
require.NoError(t, err, "cannot find page that contains key:'7563'")
require.Len(t, path2, 1, "Expected only one page that contains key:'7563'")

srcPage := path1[0][len(path1[0])-1]
targetPage := path2[0][len(path2[0])-1]
require.NoError(t, surgeon.CopyPage(db.Path(), srcPage, targetPage))

db.MustReopen()
require.NoError(t, db.Update(func(tx *bolt.Tx) error {
// Collect all the errors.
var errors []error
for err := range tx.Check(bolt.HexKVStringer()) {
errors = append(errors, err)
}
require.Len(t, errors, 1)
require.ErrorContains(t, errors[0], fmt.Sprintf("leaf page(%v) needs to be >= the key in the ancestor", targetPage))
return nil
}))
require.NoError(t, db.Close())
}

func TestTx_RecursivelyCheckPages_CorruptedLeaf(t *testing.T) {
db := btesting.MustCreateDB(t)
require.NoError(t,
db.Fill([]byte("data"), 1, 10000,
func(tx int, k int) []byte { return []byte(fmt.Sprintf("%04d", k)) },
func(tx int, k int) []byte { return make([]byte, 100) },
))
require.NoError(t, db.Close())

xray := surgeon.NewXRay(db.Path())

path1, err := xray.FindPathsToKey([]byte("0451"))
require.NoError(t, err, "cannot find page that contains key:'0451'")
require.Len(t, path1, 1, "Expected only one page that contains key:'0451'")

srcPage := path1[0][len(path1[0])-1]
p, pbuf, err := guts_cli.ReadPage(db.Path(), uint64(srcPage))
require.NoError(t, err)
require.Positive(t, p.Count(), "page must be not empty")
p.LeafPageElement(p.Count() / 2).Key()[0] = 'z'
require.NoError(t, surgeon.WritePage(db.Path(), pbuf))

db.MustReopen()
require.NoError(t, db.Update(func(tx *bolt.Tx) error {
// Collect all the errors.
var errors []error
for err := range tx.Check(bolt.HexKVStringer()) {
errors = append(errors, err)
}
require.Len(t, errors, 2)
require.ErrorContains(t, errors[0], fmt.Sprintf("leaf page(%v) needs to be < than key of the next element in ancestor", srcPage))
require.ErrorContains(t, errors[1], fmt.Sprintf("leaf page(%v) needs to be > (found <) than previous element", srcPage))
return nil
}))
require.NoError(t, db.Close())
}
12 changes: 8 additions & 4 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ func (n *node) prevSibling() *node {
}

// put inserts a key/value.
func (n *node) put(oldKey, newKey, value []byte, pgid pgid, flags uint32) {
if pgid >= n.bucket.tx.meta.pgid {
panic(fmt.Sprintf("pgid (%d) above high water mark (%d)", pgid, n.bucket.tx.meta.pgid))
func (n *node) put(oldKey, newKey, value []byte, pgId pgid, flags uint32) {
if pgId >= n.bucket.tx.meta.pgid {
panic(fmt.Sprintf("pgId (%d) above high water mark (%d)", pgId, n.bucket.tx.meta.pgid))
} else if len(oldKey) <= 0 {
panic("put: zero-length old key")
} else if len(newKey) <= 0 {
Expand All @@ -136,7 +136,7 @@ func (n *node) put(oldKey, newKey, value []byte, pgid pgid, flags uint32) {
inode.flags = flags
inode.key = newKey
inode.value = value
inode.pgid = pgid
inode.pgid = pgId
_assert(len(inode.key) > 0, "put: zero-length inode key")
}

Expand Down Expand Up @@ -585,6 +585,10 @@ func (n *node) dump() {
}
*/

func compareKeys(left, right []byte) int {
return bytes.Compare(left, right)
}

type nodes []*node

func (s nodes) Len() int { return len(s) }
Expand Down
1 change: 1 addition & 0 deletions scripts/fix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ XTESTGOFILES=$(${GO_CMD} list --f "{{with \$d:=.}}{{range .XTestGoFiles}}{{\$d.
echo "${GOFILES}" "${TESTGOFILES}" "${XTESTGOFILES}"| xargs -n 100 go run golang.org/x/tools/cmd/goimports@latest -w -local go.etcd.io

go fmt ./...
go mod tidy
Loading