Fix #3783: Improve IsPinned() lookups for indirect pins
This avoids revisiting already-searched branches and cut down the IsPinned() check times considerably when recursive pins share big underlying DAGs. A test has been added which double-checks that pinned and unpinned items lookups respond as expected with shared branches. License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
Hector Sanjuan committed
Mar 21, 2017 at 12:25 UTC
71a7a90f62743d0fdff75b648463356e4bda81ec
2 files changed
+140
-9
pin/pin.go
+13
-9
@@ -276,8 +276,9 @@ func (p *pinner) isPinnedWithType(c *cid.Cid, mode PinMode) (string, bool, error
276
}
277
278
// Default is Indirect
279
+ visitedSet := cid.NewSet()
280
for _, rc := range p.recursePin.Keys() {
280
- has, err := hasChild(p.dserv, rc, c)
281
+ has, err := hasChild(p.dserv, rc, c, visitedSet.Visit)
282
if err != nil {
283
return "", false, err
284
}
@@ -519,7 +520,9 @@ func (p *pinner) PinWithMode(c *cid.Cid, mode PinMode) {
520
}
521
}
522
522
-func hasChild(ds mdag.LinkService, root *cid.Cid, child *cid.Cid) (bool, error) {
523
+// hasChild recursively looks for a Cid among the children of a root Cid.
524
+// The visit function can be used to shortcut already-visited branches.
525
+func hasChild(ds mdag.LinkService, root *cid.Cid, child *cid.Cid, visit func(*cid.Cid) bool) (bool, error) {
526
links, err := ds.GetLinks(context.Background(), root)
527
if err != nil {
528
return false, err
@@ -529,14 +532,15 @@ func hasChild(ds mdag.LinkService, root *cid.Cid, child *cid.Cid) (bool, error)
532
if lnk.Cid.Equals(child) {
533
return true, nil
534
}
535
+ if visit(c) {
536
+ has, err := hasChild(ds, c, child, visit)
537
+ if err != nil {
538
+ return false, err
539
+ }
540
533
- has, err := hasChild(ds, c, child)
534
- if err != nil {
535
- return false, err
536
- }
537
-
538
- if has {
539
- return has, nil
541
+ if has {
542
+ return has, nil
543
+ }
544
}
545
}
546
return false, nil
pin/pin_test.go
+127
@@ -35,6 +35,17 @@ func assertPinned(t *testing.T, p Pinner, c *cid.Cid, failmsg string) {
35
}
36
}
37
38
+func assertUnpinned(t *testing.T, p Pinner, c *cid.Cid, failmsg string) {
39
+ _, pinned, err := p.IsPinned(c)
40
+ if err != nil {
41
+ t.Fatal(err)
42
+ }
43
+
44
+ if pinned {
45
+ t.Fatal(failmsg)
46
+ }
47
+}
48
+
49
func TestPinnerBasic(t *testing.T) {
50
ctx := context.Background()
51
@@ -145,6 +156,122 @@ func TestPinnerBasic(t *testing.T) {
156
assertPinned(t, np, bk, "could not find recursively pinned node")
157
}
158
159
+func TestIsPinnedLookup(t *testing.T) {
160
+ // We are going to test that lookups work in pins which share
161
+ // the same branches. For that we will construct this tree:
162
+ //
163
+ // A5->A4->A3->A2->A1->A0
164
+ // / /
165
+ // B------- /
166
+ // \ /
167
+ // C---------------
168
+ //
169
+ // We will ensure that IsPinned works for all objects both when they
170
+ // are pinned and once they have been unpinned.
171
+ aBranchLen := 6
172
+ if aBranchLen < 3 {
173
+ t.Fatal("set aBranchLen to at least 3")
174
+ }
175
+
176
+ ctx := context.Background()
177
+ dstore := dssync.MutexWrap(ds.NewMapDatastore())
178
+ bstore := blockstore.NewBlockstore(dstore)
179
+ bserv := bs.New(bstore, offline.Exchange(bstore))
180
+
181
+ dserv := mdag.NewDAGService(bserv)
182
+
183
+ // TODO does pinner need to share datastore with blockservice?
184
+ p := NewPinner(dstore, dserv, dserv)
185
+
186
+ aNodes := make([]*mdag.ProtoNode, aBranchLen, aBranchLen)
187
+ aKeys := make([]*cid.Cid, aBranchLen, aBranchLen)
188
+ for i := 0; i < aBranchLen; i++ {
189
+ a, _ := randNode()
190
+ if i >= 1 {
191
+ err := a.AddNodeLink("child", aNodes[i-1])
192
+ if err != nil {
193
+ t.Fatal(err)
194
+ }
195
+ }
196
+
197
+ ak, err := dserv.Add(a)
198
+ if err != nil {
199
+ t.Fatal(err)
200
+ }
201
+ //t.Logf("a[%d] is %s", i, ak)
202
+ aNodes[i] = a
203
+ aKeys[i] = ak
204
+ }
205
+
206
+ // Pin A5 recursively
207
+ if err := p.Pin(ctx, aNodes[aBranchLen-1], true); err != nil {
208
+ t.Fatal(err)
209
+ }
210
+
211
+ // Create node B and add A3 as child
212
+ b, _ := randNode()
213
+ if err := b.AddNodeLink("mychild", aNodes[3]); err != nil {
214
+ t.Fatal(err)
215
+ }
216
+
217
+ // Create C node
218
+ c, _ := randNode()
219
+ // Add A0 as child of C
220
+ if err := c.AddNodeLink("child", aNodes[0]); err != nil {
221
+ t.Fatal(err)
222
+ }
223
+
224
+ // Add C
225
+ ck, err := dserv.Add(c)
226
+ if err != nil {
227
+ t.Fatal(err)
228
+ }
229
+ //t.Logf("C is %s", ck)
230
+
231
+ // Add C to B and Add B
232
+ if err := b.AddNodeLink("myotherchild", c); err != nil {
233
+ t.Fatal(err)
234
+ }
235
+ bk, err := dserv.Add(b)
236
+ if err != nil {
237
+ t.Fatal(err)
238
+ }
239
+ //t.Logf("B is %s", bk)
240
+
241
+ // Pin C recursively
242
+
243
+ if err := p.Pin(ctx, c, true); err != nil {
244
+ t.Fatal(err)
245
+ }
246
+
247
+ // Pin B recursively
248
+
249
+ if err := p.Pin(ctx, b, true); err != nil {
250
+ t.Fatal(err)
251
+ }
252
+
253
+ assertPinned(t, p, aKeys[0], "A0 should be pinned")
254
+ assertPinned(t, p, aKeys[1], "A1 should be pinned")
255
+ assertPinned(t, p, ck, "C should be pinned")
256
+ assertPinned(t, p, bk, "B should be pinned")
257
+
258
+ // Unpin A5 recursively
259
+ if err := p.Unpin(ctx, aKeys[5], true); err != nil {
260
+ t.Fatal(err)
261
+ }
262
+
263
+ assertPinned(t, p, aKeys[0], "A0 should still be pinned through B")
264
+ assertUnpinned(t, p, aKeys[4], "A4 should be unpinned")
265
+
266
+ // Unpin B recursively
267
+ if err := p.Unpin(ctx, bk, true); err != nil {
268
+ t.Fatal(err)
269
+ }
270
+ assertUnpinned(t, p, bk, "B should be unpinned")
271
+ assertUnpinned(t, p, aKeys[1], "A1 should be unpinned")
272
+ assertPinned(t, p, aKeys[0], "A0 should still be pinned through C")
273
+}
274
+
275
func TestDuplicateSemantics(t *testing.T) {
276
ctx := context.Background()
277
dstore := dssync.MutexWrap(ds.NewMapDatastore())