Skip to content

Commit

Permalink
fix for #1008 and other pinning fixes
Browse files Browse the repository at this point in the history
This commit adds a new set of sharness tests for pinning, and addresses
bugs that were pointed out by said tests.

test/sharness: added more pinning tests

Pinning is currently broken. See issue #1051. This commit introduces
a few more pinning tests. These are by no means exhaustive, but
definitely surface the present problems going on. I believe these
tests are correct, but not sure. Pushing them as failing so that
pinning is fixed in this PR.

make pinning and merkledag.Get take contexts

improve 'add' commands usage of pinning

FIXUP: fix 'pin lists look good'

ipfs-pin-stat simple script to help check pinning

This is a simple shell script to help check pinning.

We ought to strive towards making adding commands this easy.
The http api is great and powerful, but our setup right now
gets in the way. Perhaps we can clean up that area.

updated t0081-repo-pinning

- fixed a couple bugs with the tests
- made it a bit clearer (still a lot going on)
- the remaining tests are correct and highlight a problem with
  pinning. Namely, that recursive pinning is buggy. At least:
  towards the end of the test, $HASH_DIR4 and $HASH_FILE4 should
  be pinned indirectly, but they're not. And thus get gc-ed out.
  There may be other problems too.

cc @whyrusleeping

fix grep params for context deadline check

fix bugs in pin and pin tests

check for block local before checking recursive pin
  • Loading branch information
whyrusleeping committed Apr 17, 2015
1 parent 2406334 commit 3cf18e8
Show file tree
Hide file tree
Showing 33 changed files with 556 additions and 99 deletions.
2 changes: 1 addition & 1 deletion cmd/ipfs/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func addDefaultAssets(out io.Writer, repoRoot string) error {
return err
}

if err := nd.Pinning.Pin(dir, true); err != nil {
if err := nd.Pinning.Pin(ctx, dir, true); err != nil {
return err
}

Expand Down
33 changes: 17 additions & 16 deletions core/commands/add.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
package commands

import (
"errors"
"fmt"
"io"
"path"
"strings"

"github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb"
context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

cmds "github.com/ipfs/go-ipfs/commands"
files "github.com/ipfs/go-ipfs/commands/files"
core "github.com/ipfs/go-ipfs/core"
coreunix "github.com/ipfs/go-ipfs/core/coreunix"
importer "github.com/ipfs/go-ipfs/importer"
"github.com/ipfs/go-ipfs/importer/chunk"
dag "github.com/ipfs/go-ipfs/merkledag"
pinning "github.com/ipfs/go-ipfs/pin"
ft "github.com/ipfs/go-ipfs/unixfs"
u "github.com/ipfs/go-ipfs/util"
"github.com/ipfs/go-ipfs/util/debugerror"

"github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb"
)

// Error indicating the max depth has been exceded.
Expand Down Expand Up @@ -105,7 +104,19 @@ remains to be implemented.
return
}

_, err = addFile(n, file, outChan, progress, wrap)
rootnd, err := addFile(n, file, outChan, progress, wrap)
if err != nil {
res.SetError(debugerror.Wrap(err), cmds.ErrNormal)
return
}

err = n.Pinning.Pin(context.Background(), rootnd, true)
if err != nil {
res.SetError(debugerror.Wrap(err), cmds.ErrNormal)
return
}

err = n.Pinning.Flush()
if err != nil {
res.SetError(debugerror.Wrap(err), cmds.ErrNormal)
return
Expand Down Expand Up @@ -200,15 +211,10 @@ remains to be implemented.
}

func add(n *core.IpfsNode, readers []io.Reader) ([]*dag.Node, error) {
mp, ok := n.Pinning.(pinning.ManualPinner)
if !ok {
return nil, errors.New("invalid pinner type! expected manual pinner")
}

dagnodes := make([]*dag.Node, 0)

for _, reader := range readers {
node, err := importer.BuildDagFromReader(reader, n.DAG, mp, chunk.DefaultSplitter)
node, err := importer.BuildDagFromReader(reader, n.DAG, nil, chunk.DefaultSplitter)
if err != nil {
return nil, err
}
Expand All @@ -229,11 +235,6 @@ func addNode(n *core.IpfsNode, node *dag.Node) error {
return err
}

err = n.Pinning.Pin(node, true) // ensure we keep it
if err != nil {
return err
}

return nil
}

Expand Down
7 changes: 6 additions & 1 deletion core/commands/ls.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"fmt"
"io"
"text/tabwriter"
"time"

context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

cmds "github.com/ipfs/go-ipfs/commands"
merkledag "github.com/ipfs/go-ipfs/merkledag"
Expand Down Expand Up @@ -77,7 +80,9 @@ it contains, with the following format:
Links: make([]LsLink, len(dagnode.Links)),
}
for j, link := range dagnode.Links {
link.Node, err = link.GetNode(node.DAG)
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
link.Node, err = link.GetNode(ctx, node.DAG)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
Expand Down
52 changes: 45 additions & 7 deletions core/commands/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,14 @@ Use --type=<type> to specify the type of pinned keys to list. Valid values are:
* "indirect": pinned indirectly by an ancestor (like a refcount)
* "all"
To see the ref count on indirect pins, pass the -count option flag.
Defaults to "direct".
`,
},

Options: []cmds.Option{
cmds.StringOption("type", "t", "The type of pinned keys to list. Can be \"direct\", \"indirect\", \"recursive\", or \"all\". Defaults to \"direct\""),
cmds.BoolOption("count", "n", "Show refcount when listing indirect pins"),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.Context().GetNode()
Expand All @@ -195,21 +197,57 @@ Defaults to "direct".
res.SetError(err, cmds.ErrClient)
}

keys := make([]u.Key, 0)
keys := make(map[string]int)
if typeStr == "direct" || typeStr == "all" {
keys = append(keys, n.Pinning.DirectKeys()...)
for _, k := range n.Pinning.DirectKeys() {
keys[k.B58String()] = 1
}
}
if typeStr == "indirect" || typeStr == "all" {
keys = append(keys, n.Pinning.IndirectKeys()...)
for k, v := range n.Pinning.IndirectKeys() {
keys[k.B58String()] = v
}
}
if typeStr == "recursive" || typeStr == "all" {
keys = append(keys, n.Pinning.RecursiveKeys()...)
for _, k := range n.Pinning.RecursiveKeys() {
keys[k.B58String()] = 1
}
}

res.SetOutput(&KeyList{Keys: keys})
res.SetOutput(&RefKeyList{Keys: keys})
},
Type: KeyList{},
Type: RefKeyList{},
Marshalers: cmds.MarshalerMap{
cmds.Text: KeyListTextMarshaler,
cmds.Text: func(res cmds.Response) (io.Reader, error) {
typeStr, _, err := res.Request().Option("type").String()
if err != nil {
return nil, err
}

count, _, err := res.Request().Option("count").Bool()
if err != nil {
return nil, err
}

keys, ok := res.Output().(*RefKeyList)
if !ok {
return nil, u.ErrCast()
}
out := new(bytes.Buffer)
if typeStr == "indirect" && count {
for k, v := range keys.Keys {
fmt.Fprintf(out, "%s %d\n", k, v)
}
} else {
for k, _ := range keys.Keys {
fmt.Fprintf(out, "%s\n", k)
}
}
return out, nil
},
},
}

type RefKeyList struct {
Keys map[string]int
}
8 changes: 6 additions & 2 deletions core/corehttp/gateway_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
return
}

rootnd, err := i.node.Resolver.DAG.Get(u.Key(h))
tctx, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
rootnd, err := i.node.Resolver.DAG.Get(tctx, u.Key(h))
if err != nil {
webError(w, "Could not resolve root object", err, http.StatusBadRequest)
return
Expand Down Expand Up @@ -414,7 +416,9 @@ func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
return
}

rootnd, err := i.node.Resolver.DAG.Get(u.Key(h))
tctx, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
rootnd, err := i.node.Resolver.DAG.Get(tctx, u.Key(h))
if err != nil {
webError(w, "Could not resolve root object", err, http.StatusBadRequest)
return
Expand Down
12 changes: 10 additions & 2 deletions core/corerepo/pinning.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package corerepo

import (
"fmt"
"time"

context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

"github.com/ipfs/go-ipfs/core"
"github.com/ipfs/go-ipfs/merkledag"
Expand All @@ -27,7 +30,9 @@ func Pin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) {
return nil, err
}

err = n.Pinning.Pin(dagnode, recursive)
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
err = n.Pinning.Pin(ctx, dagnode, recursive)
if err != nil {
return nil, fmt.Errorf("pin: %s", err)
}
Expand Down Expand Up @@ -56,7 +61,10 @@ func Unpin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) {
var unpinned []u.Key
for _, dagnode := range dagnodes {
k, _ := dagnode.Key()
err := n.Pinning.Unpin(k, recursive)

ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
err := n.Pinning.Unpin(ctx, k, recursive)
if err != nil {
return nil, err
}
Expand Down
7 changes: 6 additions & 1 deletion core/coreunix/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"io/ioutil"
"os"
gopath "path"
"time"

context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

"github.com/ipfs/go-ipfs/commands/files"
core "github.com/ipfs/go-ipfs/core"
Expand Down Expand Up @@ -108,7 +111,9 @@ func addNode(n *core.IpfsNode, node *merkledag.Node) error {
if err != nil {
return err
}
err = n.Pinning.Pin(node, true) // ensure we keep it
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
err = n.Pinning.Pin(ctx, node, true) // ensure we keep it
if err != nil {
return err
}
Expand Down
14 changes: 12 additions & 2 deletions core/coreunix/metadata.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package coreunix

import (
"time"

context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

core "github.com/ipfs/go-ipfs/core"
dag "github.com/ipfs/go-ipfs/merkledag"
ft "github.com/ipfs/go-ipfs/unixfs"
Expand All @@ -9,7 +13,10 @@ import (

func AddMetadataTo(n *core.IpfsNode, key string, m *ft.Metadata) (string, error) {
ukey := u.B58KeyDecode(key)
nd, err := n.DAG.Get(ukey)

ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
nd, err := n.DAG.Get(ctx, ukey)
if err != nil {
return "", err
}
Expand All @@ -36,7 +43,10 @@ func AddMetadataTo(n *core.IpfsNode, key string, m *ft.Metadata) (string, error)

func Metadata(n *core.IpfsNode, key string) (*ft.Metadata, error) {
ukey := u.B58KeyDecode(key)
nd, err := n.DAG.Get(ukey)

ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()
nd, err := n.DAG.Get(ctx, ukey)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion core/coreunix/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
ds "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore"
dssync "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync"
context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

bstore "github.com/ipfs/go-ipfs/blocks/blockstore"
bserv "github.com/ipfs/go-ipfs/blockservice"
core "github.com/ipfs/go-ipfs/core"
Expand Down Expand Up @@ -65,7 +66,7 @@ func TestMetadata(t *testing.T) {
t.Fatalf("something went wrong in conversion: '%s' != '%s'", rec.MimeType, m.MimeType)
}

retnode, err := ds.Get(u.B58KeyDecode(mdk))
retnode, err := ds.Get(context.Background(), u.B58KeyDecode(mdk))
if err != nil {
t.Fatal(err)
}
Expand Down
9 changes: 8 additions & 1 deletion fuse/ipns/common.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package ipns

import (
"time"

context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"

"github.com/ipfs/go-ipfs/core"
mdag "github.com/ipfs/go-ipfs/merkledag"
nsys "github.com/ipfs/go-ipfs/namesys"
Expand All @@ -17,7 +21,10 @@ func InitializeKeyspace(n *core.IpfsNode, key ci.PrivKey) error {
return err
}

err = n.Pinning.Pin(emptyDir, false)
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()

err = n.Pinning.Pin(ctx, emptyDir, false)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion fuse/readonly/ipfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func getPaths(t *testing.T, ipfs *core.IpfsNode, name string, n *dag.Node) []str
}
var out []string
for _, lnk := range n.Links {
child, err := lnk.GetNode(ipfs.DAG)
child, err := lnk.GetNode(ipfs.Context(), ipfs.DAG)
if err != nil {
t.Fatal(err)
}
Expand Down
7 changes: 6 additions & 1 deletion importer/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package helpers

import (
"fmt"
"time"

"github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"
chunk "github.com/ipfs/go-ipfs/importer/chunk"
dag "github.com/ipfs/go-ipfs/merkledag"
"github.com/ipfs/go-ipfs/pin"
Expand Down Expand Up @@ -76,7 +78,10 @@ func (n *UnixfsNode) NumChildren() int {
}

func (n *UnixfsNode) GetChild(i int, ds dag.DAGService) (*UnixfsNode, error) {
nd, err := n.node.Links[i].GetNode(ds)
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
defer cancel()

nd, err := n.node.Links[i].GetNode(ctx, ds)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion importer/trickle/trickle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ func printDag(nd *merkledag.Node, ds merkledag.DAGService, indent int) {
fmt.Println()
}
for _, lnk := range nd.Links {
child, err := lnk.GetNode(ds)
child, err := lnk.GetNode(context.Background(), ds)
if err != nil {
panic(err)
}
Expand Down
Loading

0 comments on commit 3cf18e8

Please sign in to comment.