From 8e780d2304243acaf455cf22ba46395685e2063e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 3 Dec 2017 18:29:50 -0800 Subject: [PATCH 1/5] don't create a new context per pin This came from an old commit that used a TODO context. Now that we have a real context, use that. License: MIT Signed-off-by: Steven Allen --- core/corerepo/pinning.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/corerepo/pinning.go b/core/corerepo/pinning.go index e10632fe6..7267f5d41 100644 --- a/core/corerepo/pinning.go +++ b/core/corerepo/pinning.go @@ -50,8 +50,6 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) for _, dagnode := range dagnodes { c := dagnode.Cid() - ctx, cancel := context.WithCancel(ctx) - defer cancel() err := n.Pinning.Pin(ctx, dagnode, recursive) if err != nil { return nil, fmt.Errorf("pin: %s", err) @@ -86,8 +84,6 @@ func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool return nil, err } - ctx, cancel := context.WithCancel(ctx) - defer cancel() err = n.Pinning.Unpin(ctx, k, recursive) if err != nil { return nil, err From 953b610470044bd332522b346e4881b8957c2050 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 3 Dec 2017 18:53:02 -0800 Subject: [PATCH 2/5] add node to dagserv under lock Otherwise, we could run a GC between adding and pinning. License: MIT Signed-off-by: Steven Allen --- pin/pin.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pin/pin.go b/pin/pin.go index 4faaea6f7..0e55963b3 100644 --- a/pin/pin.go +++ b/pin/pin.go @@ -172,7 +172,10 @@ func NewPinner(dstore ds.Datastore, serv, internal mdag.DAGService) Pinner { func (p *pinner) Pin(ctx context.Context, node node.Node, recurse bool) error { p.lock.Lock() defer p.lock.Unlock() - c := node.Cid() + c, err := p.dserv.Add(node) + if err != nil { + return err + } if recurse { if p.recursePin.Has(c) { From 924b2a0a34342e56738d3c2727ac630d81a2d58e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 3 Dec 2017 18:58:05 -0800 Subject: [PATCH 3/5] don't add nodes to DAG twice. Now that we add nodes to the DAG when pinning, don't bother adding them twice. License: MIT Signed-off-by: Steven Allen --- assets/assets.go | 7 +------ cmd/ipfs/init.go | 2 +- fuse/ipns/common.go | 12 ++++-------- namesys/publisher.go | 11 +++-------- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/assets/assets.go b/assets/assets.go index 8f6a8521f..a98b83f22 100644 --- a/assets/assets.go +++ b/assets/assets.go @@ -80,11 +80,6 @@ func addAssetList(nd *core.IpfsNode, l []string) (*cid.Cid, error) { return nil, err } - dcid, err := nd.DAG.Add(dir) - if err != nil { - return nil, fmt.Errorf("assets: DAG.Add(dir) failed: %s", err) - } - if err := nd.Pinning.Pin(nd.Context(), dir, true); err != nil { return nil, fmt.Errorf("assets: Pinning on init-docu failed: %s", err) } @@ -93,5 +88,5 @@ func addAssetList(nd *core.IpfsNode, l []string) (*cid.Cid, error) { return nil, fmt.Errorf("assets: Pinning flush failed: %s", err) } - return dcid, nil + return dir.Cid(), nil } diff --git a/cmd/ipfs/init.go b/cmd/ipfs/init.go index ed95a5225..b45101e58 100644 --- a/cmd/ipfs/init.go +++ b/cmd/ipfs/init.go @@ -260,5 +260,5 @@ func initializeIpnsKeyspace(repoRoot string) error { return err } - return namesys.InitializeKeyspace(ctx, nd.DAG, nd.Namesys, nd.Pinning, nd.PrivateKey) + return namesys.InitializeKeyspace(ctx, nd.Namesys, nd.Pinning, nd.PrivateKey) } diff --git a/fuse/ipns/common.go b/fuse/ipns/common.go index d0d7ba247..4c9b0d341 100644 --- a/fuse/ipns/common.go +++ b/fuse/ipns/common.go @@ -13,16 +13,12 @@ import ( // InitializeKeyspace sets the ipns record for the given key to // point to an empty directory. func InitializeKeyspace(n *core.IpfsNode, key ci.PrivKey) error { - emptyDir := ft.EmptyDirNode() - nodek, err := n.DAG.Add(emptyDir) - if err != nil { - return err - } - ctx, cancel := context.WithCancel(n.Context()) defer cancel() - err = n.Pinning.Pin(ctx, emptyDir, false) + emptyDir := ft.EmptyDirNode() + + err := n.Pinning.Pin(ctx, emptyDir, false) if err != nil { return err } @@ -34,5 +30,5 @@ func InitializeKeyspace(n *core.IpfsNode, key ci.PrivKey) error { pub := nsys.NewRoutingPublisher(n.Routing, n.Repo.Datastore()) - return pub.Publish(ctx, key, path.FromCid(nodek)) + return pub.Publish(ctx, key, path.FromCid(emptyDir.Cid())) } diff --git a/namesys/publisher.go b/namesys/publisher.go index 03feb204a..e98c23916 100644 --- a/namesys/publisher.go +++ b/namesys/publisher.go @@ -7,7 +7,6 @@ import ( "fmt" "time" - dag "github.com/ipfs/go-ipfs/merkledag" pb "github.com/ipfs/go-ipfs/namesys/pb" path "github.com/ipfs/go-ipfs/path" pin "github.com/ipfs/go-ipfs/pin" @@ -321,16 +320,12 @@ func ValidateIpnsRecord(k string, val []byte) error { // InitializeKeyspace sets the ipns record for the given key to // point to an empty directory. // TODO: this doesnt feel like it belongs here -func InitializeKeyspace(ctx context.Context, ds dag.DAGService, pub Publisher, pins pin.Pinner, key ci.PrivKey) error { +func InitializeKeyspace(ctx context.Context, pub Publisher, pins pin.Pinner, key ci.PrivKey) error { emptyDir := ft.EmptyDirNode() - nodek, err := ds.Add(emptyDir) - if err != nil { - return err - } // pin recursively because this might already be pinned // and doing a direct pin would throw an error in that case - err = pins.Pin(ctx, emptyDir, true) + err := pins.Pin(ctx, emptyDir, true) if err != nil { return err } @@ -340,7 +335,7 @@ func InitializeKeyspace(ctx context.Context, ds dag.DAGService, pub Publisher, p return err } - return pub.Publish(ctx, key, path.FromCid(nodek)) + return pub.Publish(ctx, key, path.FromCid(emptyDir.Cid())) } func IpnsKeysForID(id peer.ID) (name, ipns string) { From 498ee0dc0b0e0f1be16ff083a08d3c691ec90efc Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 3 Dec 2017 19:03:04 -0800 Subject: [PATCH 4/5] resolve and pin in one step instead of resolving all the pins first and then pinning, pin after resolving each pin. This: 1. Avoids storing all the nodes in memory. 2. Avoids not showing pin progress. fixes #4122 License: MIT Signed-off-by: Steven Allen --- core/corerepo/pinning.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/core/corerepo/pinning.go b/core/corerepo/pinning.go index 7267f5d41..e49eb210a 100644 --- a/core/corerepo/pinning.go +++ b/core/corerepo/pinning.go @@ -22,18 +22,17 @@ import ( uio "github.com/ipfs/go-ipfs/unixfs/io" cid "gx/ipfs/QmNp85zy9RLrQ5oQD4hPyS39ezrrXpcaa7R4Y9kxdWQLLQ/go-cid" - node "gx/ipfs/QmPN7cwmpcc4DWXb4KTB9dNAJgjuPY69h3npsMfhRrQL9c/go-ipld-format" ) func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) ([]*cid.Cid, error) { - dagnodes := make([]node.Node, 0) + out := make([]*cid.Cid, len(paths)) r := &path.Resolver{ DAG: n.DAG, ResolveOnce: uio.ResolveUnixfsOnce, } - for _, fpath := range paths { + for i, fpath := range paths { p, err := path.ParsePath(fpath) if err != nil { return nil, err @@ -43,18 +42,11 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) if err != nil { return nil, fmt.Errorf("pin: %s", err) } - dagnodes = append(dagnodes, dagnode) - } - - var out []*cid.Cid - for _, dagnode := range dagnodes { - c := dagnode.Cid() - - err := n.Pinning.Pin(ctx, dagnode, recursive) + err = n.Pinning.Pin(ctx, dagnode, recursive) if err != nil { return nil, fmt.Errorf("pin: %s", err) } - out = append(out, c) + out[i] = dagnode.Cid() } err := n.Pinning.Flush() From 9a335cee133391f211e8b4433bb965d802abcb04 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 3 Dec 2017 19:05:05 -0800 Subject: [PATCH 5/5] fewer allocations on unpin License: MIT Signed-off-by: Steven Allen --- core/corerepo/pinning.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/corerepo/pinning.go b/core/corerepo/pinning.go index e49eb210a..f00fa2392 100644 --- a/core/corerepo/pinning.go +++ b/core/corerepo/pinning.go @@ -58,14 +58,14 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) } func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) ([]*cid.Cid, error) { - var unpinned []*cid.Cid + unpinned := make([]*cid.Cid, len(paths)) r := &path.Resolver{ DAG: n.DAG, ResolveOnce: uio.ResolveUnixfsOnce, } - for _, p := range paths { + for i, p := range paths { p, err := path.ParsePath(p) if err != nil { return nil, err @@ -80,7 +80,7 @@ func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool if err != nil { return nil, err } - unpinned = append(unpinned, k) + unpinned[i] = k } err := n.Pinning.Flush()