From 8e15b982abdfadd8122029cac23eae858da23268 Mon Sep 17 00:00:00 2001 From: Iaroslav Gridin Date: Sun, 15 Jan 2017 19:13:46 +0200 Subject: [PATCH] Make pin rm not try to find pinned ancestors by default Also make a switch to pin rm to allow old behavior. Trying to look up pins which interfere with requested unpin can be an extremely costly operation and typically is not required by user. License: MIT Signed-off-by: Iaroslav Gridin --- core/commands/pin.go | 9 ++++++++- core/corerepo/pinning.go | 4 ++-- core/coreunix/add.go | 2 +- pin/pin.go | 37 ++++++++++++++++++++++++------------- pin/pin_test.go | 2 +- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/core/commands/pin.go b/core/commands/pin.go index 92cb9d6c287..d804b2e9fa0 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -108,6 +108,7 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.) }, Options: []cmds.Option{ cmds.BoolOption("recursive", "r", "Recursively unpin the object linked to by the specified object(s).").Default(true), + cmds.BoolOption("explain", "e", "Check for other pinned objects which could cause specified object(s) to be indirectly pinned").Default(false), }, Type: PinOutput{}, Run: func(req cmds.Request, res cmds.Response) { @@ -124,7 +125,13 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.) return } - removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive) + explain, _, err := req.Option("explain").Bool() + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + + removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive, explain) if err != nil { res.SetError(err, cmds.ErrNormal) return diff --git a/core/corerepo/pinning.go b/core/corerepo/pinning.go index f7dc324ef08..2e35050fa0e 100644 --- a/core/corerepo/pinning.go +++ b/core/corerepo/pinning.go @@ -60,7 +60,7 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) return out, nil } -func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) ([]*cid.Cid, error) { +func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool, explain bool) ([]*cid.Cid, error) { var unpinned []*cid.Cid for _, p := range paths { @@ -76,7 +76,7 @@ func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool ctx, cancel := context.WithCancel(ctx) defer cancel() - err = n.Pinning.Unpin(ctx, k, recursive) + err = n.Pinning.Unpin(ctx, k, recursive, explain) if err != nil { return nil, err } diff --git a/core/coreunix/add.go b/core/coreunix/add.go index 18b8aa6c9b9..23fb72d5b0e 100644 --- a/core/coreunix/add.go +++ b/core/coreunix/add.go @@ -173,7 +173,7 @@ func (adder *Adder) PinRoot() error { } if adder.tempRoot != nil { - err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true) + err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true, false) if err != nil { return err } diff --git a/pin/pin.go b/pin/pin.go index baf0d595856..df995f84dff 100644 --- a/pin/pin.go +++ b/pin/pin.go @@ -84,7 +84,7 @@ type Pinner interface { IsPinned(*cid.Cid) (string, bool, error) IsPinnedWithType(*cid.Cid, PinMode) (string, bool, error) Pin(context.Context, node.Node, bool) error - Unpin(context.Context, *cid.Cid, bool) error + Unpin(context.Context, *cid.Cid, bool, bool) error // Check if a set of keys are pinned, more efficient than // calling IsPinned for each key @@ -200,29 +200,40 @@ func (p *pinner) Pin(ctx context.Context, node node.Node, recurse bool) error { var ErrNotPinned = fmt.Errorf("not pinned") // Unpin a given key -func (p *pinner) Unpin(ctx context.Context, c *cid.Cid, recursive bool) error { +func (p *pinner) Unpin(ctx context.Context, c *cid.Cid, recursive bool, explain bool) error { p.lock.Lock() defer p.lock.Unlock() - reason, pinned, err := p.isPinnedWithType(c, Any) + var pinMode PinMode + if recursive { + pinMode = Recursive + } else { + pinMode = Direct + } + _, pinned, err := p.isPinnedWithType(c, pinMode) if err != nil { return err } - if !pinned { - return ErrNotPinned - } - switch reason { - case "recursive": + if pinned { if recursive { p.recursePin.Remove(c) return nil } else { + p.directPin.Remove(c) + return nil + } + } else if explain { + reason, _, err := p.isPinnedWithType(c, Any) + if err != nil { + return err + } + switch reason { + case "recursive": return fmt.Errorf("%s is pinned recursively", c) + default: + return fmt.Errorf("%s is pinned indirectly under %s", c, reason) } - case "direct": - p.directPin.Remove(c) - return nil - default: - return fmt.Errorf("%s is pinned indirectly under %s", c, reason) + } else { + return ErrNotPinned } } diff --git a/pin/pin_test.go b/pin/pin_test.go index 90bbc52139b..31819f2689c 100644 --- a/pin/pin_test.go +++ b/pin/pin_test.go @@ -123,7 +123,7 @@ func TestPinnerBasic(t *testing.T) { assertPinned(t, p, dk, "pinned node not found.") // Test recursive unpin - err = p.Unpin(ctx, dk, true) + err = p.Unpin(ctx, dk, true, false) if err != nil { t.Fatal(err) }