Check for multiple pinned blocks in a single pass.
Provide a new method, Pinner.CheckIfPinned(), which will check if any of the arguments are pinned. Previously IsPinned would need to be called once for each block. The new method will speed up the checking of multiple pinned blocks from O(p*n) to O(p) (where p is the number of pinned blocks and n is the number of blocks to be check) Use the new method in "block rm". License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>
Kevin Atkinson committed
Aug 11, 2016 at 17:45 UTC
efeb789878c358093b2659894fa536185697d8e3
3 files changed
+117
-27
core/commands/block.go
+38
-21
@@ -228,7 +228,10 @@ It takes a list of base58 encoded multihashs to remove.
228
if ignorePins {
229
pinning = nil
230
}
231
- rmBlocks(n.Blockstore, pinning, outChan, keys)
231
+ err := rmBlocks(n.Blockstore, pinning, outChan, keys)
232
+ if err != nil {
233
+ outChan <- &RemovedBlock{Error: err.Error()}
234
+ }
235
}()
236
return
237
},
@@ -246,7 +249,10 @@ It takes a list of base58 encoded multihashs to remove.
249
someFailed := false
250
for out := range outChan {
251
o := out.(*RemovedBlock)
249
- if o.Error != "" {
252
+ if o.Hash == "" && o.Error != "" {
253
+ res.SetError(fmt.Errorf("aborted: %s", o.Error), cmds.ErrNormal)
254
+ return
255
+ } else if o.Error != "" {
256
someFailed = true
257
fmt.Fprintf(res.Stderr(), "cannot remove %s: %s\n", o.Hash, o.Error)
258
} else {
@@ -261,12 +267,12 @@ It takes a list of base58 encoded multihashs to remove.
267
}
268
269
type RemovedBlock struct {
264
- Hash string
270
+ Hash string `json:",omitempty"`
271
Error string `json:",omitempty"`
272
}
273
274
// pins may be nil
269
-func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) {
275
+func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) error {
276
var unlocker bs.Unlocker
277
defer func() {
278
if unlocker != nil {
@@ -278,33 +284,44 @@ func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, k
284
// Need to make sure that some operation that is
285
// finishing with a pin is ocurr simultaneously.
286
unlocker = blocks.GCLock()
281
- stillOkay = checkIfPinned(pins, keys, out)
287
+ var err error
288
+ stillOkay, err = checkIfPinned(pins, keys, out)
289
+ if err != nil {
290
+ return fmt.Errorf("pin check failed: %s", err)
291
+ }
292
}
293
for _, k := range stillOkay {
294
err := blocks.DeleteBlock(k)
295
if err != nil {
286
- out <- &RemovedBlock{ Hash: k.String(), Error: err.Error()}
296
+ out <- &RemovedBlock{Hash: k.String(), Error: err.Error()}
297
} else {
288
- out <- &RemovedBlock{ Hash: k.String() }
298
+ out <- &RemovedBlock{Hash: k.String()}
299
}
300
}
301
+ return nil
302
}
303
293
-func checkIfPinned(pins pin.Pinner, keys []key.Key, out chan<- interface{}) []key.Key {
304
+func checkIfPinned(pins pin.Pinner, keys []key.Key, out chan<- interface{}) ([]key.Key, error) {
305
stillOkay := make([]key.Key, 0, len(keys))
295
- for _, k := range keys {
296
- reason, pinned, err := pins.IsPinned(k)
297
- if err != nil {
298
- out <- &RemovedBlock {
299
- Hash: k.String(),
300
- Error: fmt.Sprintf("pin check failed %s", err.Error()) }
301
- } else if pinned {
302
- out <- &RemovedBlock {
303
- Hash: k.String(),
304
- Error: fmt.Sprintf("pinned via %s", reason) }
305
- } else {
306
- stillOkay = append(stillOkay, k)
306
+ res, err := pins.CheckIfPinned(keys...)
307
+ if err != nil {
308
+ return nil, err
309
+ }
310
+ for _, r := range res {
311
+ switch r.Mode {
312
+ case pin.NotPinned:
313
+ stillOkay = append(stillOkay, r.Key)
314
+ case pin.Indirect:
315
+ out <- &RemovedBlock{
316
+ Hash: r.Key.String(),
317
+ Error: fmt.Sprintf("pinned via %s", r.Via)}
318
+ default:
319
+ modeStr, _ := pin.PinModeToString(r.Mode)
320
+ out <- &RemovedBlock{
321
+ Hash: r.Key.String(),
322
+ Error: fmt.Sprintf("pinned: %s", modeStr)}
323
+
324
}
325
}
309
- return stillOkay
326
+ return stillOkay, nil
327
}
pin/pin.go
+74
@@ -75,6 +75,10 @@ type Pinner interface {
75
Pin(context.Context, *mdag.Node, bool) error
76
Unpin(context.Context, key.Key, bool) error
77
78
+ // Check if a set of keys are pinned, more efficient than
79
+ // calling IsPinned for each key
80
+ CheckIfPinned(keys ...key.Key) ([]Pinned, error)
81
+
82
// PinWithMode is for manually editing the pin structure. Use with
83
// care! If used improperly, garbage collection may not be
84
// successful.
@@ -90,6 +94,12 @@ type Pinner interface {
94
InternalPins() []key.Key
95
}
96
97
+type Pinned struct {
98
+ Key key.Key
99
+ Mode PinMode
100
+ Via key.Key
101
+}
102
+
103
// pinner implements the Pinner interface
104
type pinner struct {
105
lock sync.RWMutex
@@ -255,6 +265,70 @@ func (p *pinner) isPinnedWithType(k key.Key, mode PinMode) (string, bool, error)
265
return "", false, nil
266
}
267
268
+func (p *pinner) CheckIfPinned(keys ...key.Key) ([]Pinned, error) {
269
+ p.lock.RLock()
270
+ defer p.lock.RUnlock()
271
+ pinned := make([]Pinned, 0, len(keys))
272
+ toCheck := make(map[key.Key]struct{})
273
+
274
+ // First check for non-Indirect pins directly
275
+ for _, k := range keys {
276
+ if p.recursePin.HasKey(k) {
277
+ pinned = append(pinned, Pinned{Key: k, Mode: Recursive})
278
+ } else if p.directPin.HasKey(k) {
279
+ pinned = append(pinned, Pinned{Key: k, Mode: Direct})
280
+ } else if p.isInternalPin(k) {
281
+ pinned = append(pinned, Pinned{Key: k, Mode: Internal})
282
+ } else {
283
+ toCheck[k] = struct{}{}
284
+ }
285
+ }
286
+
287
+ // Now walk all recursive pins to check for indirect pins
288
+ var checkChildren func(key.Key, key.Key) error
289
+ checkChildren = func(rk key.Key, parentKey key.Key) error {
290
+ parent, err := p.dserv.Get(context.Background(), parentKey)
291
+ if err != nil {
292
+ return err
293
+ }
294
+ for _, lnk := range parent.Links {
295
+ k := key.Key(lnk.Hash)
296
+
297
+ if _, found := toCheck[k]; found {
298
+ pinned = append(pinned,
299
+ Pinned{Key: k, Mode: Indirect, Via: rk})
300
+ delete(toCheck, k)
301
+ }
302
+
303
+ err := checkChildren(rk, k)
304
+ if err != nil {
305
+ return err
306
+ }
307
+
308
+ if len(toCheck) == 0 {
309
+ return nil
310
+ }
311
+ }
312
+ return nil
313
+ }
314
+ for _, rk := range p.recursePin.GetKeys() {
315
+ err := checkChildren(rk, rk)
316
+ if err != nil {
317
+ return nil, err
318
+ }
319
+ if len(toCheck) == 0 {
320
+ break
321
+ }
322
+ }
323
+
324
+ // Anything left in toCheck is not pinned
325
+ for k, _ := range toCheck {
326
+ pinned = append(pinned, Pinned{Key: k, Mode: NotPinned})
327
+ }
328
+
329
+ return pinned, nil
330
+}
331
+
332
func (p *pinner) RemovePinWithMode(key key.Key, mode PinMode) {
333
p.lock.Lock()
334
defer p.lock.Unlock()
test/sharness/t0050-block.sh
+5
-6
@@ -71,7 +71,7 @@ test_expect_success "can't remove pinned block" '
71
'
72
73
test_expect_success "can't remove pinned block: output looks good" '
74
- grep -q "$DIRHASH: pinned via recursive" block_rm_err
74
+ grep -q "$DIRHASH: pinned: recursive" block_rm_err
75
'
76
77
test_expect_success "can't remove indirectly pinned block" '
@@ -79,7 +79,7 @@ test_expect_success "can't remove indirectly pinned block" '
79
'
80
81
test_expect_success "can't remove indirectly pinned block: output looks good" '
82
- grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err
82
+ grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err
83
'
84
85
test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" '
@@ -100,10 +100,9 @@ test_expect_success "multi-block 'ipfs block rm' succeeds" '
100
'
101
102
test_expect_success "multi-block 'ipfs block rm' output looks good" '
103
- echo "removed $FILE1HASH" > expected_rm &&
104
- echo "removed $FILE2HASH" >> expected_rm &&
105
- echo "removed $FILE3HASH" >> expected_rm &&
106
- test_cmp expected_rm actual_rm
103
+ grep -F -q "removed $FILE1HASH" actual_rm &&
104
+ grep -F -q "removed $FILE2HASH" actual_rm &&
105
+ grep -F -q "removed $FILE3HASH" actual_rm
106
'
107
108
test_expect_success "'ipfs block stat' with nothing from stdin doesnt crash" '