Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

Commit e8562e4

Browse files
committed
basic dir integrity testing
1 parent f1271f9 commit e8562e4

File tree

5 files changed

+110
-11
lines changed

5 files changed

+110
-11
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ require (
1818
github.com/smartystreets/assertions v1.0.0 // indirect
1919
github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a // indirect
2020
github.com/spaolacci/murmur3 v1.1.0
21+
github.com/stretchr/testify v1.7.0 // indirect
2122
github.com/warpfork/go-wish v0.0.0-20190328234359-8b3e70f8e830 // indirect
2223
)
2324

go.sum

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,13 @@ github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572 h1:RC6RW7j+
271271
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572/go.mod h1:w0SWMsp6j9O/dk4/ZpIhL+3CkG8ofA2vuv7k+ltqUMc=
272272
github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI=
273273
github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
274+
github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
274275
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
275276
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
276277
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
277278
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
279+
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
280+
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
278281
github.com/syndtr/goleveldb v1.0.0/go.mod h1:ZVVdQEZoIme9iO1Ch2Jdy24qqXrMMOU6lpPAyBWyWuQ=
279282
github.com/warpfork/go-wish v0.0.0-20180510122957-5ad1f5abf436/go.mod h1:x6AKhvSSexNrVSrViXSHUEbICjmGXhtgABaHIySUSGw=
280283
github.com/warpfork/go-wish v0.0.0-20190328234359-8b3e70f8e830 h1:8kxMKmKzXXL4Ru1nyhvdms/JjWt+3YLpvRb/bAjO/y0=
@@ -374,4 +377,6 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
374377
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
375378
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
376379
honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
380+
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
381+
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
377382
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=

hamt/hamt.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,11 @@ func (ds *Shard) setValue(ctx context.Context, hv *hashBits, key string, value *
502502
// (which will be nil) to highlight there is no overwrite here: they are
503503
// done with different keys to a new (empty) shard. (At best this shard
504504
// will create new ones until we find different slots for both.)
505+
// FIXME: These two shouldn't be recursive calls where one function
506+
// adds the child and the other replaces it with a shard to go one
507+
// level deeper. This should just be a simple loop that traverses the
508+
// shared prefix in the key adding as many shards as needed and finally
509+
// inserts both leaf links together.
505510
_, err = child.setValue(ctx, hv, key, value)
506511
if err != nil {
507512
return

io/directory.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ type Directory interface {
8080
// TODO: Evaluate removing `dserv` from this layer and providing it in MFS.
8181
// (The functions should in that case add a `DAGService` argument.)
8282

83+
// Link size estimation function. For production it's usually the one here
84+
// but during test we may mock it to get fixed sizes.
85+
func productionLinkSize(linkName string, linkCid cid.Cid) int {
86+
return len(linkName) + linkCid.ByteLen()
87+
}
88+
var estimatedLinkSize = productionLinkSize
89+
8390
// BasicDirectory is the basic implementation of `Directory`. All the entries
8491
// are stored in a single node.
8592
type BasicDirectory struct {
@@ -146,6 +153,7 @@ func NewDirectoryFromNode(dserv ipld.DAGService, node ipld.Node) (Directory, err
146153
case format.TDirectory:
147154
return &UpgradeableDirectory{newBasicDirectoryFromNode(dserv, protoBufNode.Copy().(*mdag.ProtoNode))}, nil
148155
case format.THAMTShard:
156+
// FIXME: We now need to return also the UpgradeableDirectory here.
149157
shard, err := hamt.NewHamtFromDag(dserv, node)
150158
if err != nil {
151159
return nil, err
@@ -167,10 +175,6 @@ func (d *BasicDirectory) computeEstimatedSize() {
167175
})
168176
}
169177

170-
func estimatedLinkSize(linkName string, linkCid cid.Cid) int {
171-
return len(linkName) + linkCid.ByteLen()
172-
}
173-
174178
func (d *BasicDirectory) addToEstimatedSize(name string, linkCid cid.Cid) {
175179
d.estimatedSize += estimatedLinkSize(name, linkCid)
176180
}
@@ -543,6 +547,17 @@ func (d *UpgradeableDirectory) AddChild(ctx context.Context, name string, nd ipl
543547
return nil
544548
}
545549

550+
func (d *UpgradeableDirectory) getDagService() ipld.DAGService {
551+
switch v := d.Directory.(type) {
552+
case *BasicDirectory:
553+
return v.dserv
554+
case *HAMTDirectory:
555+
return v.dserv
556+
default:
557+
panic("unknown directory type")
558+
}
559+
}
560+
546561
// RemoveChild implements the `Directory` interface. Used in the case where we wrap
547562
// a HAMTDirectory that might need to be downgraded to a BasicDirectory. The
548563
// upgrade path is in AddChild.

io/directory_test.go

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import (
44
"context"
55
"fmt"
66
"math"
7+
"sort"
8+
"strings"
79
"testing"
810

11+
cid "github.com/ipfs/go-cid"
912
ipld "github.com/ipfs/go-ipld-format"
1013
mdag "github.com/ipfs/go-merkledag"
1114
mdtest "github.com/ipfs/go-merkledag/test"
1215

1316
ft "github.com/ipfs/go-unixfs"
17+
18+
"github.com/stretchr/testify/assert"
1419
)
1520

1621
func TestEmptyNode(t *testing.T) {
@@ -164,6 +169,14 @@ func TestBasicDirectory_estimatedSize(t *testing.T) {
164169
basicDir.estimatedSize, restoredBasicDir.estimatedSize)
165170
}
166171
}
172+
// FIXME: Add a similar one for HAMT directory, stressing particularly the
173+
// deleted/overwritten entries and their computation in the size variation.
174+
175+
func mockLinkSizeFunc(fixedSize int) func(linkName string, linkCid cid.Cid) int {
176+
return func(_ string, _ cid.Cid) int {
177+
return fixedSize
178+
}
179+
}
167180

168181
// Basic test on extreme threshold to trigger switch. More fine-grained sizes
169182
// are checked in TestBasicDirectory_estimatedSize (without the swtich itself
@@ -172,8 +185,12 @@ func TestBasicDirectory_estimatedSize(t *testing.T) {
172185
// upgrade on another a better structured test should test both dimensions
173186
// simultaneously.
174187
func TestUpgradeableDirectory(t *testing.T) {
188+
// FIXME: Modifying these static configuraitons is probably not
189+
// concurrent-friendly.
175190
oldHamtOption := HAMTShardingSize
176191
defer func() { HAMTShardingSize = oldHamtOption }()
192+
estimatedLinkSize = mockLinkSizeFunc(1)
193+
defer func() { estimatedLinkSize = productionLinkSize }()
177194

178195
ds := mdtest.Mock()
179196
dir := NewDirectory(ds)
@@ -212,23 +229,79 @@ func TestUpgradeableDirectory(t *testing.T) {
212229
if _, ok := dir.(*UpgradeableDirectory).Directory.(*HAMTDirectory); !ok {
213230
t.Fatal("UpgradeableDirectory wasn't upgraded to HAMTDirectory for a low threshold")
214231
}
232+
upgradedDir := copyDir(t, dir)
215233

216234
// Remove the single entry triggering the switch back to BasicDirectory
217235
err = dir.RemoveChild(ctx, "test")
218236
if err != nil {
219237
t.Fatal(err)
220238
}
221-
// For performance reasons we only switch when serializing the data
222-
// in the node format and not on any entry removal.
223-
_, err = dir.GetNode()
224-
if err != nil {
225-
t.Fatal(err)
226-
}
227239
if _, ok := dir.(*UpgradeableDirectory).Directory.(*BasicDirectory); !ok {
228240
t.Fatal("UpgradeableDirectory wasn't downgraded to BasicDirectory after removal of the single entry")
229241
}
230242

231-
// FIXME: Test integrity of entries in-between switches.
243+
// Check integrity between switches.
244+
// We need to account for the removed entry that triggered the switch
245+
// back.
246+
// FIXME: Abstract this for arbitrary entries.
247+
missingLink, err := ipld.MakeLink(child)
248+
assert.NoError(t, err)
249+
missingLink.Name = "test"
250+
compareDirectoryEntries(t, upgradedDir, dir, []*ipld.Link{missingLink})
251+
}
252+
253+
// Compare entries in the leftDir against the rightDir and possibly
254+
// missingEntries in the second.
255+
func compareDirectoryEntries(t *testing.T, leftDir Directory, rightDir Directory, missingEntries []*ipld.Link) {
256+
leftLinks, err := getAllLinksSortedByName(leftDir)
257+
assert.NoError(t, err)
258+
rightLinks, err := getAllLinksSortedByName(rightDir)
259+
assert.NoError(t, err)
260+
rightLinks = append(rightLinks, missingEntries...)
261+
sortLinksByName(rightLinks)
262+
263+
assert.Equal(t, len(leftLinks), len(rightLinks))
264+
265+
for i, leftLink := range leftLinks {
266+
assert.Equal(t, leftLink, rightLinks[i]) // FIXME: Can we just compare the entire struct?
267+
}
268+
}
269+
270+
func getAllLinksSortedByName(d Directory) ([]*ipld.Link, error) {
271+
entries, err := d.Links(context.Background())
272+
if err != nil {
273+
return nil, err
274+
}
275+
sortLinksByName(entries)
276+
return entries, nil
277+
}
278+
279+
func sortLinksByName(l []*ipld.Link) {
280+
sort.SliceStable(l, func(i, j int) bool {
281+
return strings.Compare(l[i].Name, l[j].Name) == -1 // FIXME: Is this correct?
282+
})
283+
}
284+
285+
func copyDir(t *testing.T, d Directory) Directory {
286+
dirNode, err := d.GetNode()
287+
assert.NoError(t, err)
288+
// Extract the DAG service from the directory (i.e., its link entries saved
289+
// in it). This is not exposed in the interface and we won't change that now.
290+
// FIXME: Still, this isn't nice.
291+
var ds ipld.DAGService
292+
switch v := d.(type) {
293+
case *BasicDirectory:
294+
ds = v.dserv
295+
case *HAMTDirectory:
296+
ds = v.dserv
297+
case *UpgradeableDirectory:
298+
ds = v.getDagService()
299+
default:
300+
panic("unknown directory type")
301+
}
302+
copiedDir, err := NewDirectoryFromNode(ds, dirNode)
303+
assert.NoError(t, err)
304+
return copiedDir
232305
}
233306

234307
func TestDirBuilder(t *testing.T) {

0 commit comments

Comments
 (0)