diff --git a/internal/tests/tx_check_test.go b/internal/tests/tx_check_test.go index 336c98d1f..342198fa5 100644 --- a/internal/tests/tx_check_test.go +++ b/internal/tests/tx_check_test.go @@ -2,9 +2,10 @@ package tests_test import ( "fmt" - "github.com/stretchr/testify/require" "testing" + "github.com/stretchr/testify/require" + bolt "go.etcd.io/bbolt" "go.etcd.io/bbolt/internal/btesting" "go.etcd.io/bbolt/internal/guts_cli" @@ -23,11 +24,11 @@ func TestTx_RecursivelyCheckPages_MisplacedPage(t *testing.T) { xRay := surgeon.NewXRay(db.Path()) path1, err := xRay.FindPathsToKey([]byte("0451")) - require.NoError(t, err, "Cannot find page that contains key:'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.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] @@ -60,7 +61,7 @@ func TestTx_RecursivelyCheckPages_CorruptedLeaf(t *testing.T) { xray := surgeon.NewXRay(db.Path()) path1, err := xray.FindPathsToKey([]byte("0451")) - require.NoError(t, err, "Cannot find page that contains key:'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] diff --git a/tx_check.go b/tx_check.go index 0c8299764..23e22c3da 100644 --- a/tx_check.go +++ b/tx_check.go @@ -125,23 +125,11 @@ func (tx *Tx) recursivelyCheckPagesInternal( runningMin := minKeyClosed for i := range p.branchPageElements() { elem := p.branchPageElement(uint16(i)) - if i == 0 && runningMin != nil && compareKeys(runningMin, elem.key()) > 0 { - ch <- fmt.Errorf("key (%d, %s) on the branch page(%d) needs to be >="+ - " to the key(%s) in the ancestor. Pages stack: %v", - i, keyToString(elem.key()), pgId, keyToString(runningMin), pagesStack) - } - - if maxKeyOpen != nil && compareKeys(elem.key(), maxKeyOpen) >= 0 { - ch <- fmt.Errorf("key (%d: %s) on the branch page(%d) needs to be <"+ - " than key of the next element reachable from the ancestor (%v). Pages stack: %v", - i, keyToString(elem.key()), pgId, keyToString(maxKeyOpen), pagesStack) - } + verifyKeyOrder(elem.pgid, "branch", i, elem.key(), runningMin, maxKeyOpen, ch, keyToString, pagesStack) - var maxKey []byte + maxKey := maxKeyOpen if i < len(p.branchPageElements())-1 { maxKey = p.branchPageElement(uint16(i + 1)).key() - } else { - maxKey = maxKeyOpen } maxKeyInSubtree = tx.recursivelyCheckPagesInternal(elem.pgid, elem.key(), maxKey, pagesStack, keyToString, ch) runningMin = maxKeyInSubtree @@ -151,25 +139,7 @@ func (tx *Tx) recursivelyCheckPagesInternal( runningMin := minKeyClosed for i := range p.leafPageElements() { elem := p.leafPageElement(uint16(i)) - if i == 0 && runningMin != nil && compareKeys(runningMin, elem.key()) > 0 { - ch <- fmt.Errorf("The first key[%d]=(hex)%s on leaf page(%d) needs to be >= the key in the ancestor (%s). Stack: %v", - i, keyToString(elem.key()), pgId, keyToString(runningMin), pagesStack) - } - if i > 0 { - cmpRet := compareKeys(runningMin, elem.key()) - if cmpRet > 0 { - ch <- fmt.Errorf("key[%d]=(hex)%s on leaf page(%d) needs to be > (found <) than previous element (hex)%s. Stack: %v", - i, keyToString(elem.key()), pgId, keyToString(runningMin), pagesStack) - } - if cmpRet == 0 { - ch <- fmt.Errorf("key[%d]=(hex)%s on leaf page(%d) needs to be > (found =) than previous element (hex)%s. Stack: %v", - i, keyToString(elem.key()), pgId, keyToString(runningMin), pagesStack) - } - } - if maxKeyOpen != nil && compareKeys(elem.key(), maxKeyOpen) >= 0 { - ch <- fmt.Errorf("key[%d]=(hex)%s on leaf page(%d) needs to be < than key of the next element in ancestor (hex)%s. Pages stack: %v", - i, keyToString(elem.key()), pgId, keyToString(maxKeyOpen), pagesStack) - } + verifyKeyOrder(pgId, "leaf", i, elem.key(), runningMin, maxKeyOpen, ch, keyToString, pagesStack) runningMin = elem.key() } if p.count > 0 { @@ -181,6 +151,32 @@ func (tx *Tx) recursivelyCheckPagesInternal( return maxKeyInSubtree } +/*** + * verifyKeyOrder checks whether an entry with given #index on pgId (pageType: "branch|leaf") that has given "key", + * is within range determined by (previousKey..maxKeyOpen) and reports found violations to the channel (ch). + */ +func verifyKeyOrder(pgId pgid, pageType string, index int, key []byte, previousKey []byte, maxKeyOpen []byte, ch chan error, keyToString func([]byte) string, pagesStack []pgid) { + if index == 0 && previousKey != nil && compareKeys(previousKey, key) > 0 { + ch <- fmt.Errorf("the first key[%d]=(hex)%s on %s page(%d) needs to be >= the key in the ancestor (%s). Stack: %v", + index, keyToString(key), pageType, pgId, keyToString(previousKey), pagesStack) + } + if index > 0 { + cmpRet := compareKeys(previousKey, key) + if cmpRet > 0 { + ch <- fmt.Errorf("key[%d]=(hex)%s on %s page(%d) needs to be > (found <) than previous element (hex)%s. Stack: %v", + index, keyToString(key), pageType, pgId, keyToString(previousKey), pagesStack) + } + if cmpRet == 0 { + ch <- fmt.Errorf("key[%d]=(hex)%s on %s page(%d) needs to be > (found =) than previous element (hex)%s. Stack: %v", + index, keyToString(key), pageType, pgId, keyToString(previousKey), pagesStack) + } + } + if maxKeyOpen != nil && compareKeys(key, maxKeyOpen) >= 0 { + ch <- fmt.Errorf("key[%d]=(hex)%s on %s page(%d) needs to be < than key of the next element in ancestor (hex)%s. Pages stack: %v", + index, keyToString(key), pageType, pgId, keyToString(previousKey), pagesStack) + } +} + // =========================================================================================== // KVStringer allows to prepare human-readable diagnostic messages.