From cb3bda78b3a4afb5cc5e7c2c70a651a113aaaae4 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 29 Sep 2016 12:35:40 -0700 Subject: [PATCH] fix bug in pinsets and add a stress test for the scenario License: MIT Signed-off-by: Jeromy --- pin/set.go | 2 +- pin/set_test.go | 130 +++++++++++++++++------------------------------- 2 files changed, 47 insertions(+), 85 deletions(-) diff --git a/pin/set.go b/pin/set.go index 7257ccaecb9..6a72d71894d 100644 --- a/pin/set.go +++ b/pin/set.go @@ -143,7 +143,7 @@ func storeItems(ctx context.Context, dag merkledag.DAGService, estimatedLen uint if !ok { break } - h := hash(seed, k) + h := hash(seed, k) % defaultFanout hashed[h] = append(hashed[h], item{k, data}) } for h, items := range hashed { diff --git a/pin/set_test.go b/pin/set_test.go index e4c8bd4de71..75cf1f348cf 100644 --- a/pin/set_test.go +++ b/pin/set_test.go @@ -1,103 +1,65 @@ package pin import ( + "context" + "fmt" + "os" "testing" - "testing/quick" - "github.com/ipfs/go-ipfs/blocks/blockstore" - "github.com/ipfs/go-ipfs/blocks/key" - "github.com/ipfs/go-ipfs/blockservice" - "github.com/ipfs/go-ipfs/exchange/offline" - "github.com/ipfs/go-ipfs/merkledag" - "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore" - dssync "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/sync" - mh "gx/ipfs/QmYf7ng2hG5XBtJA3tN34DQ2GUN5HNksEw1rLDkmr6vGku/go-multihash" - u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util" - "gx/ipfs/QmZy2y8t9zQH2a1b8q2ZSLKp17ATuJoCNxxyMFG5qFExpt/go-net/context" + key "github.com/ipfs/go-ipfs/blocks/key" + dag "github.com/ipfs/go-ipfs/merkledag" + mdtest "github.com/ipfs/go-ipfs/merkledag/test" ) -func ignoreKeys(key.Key) {} +func ignoreKey(_ key.Key) {} -func copyMap(m map[key.Key]uint16) map[key.Key]uint64 { - c := make(map[key.Key]uint64, len(m)) - for k, v := range m { - c[k] = uint64(v) - } - return c -} +func TestSet(t *testing.T) { + ds := mdtest.Mock() + limit := 10000 // 10000 reproduces the pinloss issue fairly reliably -func TestMultisetRoundtrip(t *testing.T) { - dstore := dssync.MutexWrap(datastore.NewMapDatastore()) - bstore := blockstore.NewBlockstore(dstore) - bserv := blockservice.New(bstore, offline.Exchange(bstore)) - dag := merkledag.NewDAGService(bserv) - - fn := func(m map[key.Key]uint16) bool { - // Convert invalid multihash from input to valid ones - for k, v := range m { - if _, err := mh.Cast([]byte(k)); err != nil { - delete(m, k) - m[key.Key(u.Hash([]byte(k)))] = v - } + if os.Getenv("STRESS_IT_OUT_YO") != "" { + limit = 10000000 + } + var inputs []key.Key + for i := 0; i < limit; i++ { + c, err := ds.Add(dag.NodeWithData([]byte(fmt.Sprint(i)))) + if err != nil { + t.Fatal(err) } - // Generate a smaller range for refcounts than full uint64, as - // otherwise this just becomes overly cpu heavy, splitting it - // out into too many items. That means we need to convert to - // the right kind of map. As storeMultiset mutates the map as - // part of its bookkeeping, this is actually good. - refcounts := copyMap(m) + inputs = append(inputs, c) + } - ctx := context.Background() - n, err := storeMultiset(ctx, dag, refcounts, ignoreKeys) - if err != nil { - t.Fatalf("storing multiset: %v", err) - } + out, err := storeSet(context.Background(), ds, inputs, ignoreKey) + if err != nil { + t.Fatal(err) + } - // Check that the node n is in the DAG - k, err := n.Key() - if err != nil { - t.Fatalf("Could not get key: %v", err) - } - _, err = dag.Get(ctx, k) - if err != nil { - t.Fatalf("Could not get node: %v", err) - } + // weird wrapper node because loadSet expects us to pass an + // object pointing to multiple named sets + setroot := &dag.Node{} + err = setroot.AddNodeLinkClean("foo", out) + if err != nil { + t.Fatal(err) + } - root := &merkledag.Node{} - const linkName = "dummylink" - if err := root.AddNodeLink(linkName, n); err != nil { - t.Fatalf("adding link to root node: %v", err) - } + outset, err := loadSet(context.Background(), ds, setroot, "foo", ignoreKey) + if err != nil { + t.Fatal(err) + } - roundtrip, err := loadMultiset(ctx, dag, root, linkName, ignoreKeys) - if err != nil { - t.Fatalf("loading multiset: %v", err) - } + if len(outset) != limit { + t.Fatal("got wrong number", len(outset), limit) + } - orig := copyMap(m) - success := true - for k, want := range orig { - if got, ok := roundtrip[k]; ok { - if got != want { - success = false - t.Logf("refcount changed: %v -> %v for %q", want, got, k) - } - delete(orig, k) - delete(roundtrip, k) - } - } - for k, v := range orig { - success = false - t.Logf("refcount missing: %v for %q", v, k) - } - for k, v := range roundtrip { - success = false - t.Logf("refcount extra: %v for %q", v, k) - } - return success + seen := key.NewKeySet() + for _, c := range outset { + seen.Add(c) } - if err := quick.Check(fn, nil); err != nil { - t.Fatal(err) + + for _, c := range inputs { + if !seen.Has(c) { + t.Fatalf("expected to have %s, didnt find it") + } } }