From cb3bda78b3a4afb5cc5e7c2c70a651a113aaaae4 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 29 Sep 2016 12:35:40 -0700 Subject: [PATCH 1/2] 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 | 136 +++++++++++++++++------------------------------- 2 files changed, 50 insertions(+), 88 deletions(-) diff --git a/pin/set.go b/pin/set.go index 7257ccaec..6a72d7189 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 e4c8bd4de..75cf1f348 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) +func TestSet(t *testing.T) { + ds := mdtest.Mock() + limit := 10000 // 10000 reproduces the pinloss issue fairly reliably + + if os.Getenv("STRESS_IT_OUT_YO") != "" { + limit = 10000000 } - return c -} - -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 - } - } - - // 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) - - ctx := context.Background() - n, err := storeMultiset(ctx, dag, refcounts, ignoreKeys) + var inputs []key.Key + for i := 0; i < limit; i++ { + c, err := ds.Add(dag.NodeWithData([]byte(fmt.Sprint(i)))) if err != nil { - t.Fatalf("storing multiset: %v", err) + 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) - } - - root := &merkledag.Node{} - const linkName = "dummylink" - if err := root.AddNodeLink(linkName, n); err != nil { - t.Fatalf("adding link to root node: %v", err) - } - - roundtrip, err := loadMultiset(ctx, dag, root, linkName, ignoreKeys) - if err != nil { - t.Fatalf("loading multiset: %v", err) - } - - 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 + inputs = append(inputs, c) } - if err := quick.Check(fn, nil); err != nil { + + out, err := storeSet(context.Background(), ds, inputs, ignoreKey) + if err != nil { t.Fatal(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) + } + + outset, err := loadSet(context.Background(), ds, setroot, "foo", ignoreKey) + if err != nil { + t.Fatal(err) + } + + if len(outset) != limit { + t.Fatal("got wrong number", len(outset), limit) + } + + seen := key.NewKeySet() + for _, c := range outset { + seen.Add(c) + } + + for _, c := range inputs { + if !seen.Has(c) { + t.Fatalf("expected to have %s, didnt find it") + } + } } From d905d485192616abaea25f7e721062a9e1093ab9 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 10 Oct 2016 07:39:03 -0700 Subject: [PATCH 2/2] Ipfs v0.4.4 - Hotfix pinset release License: MIT Signed-off-by: Jeromy --- CHANGELOG.md | 4 ++++ package.json | 2 +- repo/config/version.go | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 138f1cde6..5299e3964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # go-ipfs changelog +### 0.4.4 - 2016-10-10 + +This is a hotfix release that adds a patch for [the pinsets bug](https://github.com/ipfs/go-ipfs/pull/3273). + ### 0.4.3-rc4 - 2016-09-09 This release candidate fixes issues in Bitswap and the `ipfs add` command, and improves testing. diff --git a/package.json b/package.json index 4611c2578..c8f3a9fd1 100644 --- a/package.json +++ b/package.json @@ -201,5 +201,5 @@ "language": "go", "license": "MIT", "name": "go-ipfs", - "version": "0.4.3" + "version": "0.4.4" } diff --git a/repo/config/version.go b/repo/config/version.go index 8708139d3..2b2bfd4e0 100644 --- a/repo/config/version.go +++ b/repo/config/version.go @@ -4,6 +4,6 @@ package config var CurrentCommit string // CurrentVersionNumber is the current application's version literal -const CurrentVersionNumber = "0.4.3" +const CurrentVersionNumber = "0.4.4" const ApiVersion = "/go-ipfs/" + CurrentVersionNumber + "/"