Skip to content

Commit 501c1db

Browse files
hannahhowardhannahhoward
authored
Update graphsync & fix in-progress request memory leak by consuming responses (#109)
* feat(deps): update to graphsync v0.4.1 * feat(graphsync): consume response channel consume response channel so graphsync does not buffer responses in memory * fix(deps): update graphsync to fix bug
1 parent 75d8843 commit 501c1db

File tree

5 files changed

+23
-24
lines changed

5 files changed

+23
-24
lines changed

‎go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ require (
1414
github.com/ipfs/go-cid v0.0.7
1515
github.com/ipfs/go-datastore v0.4.5
1616
github.com/ipfs/go-ds-badger v0.2.1
17-
github.com/ipfs/go-graphsync v0.3.1
17+
github.com/ipfs/go-graphsync v0.4.2
1818
github.com/ipfs/go-ipfs-blockstore v1.0.1
1919
github.com/ipfs/go-ipfs-blocksutil v0.0.1
2020
github.com/ipfs/go-ipfs-chunker v0.0.5
@@ -26,7 +26,7 @@ require (
2626
github.com/ipfs/go-log/v2 v2.0.3
2727
github.com/ipfs/go-merkledag v0.3.2
2828
github.com/ipfs/go-unixfs v0.2.4
29-
github.com/ipld/go-ipld-prime v0.5.1-0.20200828233916-988837377a7f
29+
github.com/ipld/go-ipld-prime v0.5.1-0.20201021195245-109253e8a018
3030
github.com/jbenet/go-random v0.0.0-20190219211222-123a90aedc0c
3131
github.com/jpillora/backoff v1.0.0
3232
github.com/libp2p/go-libp2p v0.6.0

‎go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ github.com/ipfs/go-ds-badger v0.2.1 h1:RsC9DDlwFhFdfT+s2PeC8joxbSp2YMufK8w/RBOxK
212212
github.com/ipfs/go-ds-badger v0.2.1/go.mod h1:Tx7l3aTph3FMFrRS838dcSJh+jjA7cX9DrGVwx/NOwE=
213213
github.com/ipfs/go-ds-leveldb v0.0.1/go.mod h1:feO8V3kubwsEF22n0YRQCffeb79OOYIykR4L04tMOYc=
214214
github.com/ipfs/go-ds-leveldb v0.4.1/go.mod h1:jpbku/YqBSsBc1qgME8BkWS4AxzF2cEu1Ii2r79Hh9s=
215-
github.com/ipfs/go-graphsync v0.3.1 h1:dJLYrck4oyJDfMVhGEKiWHxaY8oYMWko4m2Fi+4bofo=
216-
github.com/ipfs/go-graphsync v0.3.1/go.mod h1:bw4LiLM5Oq/uLdzEtih9LK8GrwSijv+XqYiWCTxHMqs=
215+
github.com/ipfs/go-graphsync v0.4.2 h1:Y/jt5r619yj0LI7OLtGKh4jYm8goYUcuJ09y7TZ3zMo=
216+
github.com/ipfs/go-graphsync v0.4.2/go.mod h1:/VmbZTUdUMTbNkgzAiCEucIIAU3BkLE2cZrDCVUhyi0=
217217
github.com/ipfs/go-ipfs-blockstore v0.0.1/go.mod h1:d3WClOmRQKFnJ0Jz/jj/zmksX0ma1gROTlovZKBmN08=
218218
github.com/ipfs/go-ipfs-blockstore v0.1.0/go.mod h1:5aD0AvHPi7mZc6Ci1WCAhiBQu2IsfTduLl+422H6Rqw=
219219
github.com/ipfs/go-ipfs-blockstore v0.1.4 h1:2SGI6U1B44aODevza8Rde3+dY30Pb+lbcObe1LETxOQ=
@@ -290,10 +290,10 @@ github.com/ipfs/go-unixfs v0.2.4 h1:6NwppOXefWIyysZ4LR/qUBPvXd5//8J3jiMdvpbw6Lo=
290290
github.com/ipfs/go-unixfs v0.2.4/go.mod h1:SUdisfUjNoSDzzhGVxvCL9QO/nKdwXdr+gbMUdqcbYw=
291291
github.com/ipfs/go-verifcid v0.0.1 h1:m2HI7zIuR5TFyQ1b79Da5N9dnnCP1vcu2QqawmWlK2E=
292292
github.com/ipfs/go-verifcid v0.0.1/go.mod h1:5Hrva5KBeIog4A+UpqlaIU+DEstipcJYQQZc0g37pY0=
293-
github.com/ipld/go-ipld-prime v0.5.1-0.20200828233916-988837377a7f h1:XpOuNQ5GbXxUcSukbQcW9jkE7REpaFGJU2/T00fo9kA=
294-
github.com/ipld/go-ipld-prime v0.5.1-0.20200828233916-988837377a7f/go.mod h1:0xEgdD6MKbZ1vF0GC+YcR/C4SQCAlRuOjIJ2i0HxqzM=
295-
github.com/ipld/go-ipld-prime-proto v0.0.0-20200922192210-9a2bfd4440a6 h1:6Mq+tZGSEMEoJJ1NbJRhddeelkXZcU8yfH/ZRYUo/Es=
296-
github.com/ipld/go-ipld-prime-proto v0.0.0-20200922192210-9a2bfd4440a6/go.mod h1:3pHYooM9Ea65jewRwrb2u5uHZCNkNTe9ABsVB+SrkH0=
293+
github.com/ipld/go-ipld-prime v0.5.1-0.20201021195245-109253e8a018 h1:RbRHv8epkmvBYA5cGfz68GUSbOgx5j/7ObLIl4Rsif0=
294+
github.com/ipld/go-ipld-prime v0.5.1-0.20201021195245-109253e8a018/go.mod h1:0xEgdD6MKbZ1vF0GC+YcR/C4SQCAlRuOjIJ2i0HxqzM=
295+
github.com/ipld/go-ipld-prime-proto v0.1.0 h1:j7gjqrfwbT4+gXpHwEx5iMssma3mnctC7YaCimsFP70=
296+
github.com/ipld/go-ipld-prime-proto v0.1.0/go.mod h1:11zp8f3sHVgIqtb/c9Kr5ZGqpnCLF1IVTNOez9TopzE=
297297
github.com/jackpal/gateway v1.0.5 h1:qzXWUJfuMdlLMtt0a3Dgt+xkWQiA5itDEITVJtuSwMc=
298298
github.com/jackpal/gateway v1.0.5/go.mod h1:lTpwd4ACLXmpyiCTRtfiNyVnUmqT9RivzCDQetPfnjA=
299299
github.com/jackpal/go-nat-pmp v1.0.1 h1:i0LektDkO1QlrTm/cSuP+PyBCDnYvjPLGl4LdWEMiaA=

‎testutil/fakegraphsync.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type ReceivedGraphSyncRequest struct {
4343
Root ipld.Link
4444
Selector ipld.Node
4545
Extensions []graphsync.ExtensionData
46+
ResponseChan chan graphsync.ResponseProgress
4647
ResponseErrChan chan error
4748
}
4849

@@ -247,8 +248,8 @@ func (fgs *FakeGraphSync) AssertDoesNotHavePersistenceOption(t *testing.T, name
247248
// Request initiates a new GraphSync request to the given peer using the given selector spec.
248249
func (fgs *FakeGraphSync) Request(ctx context.Context, p peer.ID, root ipld.Link, selector ipld.Node, extensions ...graphsync.ExtensionData) (<-chan graphsync.ResponseProgress, <-chan error) {
249250
errors := make(chan error)
250-
fgs.requests <- ReceivedGraphSyncRequest{ctx, p, root, selector, extensions, errors}
251251
responses := make(chan graphsync.ResponseProgress)
252+
fgs.requests <- ReceivedGraphSyncRequest{ctx, p, root, selector, extensions, responses, errors}
252253
if !fgs.leaveRequestsOpen {
253254
close(responses)
254255
close(errors)

‎transport/graphsync/graphsync.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,29 +126,24 @@ func (t *Transport) OpenChannel(ctx context.Context,
126126
Data: bz}
127127
exts = append(exts, doNotSendExt)
128128
}
129-
_, errChan := t.gs.Request(internalCtx, dataSender, root, stor, exts...)
129+
responseChan, errChan := t.gs.Request(internalCtx, dataSender, root, stor, exts...)
130130

131-
go t.executeGsRequest(ctx, channelID, errChan)
131+
go t.executeGsRequest(ctx, channelID, responseChan, errChan)
132132
return nil
133133
}
134134

135-
func (t *Transport) consumeResponses(ctx context.Context, errChan <-chan error) error {
135+
func (t *Transport) consumeResponses(responseChan <-chan graphsync.ResponseProgress, errChan <-chan error) error {
136136
var lastError error
137-
for {
138-
select {
139-
case <-ctx.Done():
140-
return errContextCancelled
141-
case err, ok := <-errChan:
142-
if !ok {
143-
return lastError
144-
}
145-
lastError = err
146-
}
137+
for range responseChan {
138+
}
139+
for err := range errChan {
140+
lastError = err
147141
}
142+
return lastError
148143
}
149144

150-
func (t *Transport) executeGsRequest(ctx context.Context, channelID datatransfer.ChannelID, errChan <-chan error) {
151-
lastError := t.consumeResponses(ctx, errChan)
145+
func (t *Transport) executeGsRequest(ctx context.Context, channelID datatransfer.ChannelID, responseChan <-chan graphsync.ResponseProgress, errChan <-chan error) {
146+
lastError := t.consumeResponses(responseChan, errChan)
152147

153148
if _, ok := lastError.(graphsync.RequestContextCancelledErr); ok {
154149
log.Warnf("graphsync request context cancelled, channel Id: %v", channelID)

‎transport/graphsync/graphsync_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,7 @@ func TestManager(t *testing.T) {
710710
},
711711
check: func(t *testing.T, events *fakeEvents, gsData *harness) {
712712
requestReceived := gsData.fgs.AssertRequestReceived(gsData.ctx, t)
713+
close(requestReceived.ResponseChan)
713714
close(requestReceived.ResponseErrChan)
714715

715716
require.Eventually(t, func() bool {
@@ -734,6 +735,7 @@ func TestManager(t *testing.T) {
734735
},
735736
check: func(t *testing.T, events *fakeEvents, gsData *harness) {
736737
requestReceived := gsData.fgs.AssertRequestReceived(gsData.ctx, t)
738+
close(requestReceived.ResponseChan)
737739
requestReceived.ResponseErrChan <- graphsync.RequestFailedUnknownErr{}
738740
close(requestReceived.ResponseErrChan)
739741

@@ -789,6 +791,7 @@ func TestManager(t *testing.T) {
789791
},
790792
check: func(t *testing.T, events *fakeEvents, gsData *harness) {
791793
requestReceived := gsData.fgs.AssertRequestReceived(gsData.ctx, t)
794+
close(requestReceived.ResponseChan)
792795
requestReceived.ResponseErrChan <- graphsync.RequestContextCancelledErr{}
793796
close(requestReceived.ResponseErrChan)
794797

0 commit comments

Comments
 (0)