From 5be35a83f18d06e31723646aa9cbf0f85f7813c2 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 18 Nov 2014 21:31:00 -0800 Subject: [PATCH 01/90] beginnings of a bitswap refactor --- blockservice/blockservice.go | 2 +- exchange/bitswap/bitswap.go | 139 ++++++++++++++++++++++--------- exchange/bitswap/bitswap_test.go | 18 ++-- exchange/interface.go | 4 +- exchange/offline/offline.go | 4 +- exchange/offline/offline_test.go | 2 +- 6 files changed, 117 insertions(+), 52 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 82a1b03c1..2eb3d695d 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -85,7 +85,7 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er }, nil } else if err == ds.ErrNotFound && s.Remote != nil { log.Debug("Blockservice: Searching bitswap.") - blk, err := s.Remote.Block(ctx, k) + blk, err := s.Remote.GetBlock(ctx, k) if err != nil { return nil, err } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 8af8426d3..6daf32555 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -42,8 +42,10 @@ func New(ctx context.Context, p peer.Peer, routing: routing, sender: network, wantlist: u.NewKeySet(), + blockReq: make(chan u.Key, 32), } network.SetDelegate(bs) + go bs.run(ctx) return bs } @@ -63,6 +65,8 @@ type bitswap struct { notifications notifications.PubSub + blockReq chan u.Key + // strategy listens to network traffic and makes decisions about how to // interact with partners. // TODO(brian): save the strategy's state to the datastore @@ -75,7 +79,7 @@ type bitswap struct { // deadline enforced by the context // // TODO ensure only one active request per key -func (bs *bitswap) Block(parent context.Context, k u.Key) (*blocks.Block, error) { +func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, error) { log.Debugf("Get Block %v", k) now := time.Now() defer func() { @@ -88,42 +92,11 @@ func (bs *bitswap) Block(parent context.Context, k u.Key) (*blocks.Block, error) bs.wantlist.Add(k) promise := bs.notifications.Subscribe(ctx, k) - const maxProviders = 20 - peersToQuery := bs.routing.FindProvidersAsync(ctx, k, maxProviders) - - go func() { - message := bsmsg.New() - for _, wanted := range bs.wantlist.Keys() { - message.AddWanted(wanted) - } - for peerToQuery := range peersToQuery { - log.Debugf("bitswap got peersToQuery: %s", peerToQuery) - go func(p peer.Peer) { - - log.Debugf("bitswap dialing peer: %s", p) - err := bs.sender.DialPeer(ctx, p) - if err != nil { - log.Errorf("Error sender.DialPeer(%s)", p) - return - } - - response, err := bs.sender.SendRequest(ctx, p, message) - if err != nil { - log.Errorf("Error sender.SendRequest(%s) = %s", p, err) - return - } - // FIXME ensure accounting is handled correctly when - // communication fails. May require slightly different API to - // get better guarantees. May need shared sequence numbers. - bs.strategy.MessageSent(p, message) - - if response == nil { - return - } - bs.ReceiveMessage(ctx, p, response) - }(peerToQuery) - } - }() + select { + case bs.blockReq <- k: + case <-parent.Done(): + return nil, parent.Err() + } select { case block := <-promise: @@ -134,6 +107,96 @@ func (bs *bitswap) Block(parent context.Context, k u.Key) (*blocks.Block, error) } } +func (bs *bitswap) GetBlocks(parent context.Context, ks []u.Key) (*blocks.Block, error) { + // TODO: something smart + return nil, nil +} + +func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { + message := bsmsg.New() + for _, wanted := range bs.wantlist.Keys() { + message.AddWanted(wanted) + } + for peerToQuery := range peers { + log.Debugf("bitswap got peersToQuery: %s", peerToQuery) + go func(p peer.Peer) { + + log.Debugf("bitswap dialing peer: %s", p) + err := bs.sender.DialPeer(ctx, p) + if err != nil { + log.Errorf("Error sender.DialPeer(%s)", p) + return + } + + response, err := bs.sender.SendRequest(ctx, p, message) + if err != nil { + log.Errorf("Error sender.SendRequest(%s) = %s", p, err) + return + } + // FIXME ensure accounting is handled correctly when + // communication fails. May require slightly different API to + // get better guarantees. May need shared sequence numbers. + bs.strategy.MessageSent(p, message) + + if response == nil { + return + } + bs.ReceiveMessage(ctx, p, response) + }(peerToQuery) + } + return nil +} + +func (bs *bitswap) run(ctx context.Context) { + var sendlist <-chan peer.Peer + + // Every so often, we should resend out our current want list + rebroadcastTime := time.Second * 5 + + // Time to wait before sending out wantlists to better batch up requests + bufferTime := time.Millisecond * 3 + peersPerSend := 6 + + timeout := time.After(rebroadcastTime) + threshold := 10 + unsent := 0 + for { + select { + case <-timeout: + if sendlist == nil { + // rely on semi randomness of maps + firstKey := bs.wantlist.Keys()[0] + sendlist = bs.routing.FindProvidersAsync(ctx, firstKey, 6) + } + err := bs.sendWantListTo(ctx, sendlist) + if err != nil { + log.Error("error sending wantlist: %s", err) + } + sendlist = nil + timeout = time.After(rebroadcastTime) + case k := <-bs.blockReq: + if unsent == 0 { + sendlist = bs.routing.FindProvidersAsync(ctx, k, peersPerSend) + } + unsent++ + + if unsent >= threshold { + // send wantlist to sendlist + bs.sendWantListTo(ctx, sendlist) + unsent = 0 + timeout = time.After(rebroadcastTime) + sendlist = nil + } else { + // set a timeout to wait for more blocks or send current wantlist + + timeout = time.After(bufferTime) + } + case <-ctx.Done(): + return + } + } +} + // HasBlock announces the existance of a block to this bitswap service. The // service will potentially notify its peers. func (bs *bitswap) HasBlock(ctx context.Context, blk blocks.Block) error { @@ -192,8 +255,8 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } } - defer bs.strategy.MessageSent(p, message) + bs.strategy.MessageSent(p, message) log.Debug("Returning message.") return p, message } diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index a851f0f56..ee1e7644d 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -31,7 +31,7 @@ func TestGetBlockTimeout(t *testing.T) { ctx, _ := context.WithTimeout(context.Background(), time.Nanosecond) block := blocks.NewBlock([]byte("block")) - _, err := self.exchange.Block(ctx, block.Key()) + _, err := self.exchange.GetBlock(ctx, block.Key()) if err != context.DeadlineExceeded { t.Fatal("Expected DeadlineExceeded error") @@ -50,7 +50,7 @@ func TestProviderForKeyButNetworkCannotFind(t *testing.T) { solo := g.Next() ctx, _ := context.WithTimeout(context.Background(), time.Nanosecond) - _, err := solo.exchange.Block(ctx, block.Key()) + _, err := solo.exchange.GetBlock(ctx, block.Key()) if err != context.DeadlineExceeded { t.Fatal("Expected DeadlineExceeded error") @@ -78,7 +78,7 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { wantsBlock := g.Next() ctx, _ := context.WithTimeout(context.Background(), time.Second) - received, err := wantsBlock.exchange.Block(ctx, block.Key()) + received, err := wantsBlock.exchange.GetBlock(ctx, block.Key()) if err != nil { t.Log(err) t.Fatal("Expected to succeed") @@ -100,7 +100,7 @@ func TestSwarm(t *testing.T) { t.Log("Create a ton of instances, and just a few blocks") - numInstances := 500 + numInstances := 5 numBlocks := 2 instances := sg.Instances(numInstances) @@ -142,7 +142,7 @@ func TestSwarm(t *testing.T) { func getOrFail(bitswap instance, b *blocks.Block, t *testing.T, wg *sync.WaitGroup) { if _, err := bitswap.blockstore.Get(b.Key()); err != nil { - _, err := bitswap.exchange.Block(context.Background(), b.Key()) + _, err := bitswap.exchange.GetBlock(context.Background(), b.Key()) if err != nil { t.Fatal(err) } @@ -171,7 +171,7 @@ func TestSendToWantingPeer(t *testing.T) { t.Logf("Peer %v attempts to get %v. NB: not available\n", w.peer, alpha.Key()) ctx, _ := context.WithTimeout(context.Background(), timeout) - _, err := w.exchange.Block(ctx, alpha.Key()) + _, err := w.exchange.GetBlock(ctx, alpha.Key()) if err == nil { t.Fatalf("Expected %v to NOT be available", alpha.Key()) } @@ -186,7 +186,7 @@ func TestSendToWantingPeer(t *testing.T) { t.Logf("%v gets %v from %v and discovers it wants %v\n", me.peer, beta.Key(), w.peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if _, err := me.exchange.Block(ctx, beta.Key()); err != nil { + if _, err := me.exchange.GetBlock(ctx, beta.Key()); err != nil { t.Fatal(err) } @@ -199,7 +199,7 @@ func TestSendToWantingPeer(t *testing.T) { t.Logf("%v requests %v\n", me.peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if _, err := me.exchange.Block(ctx, alpha.Key()); err != nil { + if _, err := me.exchange.GetBlock(ctx, alpha.Key()); err != nil { t.Fatal(err) } @@ -290,8 +290,10 @@ func session(net tn.Network, rs mock.RoutingServer, id peer.ID) instance { routing: htc, sender: adapter, wantlist: util.NewKeySet(), + blockReq: make(chan util.Key, 32), } adapter.SetDelegate(bs) + go bs.run(context.TODO()) return instance{ peer: p, exchange: bs, diff --git a/exchange/interface.go b/exchange/interface.go index 82782a046..b62a47957 100644 --- a/exchange/interface.go +++ b/exchange/interface.go @@ -12,8 +12,8 @@ import ( // exchange protocol. type Interface interface { - // Block returns the block associated with a given key. - Block(context.Context, u.Key) (*blocks.Block, error) + // GetBlock returns the block associated with a given key. + GetBlock(context.Context, u.Key) (*blocks.Block, error) // TODO Should callers be concerned with whether the block was made // available on the network? diff --git a/exchange/offline/offline.go b/exchange/offline/offline.go index 5f7ef8835..37d672f73 100644 --- a/exchange/offline/offline.go +++ b/exchange/offline/offline.go @@ -23,10 +23,10 @@ func NewOfflineExchange() exchange.Interface { type offlineExchange struct { } -// Block returns nil to signal that a block could not be retrieved for the +// GetBlock returns nil to signal that a block could not be retrieved for the // given key. // NB: This function may return before the timeout expires. -func (_ *offlineExchange) Block(context.Context, u.Key) (*blocks.Block, error) { +func (_ *offlineExchange) GetBlock(context.Context, u.Key) (*blocks.Block, error) { return nil, OfflineMode } diff --git a/exchange/offline/offline_test.go b/exchange/offline/offline_test.go index cc3f3ec82..ae3fdaa0a 100644 --- a/exchange/offline/offline_test.go +++ b/exchange/offline/offline_test.go @@ -11,7 +11,7 @@ import ( func TestBlockReturnsErr(t *testing.T) { off := NewOfflineExchange() - _, err := off.Block(context.Background(), u.Key("foo")) + _, err := off.GetBlock(context.Background(), u.Key("foo")) if err != nil { return // as desired } From 5dece164cc23b325cc6576e5ff4a958c93b85018 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 18 Nov 2014 21:46:13 -0800 Subject: [PATCH 02/90] dont panic on empty wantlist --- exchange/bitswap/bitswap.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 6daf32555..4aaacdbfd 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -163,9 +163,13 @@ func (bs *bitswap) run(ctx context.Context) { for { select { case <-timeout: + wantlist := bs.wantlist.Keys() + if len(wantlist) == 0 { + continue + } if sendlist == nil { // rely on semi randomness of maps - firstKey := bs.wantlist.Keys()[0] + firstKey := wantlist[0] sendlist = bs.routing.FindProvidersAsync(ctx, firstKey, 6) } err := bs.sendWantListTo(ctx, sendlist) From cfd7d5369b5b442a9001f0faae111ddead4b63aa Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 21:52:58 -0800 Subject: [PATCH 03/90] test(bitswap) @whyrusleeping This appears to be a timing issue. The asynchronous nature of the new structure provides has the bitswap waiting on the context a bit more. This isn't a problem at all, but in this test, it makes the functions return in an inconveniently timely manner. TODO don't let the test depend on time. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index ee1e7644d..f69cb7629 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -152,6 +152,10 @@ func getOrFail(bitswap instance, b *blocks.Block, t *testing.T, wg *sync.WaitGro // TODO simplify this test. get to the _essence_! func TestSendToWantingPeer(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + net := tn.VirtualNetwork() rs := mock.VirtualRoutingServer() sg := NewSessionGenerator(net, rs) @@ -167,7 +171,7 @@ func TestSendToWantingPeer(t *testing.T) { alpha := bg.Next() - const timeout = 1 * time.Millisecond // FIXME don't depend on time + const timeout = 100 * time.Millisecond // FIXME don't depend on time t.Logf("Peer %v attempts to get %v. NB: not available\n", w.peer, alpha.Key()) ctx, _ := context.WithTimeout(context.Background(), timeout) From 77696a47f7ec4a06e1b4a92ac4bf3333581194e1 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:05:53 -0800 Subject: [PATCH 04/90] events(bitswap) try the new event logger in the bitswap GetBlock method @jbenet @whyrusleeping Let me know if you want to direct the eventlog output to _both_ the file and stderr. Right now it goes to file. Perhaps this is just a minor bip in the larger discussion around log levels. https://github.com/jbenet/go-ipfs/issues/292 License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 19 +++++++++++++------ util/key.go | 6 ++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 4aaacdbfd..a4bb0ec0c 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -17,9 +17,10 @@ import ( strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" + "github.com/jbenet/go-ipfs/util/eventlog" ) -var log = u.Logger("bitswap") +var log = eventlog.Logger("bitswap") // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as @@ -80,15 +81,21 @@ type bitswap struct { // // TODO ensure only one active request per key func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, error) { - log.Debugf("Get Block %v", k) - now := time.Now() - defer func() { - log.Debugf("GetBlock took %f secs", time.Now().Sub(now).Seconds()) - }() + + // make sure to derive a new |ctx| and pass it to children. It's correct to + // listen on |parent| here, but incorrect to pass |parent| to new async + // functions. This is difficult to enforce. May this comment keep you safe. ctx, cancelFunc := context.WithCancel(parent) defer cancelFunc() + ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("BitswapGetBlockRequest")) + log.Event(ctx, "BitswapGetBlockRequestBegin", &k) + + defer func() { + log.Event(ctx, "BitSwapGetBlockRequestEnd", &k) + }() + bs.wantlist.Add(k) promise := bs.notifications.Subscribe(ctx, k) diff --git a/util/key.go b/util/key.go index 7cbf09518..eca1255b5 100644 --- a/util/key.go +++ b/util/key.go @@ -63,6 +63,12 @@ func (k *Key) MarshalJSON() ([]byte, error) { return json.Marshal(b58.Encode([]byte(*k))) } +func (k *Key) Loggable() map[string]interface{} { + return map[string]interface{}{ + "key": k.String(), + } +} + // KeyFromDsKey returns a Datastore key func KeyFromDsKey(dsk ds.Key) Key { return Key(dsk.BaseNamespace()) From ef831268e0ab1aaed35aec32ff4caa24cfc1df51 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:14:10 -0800 Subject: [PATCH 05/90] fix(bitswap) handle error @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index a4bb0ec0c..4a66aaa06 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -193,7 +193,10 @@ func (bs *bitswap) run(ctx context.Context) { if unsent >= threshold { // send wantlist to sendlist - bs.sendWantListTo(ctx, sendlist) + err := bs.sendWantListTo(ctx, sendlist) + if err != nil { + log.Errorf("error sending wantlist: %s", err) + } unsent = 0 timeout = time.After(rebroadcastTime) sendlist = nil From 768cd3682c61e4756c9332dc9e23f96ab3c59c53 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:14:36 -0800 Subject: [PATCH 06/90] fix(bitswap) consistent event names @whyrusleeping @jbenet since the logger is created with package scope, don't need to specify the package name in event messages License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 4a66aaa06..bcfcebd94 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -89,11 +89,11 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err ctx, cancelFunc := context.WithCancel(parent) defer cancelFunc() - ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("BitswapGetBlockRequest")) - log.Event(ctx, "BitswapGetBlockRequestBegin", &k) + ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("GetBlockRequest")) + log.Event(ctx, "GetBlockRequestBegin", &k) defer func() { - log.Event(ctx, "BitSwapGetBlockRequestEnd", &k) + log.Event(ctx, "GetBlockRequestEnd", &k) }() bs.wantlist.Add(k) From 5a4eed4c0045b329b185d1f45041bf2d1aa9e913 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:17:09 -0800 Subject: [PATCH 07/90] fix(log) ->f @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index bcfcebd94..9f0b7c7b9 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -181,7 +181,7 @@ func (bs *bitswap) run(ctx context.Context) { } err := bs.sendWantListTo(ctx, sendlist) if err != nil { - log.Error("error sending wantlist: %s", err) + log.Errorf("error sending wantlist: %s", err) } sendlist = nil timeout = time.After(rebroadcastTime) From 5165fce9d9dd49b36b569b3dd6829af4dac38137 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:18:25 -0800 Subject: [PATCH 08/90] use event logger here too? License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 9f0b7c7b9..7e82168bf 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -125,10 +125,10 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e message.AddWanted(wanted) } for peerToQuery := range peers { - log.Debugf("bitswap got peersToQuery: %s", peerToQuery) + log.Event(ctx, "PeerToQuery", peerToQuery) go func(p peer.Peer) { - log.Debugf("bitswap dialing peer: %s", p) + log.Event(ctx, "DialPeer", p) err := bs.sender.DialPeer(ctx, p) if err != nil { log.Errorf("Error sender.DialPeer(%s)", p) From 7239036e2d919c45241d8e346ce9e40f407c155f Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:20:25 -0800 Subject: [PATCH 09/90] clarify MessageReceived contract License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/strategy.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index b778c7a34..78209c38e 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -72,6 +72,8 @@ func (s *strategist) Seed(int64) { // TODO } +// MessageReceived performs book-keeping. Returns error if passed invalid +// arguments. func (s *strategist) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { s.lock.Lock() defer s.lock.Unlock() @@ -91,7 +93,7 @@ func (s *strategist) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error // FIXME extract blocks.NumBytes(block) or block.NumBytes() method l.ReceivedBytes(len(block.Data)) } - return errors.New("TODO") + return nil } // TODO add contents of m.WantList() to my local wantlist? NB: could introduce From 3ee7ff54bab5251c7d711f12505edcc2cb0ba6bb Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:31:42 -0800 Subject: [PATCH 10/90] naming License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 7e82168bf..87116fd42 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -43,7 +43,7 @@ func New(ctx context.Context, p peer.Peer, routing: routing, sender: network, wantlist: u.NewKeySet(), - blockReq: make(chan u.Key, 32), + blockRequests: make(chan u.Key, 32), } network.SetDelegate(bs) go bs.run(ctx) @@ -66,7 +66,7 @@ type bitswap struct { notifications notifications.PubSub - blockReq chan u.Key + blockRequests chan u.Key // strategy listens to network traffic and makes decisions about how to // interact with partners. @@ -100,7 +100,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err promise := bs.notifications.Subscribe(ctx, k) select { - case bs.blockReq <- k: + case bs.blockRequests <- k: case <-parent.Done(): return nil, parent.Err() } @@ -185,7 +185,7 @@ func (bs *bitswap) run(ctx context.Context) { } sendlist = nil timeout = time.After(rebroadcastTime) - case k := <-bs.blockReq: + case k := <-bs.blockRequests: if unsent == 0 { sendlist = bs.routing.FindProvidersAsync(ctx, k, peersPerSend) } From 6c2a6669cb97a81c3f40364962530c0a800dddc1 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:37:47 -0800 Subject: [PATCH 11/90] constify to make it clear what _can_ and _can't_ change over time License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 87116fd42..73c95c230 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -158,14 +158,14 @@ func (bs *bitswap) run(ctx context.Context) { var sendlist <-chan peer.Peer // Every so often, we should resend out our current want list - rebroadcastTime := time.Second * 5 + const rebroadcastTime = time.Second * 5 // Time to wait before sending out wantlists to better batch up requests - bufferTime := time.Millisecond * 3 + const bufferTime = time.Millisecond * 3 peersPerSend := 6 timeout := time.After(rebroadcastTime) - threshold := 10 + const threshold = 10 unsent := 0 for { select { From e5983cbe7dbcb38278895bb6b4a77a1786ad5f72 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:58:06 -0800 Subject: [PATCH 12/90] some renaming License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 24 +++++++++++------------- exchange/bitswap/bitswap_test.go | 2 +- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 73c95c230..b8f8a7d18 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -155,21 +155,19 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } func (bs *bitswap) run(ctx context.Context) { - var sendlist <-chan peer.Peer - // Every so often, we should resend out our current want list - const rebroadcastTime = time.Second * 5 - - // Time to wait before sending out wantlists to better batch up requests - const bufferTime = time.Millisecond * 3 - peersPerSend := 6 - - timeout := time.After(rebroadcastTime) + const rebroadcastPeriod = time.Second * 5 // Every so often, we should resend out our current want list + const batchDelay = time.Millisecond * 3 // Time to wait before sending out wantlists to better batch up requests + const peersPerSend = 6 const threshold = 10 + + var sendlist <-chan peer.Peer // NB: must be initialized to zero value + broadcastSignal := time.After(rebroadcastPeriod) unsent := 0 + for { select { - case <-timeout: + case <-broadcastSignal: wantlist := bs.wantlist.Keys() if len(wantlist) == 0 { continue @@ -184,7 +182,7 @@ func (bs *bitswap) run(ctx context.Context) { log.Errorf("error sending wantlist: %s", err) } sendlist = nil - timeout = time.After(rebroadcastTime) + broadcastSignal = time.After(rebroadcastPeriod) case k := <-bs.blockRequests: if unsent == 0 { sendlist = bs.routing.FindProvidersAsync(ctx, k, peersPerSend) @@ -198,12 +196,12 @@ func (bs *bitswap) run(ctx context.Context) { log.Errorf("error sending wantlist: %s", err) } unsent = 0 - timeout = time.After(rebroadcastTime) + broadcastSignal = time.After(rebroadcastPeriod) sendlist = nil } else { // set a timeout to wait for more blocks or send current wantlist - timeout = time.After(bufferTime) + broadcastSignal = time.After(batchDelay) } case <-ctx.Done(): return diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index f69cb7629..e06eabefa 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -294,7 +294,7 @@ func session(net tn.Network, rs mock.RoutingServer, id peer.ID) instance { routing: htc, sender: adapter, wantlist: util.NewKeySet(), - blockReq: make(chan util.Key, 32), + blockRequests: make(chan util.Key, 32), } adapter.SetDelegate(bs) go bs.run(context.TODO()) From f8243c36be4dfbc33450fdf50e0e2afd9abc5dd2 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 18 Nov 2014 22:58:30 -0800 Subject: [PATCH 13/90] simplify License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index b8f8a7d18..1102dda75 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -91,10 +91,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("GetBlockRequest")) log.Event(ctx, "GetBlockRequestBegin", &k) - - defer func() { - log.Event(ctx, "GetBlockRequestEnd", &k) - }() + defer log.Event(ctx, "GetBlockRequestEnd", &k) bs.wantlist.Add(k) promise := bs.notifications.Subscribe(ctx, k) From 9af9ee6255b2f0e90bf02bb198f23cefcf11cd51 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 09:40:21 -0800 Subject: [PATCH 14/90] misc(bitswap) renaming License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 39 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 1102dda75..e904d28a6 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -153,14 +153,14 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e func (bs *bitswap) run(ctx context.Context) { + const batchDelay = time.Millisecond * 3 // Time to wait before sending out wantlists to better batch up requests + const numKeysPerBatch = 10 + const maxProvidersPerRequest = 6 const rebroadcastPeriod = time.Second * 5 // Every so often, we should resend out our current want list - const batchDelay = time.Millisecond * 3 // Time to wait before sending out wantlists to better batch up requests - const peersPerSend = 6 - const threshold = 10 - var sendlist <-chan peer.Peer // NB: must be initialized to zero value + var providers <-chan peer.Peer // NB: must be initialized to zero value broadcastSignal := time.After(rebroadcastPeriod) - unsent := 0 + unsentKeys := 0 for { select { @@ -169,32 +169,33 @@ func (bs *bitswap) run(ctx context.Context) { if len(wantlist) == 0 { continue } - if sendlist == nil { + if providers == nil { // rely on semi randomness of maps firstKey := wantlist[0] - sendlist = bs.routing.FindProvidersAsync(ctx, firstKey, 6) + providers = bs.routing.FindProvidersAsync(ctx, firstKey, 6) } - err := bs.sendWantListTo(ctx, sendlist) + err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } - sendlist = nil + providers = nil broadcastSignal = time.After(rebroadcastPeriod) - case k := <-bs.blockRequests: - if unsent == 0 { - sendlist = bs.routing.FindProvidersAsync(ctx, k, peersPerSend) - } - unsent++ - if unsent >= threshold { - // send wantlist to sendlist - err := bs.sendWantListTo(ctx, sendlist) + case k := <-bs.blockRequests: + if unsentKeys == 0 { + providers = bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + } + unsentKeys++ + + if unsentKeys >= numKeysPerBatch { + // send wantlist to providers + err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } - unsent = 0 + unsentKeys = 0 broadcastSignal = time.After(rebroadcastPeriod) - sendlist = nil + providers = nil } else { // set a timeout to wait for more blocks or send current wantlist From 4d1447589c61bb42d853f61b9ba36aa2181335b8 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 19 Nov 2014 10:13:31 -0800 Subject: [PATCH 15/90] added a new test for a dhthell scenario that was failing --- exchange/bitswap/bitswap.go | 8 ++--- exchange/bitswap/bitswap_test.go | 53 +++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index e904d28a6..1539b5fc8 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -128,7 +128,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e log.Event(ctx, "DialPeer", p) err := bs.sender.DialPeer(ctx, p) if err != nil { - log.Errorf("Error sender.DialPeer(%s)", p) + log.Errorf("Error sender.DialPeer(%s): %s", p, err) return } @@ -153,10 +153,8 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e func (bs *bitswap) run(ctx context.Context) { - const batchDelay = time.Millisecond * 3 // Time to wait before sending out wantlists to better batch up requests - const numKeysPerBatch = 10 - const maxProvidersPerRequest = 6 - const rebroadcastPeriod = time.Second * 5 // Every so often, we should resend out our current want list + // Every so often, we should resend out our current want list + rebroadcastTime := time.Second * 5 var providers <-chan peer.Peer // NB: must be initialized to zero value broadcastSignal := time.After(rebroadcastPeriod) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index e06eabefa..e3b4d913a 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -100,7 +100,7 @@ func TestSwarm(t *testing.T) { t.Log("Create a ton of instances, and just a few blocks") - numInstances := 5 + numInstances := 500 numBlocks := 2 instances := sg.Instances(numInstances) @@ -140,6 +140,57 @@ func TestSwarm(t *testing.T) { } } +func TestLargeFile(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + net := tn.VirtualNetwork() + rs := mock.VirtualRoutingServer() + sg := NewSessionGenerator(net, rs) + bg := NewBlockGenerator() + + t.Log("Test a few nodes trying to get one file with a lot of blocks") + + numInstances := 10 + numBlocks := 100 + + instances := sg.Instances(numInstances) + blocks := bg.Blocks(numBlocks) + + t.Log("Give the blocks to the first instance") + + first := instances[0] + for _, b := range blocks { + first.blockstore.Put(b) + first.exchange.HasBlock(context.Background(), *b) + rs.Announce(first.peer, b.Key()) + } + + t.Log("Distribute!") + + var wg sync.WaitGroup + + for _, inst := range instances { + for _, b := range blocks { + wg.Add(1) + // NB: executing getOrFail concurrently puts tremendous pressure on + // the goroutine scheduler + getOrFail(inst, b, t, &wg) + } + } + wg.Wait() + + t.Log("Verify!") + + for _, inst := range instances { + for _, b := range blocks { + if _, err := inst.blockstore.Get(b.Key()); err != nil { + t.Fatal(err) + } + } + } +} + func getOrFail(bitswap instance, b *blocks.Block, t *testing.T, wg *sync.WaitGroup) { if _, err := bitswap.blockstore.Get(b.Key()); err != nil { _, err := bitswap.exchange.GetBlock(context.Background(), b.Key()) From 0abc72c0625374c48db5ee7d2e97a4d120fddaff Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 19 Nov 2014 23:32:51 +0000 Subject: [PATCH 16/90] move some variables into strategy --- blockservice/blockservice.go | 5 +++++ exchange/bitswap/bitswap.go | 18 ++++++++++-------- exchange/bitswap/strategy/interface.go | 7 +++++++ exchange/bitswap/strategy/strategy.go | 13 +++++++++++++ net/interface.go | 4 ++++ net/mux/mux.go | 16 ++++++++++++++++ net/net.go | 5 +++++ 7 files changed, 60 insertions(+), 8 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 2eb3d695d..4413eee16 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -96,6 +96,11 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er } } +func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) (<-chan blocks.Block, error) { + // TODO: + return nil, nil +} + // DeleteBlock deletes a block in the blockservice from the datastore func (s *BlockService) DeleteBlock(k u.Key) error { return s.Datastore.Delete(k.DsKey()) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 1539b5fc8..7ad9afb6e 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -157,9 +157,10 @@ func (bs *bitswap) run(ctx context.Context) { rebroadcastTime := time.Second * 5 var providers <-chan peer.Peer // NB: must be initialized to zero value - broadcastSignal := time.After(rebroadcastPeriod) - unsentKeys := 0 + broadcastSignal := time.After(bs.strategy.GetRebroadcastDelay()) + // Number of unsent keys for the current batch + unsentKeys := 0 for { select { case <-broadcastSignal: @@ -170,14 +171,14 @@ func (bs *bitswap) run(ctx context.Context) { if providers == nil { // rely on semi randomness of maps firstKey := wantlist[0] - providers = bs.routing.FindProvidersAsync(ctx, firstKey, 6) + providers = bs.routing.FindProvidersAsync(ctx, firstKey, maxProvidersPerRequest) } err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } providers = nil - broadcastSignal = time.After(rebroadcastPeriod) + broadcastSignal = time.After(bs.strategy.GetRebroadcastDelay()) case k := <-bs.blockRequests: if unsentKeys == 0 { @@ -185,19 +186,19 @@ func (bs *bitswap) run(ctx context.Context) { } unsentKeys++ - if unsentKeys >= numKeysPerBatch { + if unsentKeys >= bs.strategy.GetBatchSize() { // send wantlist to providers err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } unsentKeys = 0 - broadcastSignal = time.After(rebroadcastPeriod) + broadcastSignal = time.After(bs.strategy.GetRebroadcastDelay()) providers = nil } else { // set a timeout to wait for more blocks or send current wantlist - broadcastSignal = time.After(batchDelay) + broadcastSignal = time.After(bs.strategy.GetBatchDelay()) } case <-ctx.Done(): return @@ -217,7 +218,7 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk blocks.Block) error { // TODO(brian): handle errors func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsmsg.BitSwapMessage) ( peer.Peer, bsmsg.BitSwapMessage) { - log.Debugf("ReceiveMessage from %v", p.Key()) + log.Debugf("ReceiveMessage from %s", p) log.Debugf("Message wantlist: %v", incoming.Wantlist()) if p == nil { @@ -239,6 +240,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm for _, block := range incoming.Blocks() { // TODO verify blocks? if err := bs.blockstore.Put(&block); err != nil { + log.Criticalf("error putting block: %s", err) continue // FIXME(brian): err ignored } bs.notifications.Publish(block) diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go index ac1f09a1f..9ac601d70 100644 --- a/exchange/bitswap/strategy/interface.go +++ b/exchange/bitswap/strategy/interface.go @@ -1,6 +1,8 @@ package strategy import ( + "time" + bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" @@ -29,4 +31,9 @@ type Strategy interface { NumBytesSentTo(peer.Peer) uint64 NumBytesReceivedFrom(peer.Peer) uint64 + + // Values determining bitswap behavioural patterns + GetBatchSize() int + GetBatchDelay() time.Duration + GetRebroadcastDelay() time.Duration } diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index 78209c38e..d58894b05 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -3,6 +3,7 @@ package strategy import ( "errors" "sync" + "time" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" peer "github.com/jbenet/go-ipfs/peer" @@ -139,3 +140,15 @@ func (s *strategist) ledger(p peer.Peer) *ledger { } return l } + +func (s *strategist) GetBatchSize() int { + return 10 +} + +func (s *strategist) GetBatchDelay() time.Duration { + return time.Millisecond * 3 +} + +func (s *strategist) GetRebroadcastDelay() time.Duration { + return time.Second * 2 +} diff --git a/net/interface.go b/net/interface.go index 3c0e8d204..60e650c2f 100644 --- a/net/interface.go +++ b/net/interface.go @@ -42,6 +42,10 @@ type Network interface { // the network since it was instantiated GetBandwidthTotals() (uint64, uint64) + // GetMessageCounts returns the total number of messages passed through + // the network since it was instantiated + GetMessageCounts() (uint64, uint64) + // SendMessage sends given Message out SendMessage(msg.NetMessage) error diff --git a/net/mux/mux.go b/net/mux/mux.go index 1b4c06344..d971e9054 100644 --- a/net/mux/mux.go +++ b/net/mux/mux.go @@ -45,9 +45,11 @@ type Muxer struct { bwiLock sync.Mutex bwIn uint64 + msgIn uint64 bwoLock sync.Mutex bwOut uint64 + msgOut uint64 *msg.Pipe ctxc.ContextCloser @@ -76,6 +78,18 @@ func (m *Muxer) GetPipe() *msg.Pipe { return m.Pipe } +// GetMessageCounts return the in/out message count measured over this muxer. +func (m *Muxer) GetMessageCounts() (in uint64, out uint64) { + m.bwiLock.Lock() + in = m.msgIn + m.bwiLock.Unlock() + + m.bwoLock.Lock() + out = m.msgOut + m.bwoLock.Unlock() + return +} + // GetBandwidthTotals return the in/out bandwidth measured over this muxer. func (m *Muxer) GetBandwidthTotals() (in uint64, out uint64) { m.bwiLock.Lock() @@ -125,6 +139,7 @@ func (m *Muxer) handleIncomingMessage(m1 msg.NetMessage) { m.bwiLock.Lock() // TODO: compensate for overhead m.bwIn += uint64(len(m1.Data())) + m.msgIn++ m.bwiLock.Unlock() data, pid, err := unwrapData(m1.Data()) @@ -182,6 +197,7 @@ func (m *Muxer) handleOutgoingMessage(pid pb.ProtocolID, m1 msg.NetMessage) { // TODO: compensate for overhead // TODO(jbenet): switch this to a goroutine to prevent sync waiting. m.bwOut += uint64(len(data)) + m.msgOut++ m.bwoLock.Unlock() m2 := msg.New(m1.Peer(), data) diff --git a/net/net.go b/net/net.go index 0ee1e76b5..3778d839a 100644 --- a/net/net.go +++ b/net/net.go @@ -110,6 +110,11 @@ func (n *IpfsNetwork) GetBandwidthTotals() (in uint64, out uint64) { return n.muxer.GetBandwidthTotals() } +// GetBandwidthTotals returns the total amount of messages transferred +func (n *IpfsNetwork) GetMessageCounts() (in uint64, out uint64) { + return n.muxer.GetMessageCounts() +} + // ListenAddresses returns a list of addresses at which this network listens. func (n *IpfsNetwork) ListenAddresses() []ma.Multiaddr { return n.swarm.ListenAddresses() From e4b2ae3bb2b946e0161a17ec1359d0a819d253c9 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 19 Nov 2014 23:34:40 +0000 Subject: [PATCH 17/90] fix tests halting --- exchange/bitswap/bitswap.go | 4 ++++ routing/mock/routing.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 7ad9afb6e..3115c73bb 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -117,6 +117,9 @@ func (bs *bitswap) GetBlocks(parent context.Context, ks []u.Key) (*blocks.Block, } func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { + if peers == nil { + panic("Cant send wantlist to nil peerchan") + } message := bsmsg.New() for _, wanted := range bs.wantlist.Keys() { message.AddWanted(wanted) @@ -164,6 +167,7 @@ func (bs *bitswap) run(ctx context.Context) { for { select { case <-broadcastSignal: + unsentKeys = 0 wantlist := bs.wantlist.Keys() if len(wantlist) == 0 { continue diff --git a/routing/mock/routing.go b/routing/mock/routing.go index 358d57901..ff83ddca3 100644 --- a/routing/mock/routing.go +++ b/routing/mock/routing.go @@ -59,7 +59,7 @@ func (mr *MockRouter) FindProviders(ctx context.Context, key u.Key) ([]peer.Peer } func (mr *MockRouter) FindPeer(ctx context.Context, pid peer.ID) (peer.Peer, error) { - log.Debug("FindPeer: %s", pid) + log.Debugf("FindPeer: %s", pid) return nil, nil } From 19da05701d88aeef7f5a4ad8584786668d4231c3 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 20 Nov 2014 04:58:26 +0000 Subject: [PATCH 18/90] remove buffer timing in bitswap in favor of manual batching --- exchange/bitswap/bitswap.go | 52 +++++++++++--------------------- exchange/bitswap/bitswap_test.go | 2 +- merkledag/merkledag.go | 20 ++++++++++++ 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 3115c73bb..a497a4594 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -43,7 +43,7 @@ func New(ctx context.Context, p peer.Peer, routing: routing, sender: network, wantlist: u.NewKeySet(), - blockRequests: make(chan u.Key, 32), + batchRequests: make(chan []u.Key, 32), } network.SetDelegate(bs) go bs.run(ctx) @@ -66,7 +66,10 @@ type bitswap struct { notifications notifications.PubSub - blockRequests chan u.Key + // Requests for a set of related blocks + // the assumption is made that the same peer is likely to + // have more than a single block in the set + batchRequests chan []u.Key // strategy listens to network traffic and makes decisions about how to // interact with partners. @@ -97,7 +100,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err promise := bs.notifications.Subscribe(ctx, k) select { - case bs.blockRequests <- k: + case bs.batchRequests <- []u.Key{k}: case <-parent.Done(): return nil, parent.Err() } @@ -159,50 +162,31 @@ func (bs *bitswap) run(ctx context.Context) { // Every so often, we should resend out our current want list rebroadcastTime := time.Second * 5 - var providers <-chan peer.Peer // NB: must be initialized to zero value - broadcastSignal := time.After(bs.strategy.GetRebroadcastDelay()) + broadcastSignal := time.NewTicker(bs.strategy.GetRebroadcastDelay()) - // Number of unsent keys for the current batch - unsentKeys := 0 for { select { - case <-broadcastSignal: - unsentKeys = 0 + case <-broadcastSignal.C: wantlist := bs.wantlist.Keys() if len(wantlist) == 0 { continue } - if providers == nil { - // rely on semi randomness of maps - firstKey := wantlist[0] - providers = bs.routing.FindProvidersAsync(ctx, firstKey, maxProvidersPerRequest) - } + providers := bs.routing.FindProvidersAsync(ctx, wantlist[0], maxProvidersPerRequest) + err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } - providers = nil - broadcastSignal = time.After(bs.strategy.GetRebroadcastDelay()) - - case k := <-bs.blockRequests: - if unsentKeys == 0 { - providers = bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + case ks := <-bs.batchRequests: + if len(ks) == 0 { + log.Warning("Received batch request for zero blocks") + continue } - unsentKeys++ + providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) - if unsentKeys >= bs.strategy.GetBatchSize() { - // send wantlist to providers - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) - } - unsentKeys = 0 - broadcastSignal = time.After(bs.strategy.GetRebroadcastDelay()) - providers = nil - } else { - // set a timeout to wait for more blocks or send current wantlist - - broadcastSignal = time.After(bs.strategy.GetBatchDelay()) + err := bs.sendWantListTo(ctx, providers) + if err != nil { + log.Errorf("error sending wantlist: %s", err) } case <-ctx.Done(): return diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index e3b4d913a..7b4b36fa0 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -345,7 +345,7 @@ func session(net tn.Network, rs mock.RoutingServer, id peer.ID) instance { routing: htc, sender: adapter, wantlist: util.NewKeySet(), - blockRequests: make(chan util.Key, 32), + batchRequests: make(chan []util.Key, 32), } adapter.SetDelegate(bs) go bs.run(context.TODO()) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 1874e5304..d3fe79d9e 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -252,6 +252,7 @@ func (n *dagService) Remove(nd *Node) error { // FetchGraph asynchronously fetches all nodes that are children of the given // node, and returns a channel that may be waited upon for the fetch to complete func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} { + log.Warning("Untested.") var wg sync.WaitGroup done := make(chan struct{}) @@ -284,3 +285,22 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} return done } + +// Take advantage of blockservice/bitswap batched requests to fetch all +// child nodes of a given node +// TODO: finish this +func (ds *dagService) BatchFetch(ctx context.Context, root *Node) error { + var keys []u.Key + for _, lnk := range root.Links { + keys = append(keys, u.Key(lnk.Hash)) + } + + blocks, err := ds.Blocks.GetBlocks(keys) + if err != nil { + return err + } + + _ = blocks + //what do i do with blocks? + return nil +} From 297ff3d4b24f553865e61b6788085dc33e3127b1 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 20 Nov 2014 06:16:53 +0000 Subject: [PATCH 19/90] randomize rebroadcast target --- exchange/bitswap/bitswap.go | 9 +++++++-- exchange/bitswap/strategy/interface.go | 1 - exchange/bitswap/strategy/strategy.go | 4 ---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index a497a4594..35346644b 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -3,6 +3,7 @@ package bitswap import ( + "math/rand" "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" @@ -96,7 +97,6 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err log.Event(ctx, "GetBlockRequestBegin", &k) defer log.Event(ctx, "GetBlockRequestEnd", &k) - bs.wantlist.Add(k) promise := bs.notifications.Subscribe(ctx, k) select { @@ -171,17 +171,22 @@ func (bs *bitswap) run(ctx context.Context) { if len(wantlist) == 0 { continue } - providers := bs.routing.FindProvidersAsync(ctx, wantlist[0], maxProvidersPerRequest) + n := rand.Intn(len(wantlist)) + providers := bs.routing.FindProvidersAsync(ctx, wantlist[n], maxProvidersPerRequest) err := bs.sendWantListTo(ctx, providers) if err != nil { log.Errorf("error sending wantlist: %s", err) } case ks := <-bs.batchRequests: + // TODO: implement batching on len(ks) > X for some X if len(ks) == 0 { log.Warning("Received batch request for zero blocks") continue } + for _, k := range ks { + bs.wantlist.Add(k) + } providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) err := bs.sendWantListTo(ctx, providers) diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go index 9ac601d70..503a50d41 100644 --- a/exchange/bitswap/strategy/interface.go +++ b/exchange/bitswap/strategy/interface.go @@ -34,6 +34,5 @@ type Strategy interface { // Values determining bitswap behavioural patterns GetBatchSize() int - GetBatchDelay() time.Duration GetRebroadcastDelay() time.Duration } diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index d58894b05..ad69b841a 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -145,10 +145,6 @@ func (s *strategist) GetBatchSize() int { return 10 } -func (s *strategist) GetBatchDelay() time.Duration { - return time.Millisecond * 3 -} - func (s *strategist) GetRebroadcastDelay() time.Duration { return time.Second * 2 } From 85229be43a2b274bb120388960f1457ee44a24ad Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 12:56:23 -0800 Subject: [PATCH 20/90] style(bitswap/notifications) make it more obvious License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/notifications/notifications.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index 34888d510..bd30bbad6 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -8,6 +8,8 @@ import ( u "github.com/jbenet/go-ipfs/util" ) +const bufferSize = 16 + type PubSub interface { Publish(block blocks.Block) Subscribe(ctx context.Context, k u.Key) <-chan blocks.Block @@ -15,7 +17,6 @@ type PubSub interface { } func New() PubSub { - const bufferSize = 16 return &impl{*pubsub.New(bufferSize)} } From 8f8230823f898184a55e488edb0e976ec1d33f5f Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 14:19:22 -0800 Subject: [PATCH 21/90] feat(bitswap/notifications) Subscribe to multiple keys License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/notifications/notifications.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index bd30bbad6..a2646c814 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -12,7 +12,7 @@ const bufferSize = 16 type PubSub interface { Publish(block blocks.Block) - Subscribe(ctx context.Context, k u.Key) <-chan blocks.Block + Subscribe(ctx context.Context, keys ...u.Key) <-chan blocks.Block Shutdown() } @@ -31,10 +31,13 @@ func (ps *impl) Publish(block blocks.Block) { // Subscribe returns a one-time use |blockChannel|. |blockChannel| returns nil // if the |ctx| times out or is cancelled. Then channel is closed after the -// block given by |k| is sent. -func (ps *impl) Subscribe(ctx context.Context, k u.Key) <-chan blocks.Block { - topic := string(k) - subChan := ps.wrapped.SubOnce(topic) +// blocks given by |keys| are sent. +func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan blocks.Block { + topics := make([]string, 0) + for _, key := range keys { + topics = append(topics, string(key)) + } + subChan := ps.wrapped.SubOnce(topics...) blockChannel := make(chan blocks.Block, 1) // buffered so the sender doesn't wait on receiver go func() { defer close(blockChannel) @@ -45,7 +48,7 @@ func (ps *impl) Subscribe(ctx context.Context, k u.Key) <-chan blocks.Block { blockChannel <- block } case <-ctx.Done(): - ps.wrapped.Unsub(subChan, topic) + ps.wrapped.Unsub(subChan, topics...) } }() return blockChannel From 81a3ba067720496d816aa91cc8fecf5eaa5776f9 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 22:51:34 -0800 Subject: [PATCH 22/90] tests(bitswap) share constructor between tests @whyrusleeping i hope this makes it a bit easier to work with tests License: MIT Signed-off-by: Brian Tiger Chow --- core/core.go | 8 +++++--- exchange/bitswap/bitswap.go | 8 +++----- exchange/bitswap/bitswap_test.go | 23 +++++++---------------- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/core/core.go b/core/core.go index 58d99b6f4..4553a980d 100644 --- a/core/core.go +++ b/core/core.go @@ -9,6 +9,7 @@ import ( ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" bserv "github.com/jbenet/go-ipfs/blockservice" + blockstore "github.com/jbenet/go-ipfs/blockstore" config "github.com/jbenet/go-ipfs/config" diag "github.com/jbenet/go-ipfs/diagnostics" exchange "github.com/jbenet/go-ipfs/exchange" @@ -28,8 +29,7 @@ import ( dht "github.com/jbenet/go-ipfs/routing/dht" u "github.com/jbenet/go-ipfs/util" ctxc "github.com/jbenet/go-ipfs/util/ctxcloser" - "github.com/jbenet/go-ipfs/util/debugerror" - "github.com/jbenet/go-ipfs/util/eventlog" + debugerror "github.com/jbenet/go-ipfs/util/debugerror" ) const IpnsValidatorTag = "ipns" @@ -169,7 +169,9 @@ func NewIpfsNode(cfg *config.Config, online bool) (n *IpfsNode, err error) { // setup exchange service const alwaysSendToPeer = true // use YesManStrategy bitswapNetwork := bsnet.NewFromIpfsNetwork(exchangeService, n.Network) - n.Exchange = bitswap.New(ctx, n.Identity, bitswapNetwork, n.Routing, n.Datastore, alwaysSendToPeer) + bstore := blockstore.NewBlockstore(n.Datastore) + + n.Exchange = bitswap.New(ctx, n.Identity, bitswapNetwork, n.Routing, bstore, alwaysSendToPeer) go initConnections(ctx, n.Config, n.Peerstore, dhtRouting) } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 35346644b..a14d68cc0 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -7,7 +7,6 @@ import ( "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" blocks "github.com/jbenet/go-ipfs/blocks" blockstore "github.com/jbenet/go-ipfs/blockstore" @@ -27,9 +26,8 @@ var log = eventlog.Logger("bitswap") // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. // Runs until context is cancelled -func New(ctx context.Context, p peer.Peer, - network bsnet.BitSwapNetwork, routing bsnet.Routing, - d ds.ThreadSafeDatastore, nice bool) exchange.Interface { +func New(ctx context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing bsnet.Routing, + bstore blockstore.Blockstore, nice bool) exchange.Interface { notif := notifications.New() go func() { @@ -38,7 +36,7 @@ func New(ctx context.Context, p peer.Peer, }() bs := &bitswap{ - blockstore: blockstore.NewBlockstore(d), + blockstore: bstore, notifications: notif, strategy: strategy.New(nice), routing: routing, diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 7b4b36fa0..78509e649 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -11,14 +11,12 @@ import ( ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" + blockstore "github.com/jbenet/go-ipfs/blockstore" bstore "github.com/jbenet/go-ipfs/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" - notifications "github.com/jbenet/go-ipfs/exchange/bitswap/notifications" - strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" peer "github.com/jbenet/go-ipfs/peer" mock "github.com/jbenet/go-ipfs/routing/mock" - util "github.com/jbenet/go-ipfs/util" ) func TestGetBlockTimeout(t *testing.T) { @@ -335,23 +333,16 @@ func session(net tn.Network, rs mock.RoutingServer, id peer.ID) instance { adapter := net.Adapter(p) htc := rs.Client(p) + bstore := blockstore.NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore())) - blockstore := bstore.NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore())) const alwaysSendToPeer = true - bs := &bitswap{ - blockstore: blockstore, - notifications: notifications.New(), - strategy: strategy.New(alwaysSendToPeer), - routing: htc, - sender: adapter, - wantlist: util.NewKeySet(), - batchRequests: make(chan []util.Key, 32), - } - adapter.SetDelegate(bs) - go bs.run(context.TODO()) + ctx := context.TODO() + + bs := New(ctx, p, adapter, htc, bstore, alwaysSendToPeer) + return instance{ peer: p, exchange: bs, - blockstore: blockstore, + blockstore: bstore, } } From cc92ec3d9c9a7436143674e88faea9fae286e924 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 23:11:06 -0800 Subject: [PATCH 23/90] fix(merkledag) missing arg License: MIT Signed-off-by: Brian Tiger Chow --- merkledag/merkledag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index d3fe79d9e..9d3e6ce95 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -295,7 +295,7 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) error { keys = append(keys, u.Key(lnk.Hash)) } - blocks, err := ds.Blocks.GetBlocks(keys) + blocks, err := ds.Blocks.GetBlocks(ctx, keys) if err != nil { return err } From b13a5a940b40e6b24c1031275905dc763e667c2e Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 23:11:58 -0800 Subject: [PATCH 24/90] refactor(bitswap) move wantlist to loop receive License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index a14d68cc0..608656e53 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -178,6 +178,9 @@ func (bs *bitswap) run(ctx context.Context) { } case ks := <-bs.batchRequests: // TODO: implement batching on len(ks) > X for some X + for _, k := range ks { + bs.wantlist.Add(k) + } if len(ks) == 0 { log.Warning("Received batch request for zero blocks") continue From 11f2856d31ad8424d0024a92c3e62810800a7627 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 23:27:08 -0800 Subject: [PATCH 25/90] feat(bitswap) implement GetBlocks @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 608656e53..6ff604134 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -79,9 +79,7 @@ type bitswap struct { } // GetBlock attempts to retrieve a particular block from peers within the -// deadline enforced by the context -// -// TODO ensure only one active request per key +// deadline enforced by the context. func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, error) { // make sure to derive a new |ctx| and pass it to children. It's correct to @@ -95,26 +93,36 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err log.Event(ctx, "GetBlockRequestBegin", &k) defer log.Event(ctx, "GetBlockRequestEnd", &k) - promise := bs.notifications.Subscribe(ctx, k) - - select { - case bs.batchRequests <- []u.Key{k}: - case <-parent.Done(): - return nil, parent.Err() + promise, err := bs.GetBlocks(parent, []u.Key{k}) + if err != nil { + return nil, err } select { case block := <-promise: - bs.wantlist.Remove(k) return &block, nil case <-parent.Done(): return nil, parent.Err() } } -func (bs *bitswap) GetBlocks(parent context.Context, ks []u.Key) (*blocks.Block, error) { - // TODO: something smart - return nil, nil +// GetBlocks returns a channel where the caller may receive blocks that +// correspond to the provided |keys|. Returns an error if BitSwap is unable to +// begin this request within the deadline enforced by the context. +// +// NB: Your request remains open until the context expires. To conserve +// resources, provide a context with a reasonably short deadline (ie. not one +// that lasts throughout the lifetime of the server) +func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan blocks.Block, error) { + // TODO log the request + + promise := bs.notifications.Subscribe(ctx, keys...) + select { + case bs.batchRequests <- keys: + return promise, nil + case <-ctx.Done(): + return nil, ctx.Err() + } } func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { @@ -155,6 +163,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e return nil } +// TODO ensure only one active request per key func (bs *bitswap) run(ctx context.Context) { // Every so often, we should resend out our current want list @@ -238,6 +247,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm continue // FIXME(brian): err ignored } bs.notifications.Publish(block) + bs.wantlist.Remove(block.Key()) err := bs.HasBlock(ctx, block) if err != nil { log.Warningf("HasBlock errored: %s", err) From a5754a5ff4d06a767b7f0049c19c9d9a4f0d4e99 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 19 Nov 2014 23:34:14 -0800 Subject: [PATCH 26/90] fix(bitswap) stop the ticker when the run loop exits @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 1 + 1 file changed, 1 insertion(+) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 6ff604134..97fd0576f 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -170,6 +170,7 @@ func (bs *bitswap) run(ctx context.Context) { rebroadcastTime := time.Second * 5 broadcastSignal := time.NewTicker(bs.strategy.GetRebroadcastDelay()) + defer broadcastSignal.Stop() for { select { From 5babfb975de888f312536c87a19c92bb5f334b06 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 00:02:20 -0800 Subject: [PATCH 27/90] tests(bitswap) share code between the two large tests License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap_test.go | 60 +++++++------------------------- 1 file changed, 13 insertions(+), 47 deletions(-) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 78509e649..ce881f846 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -87,58 +87,27 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { } } -func TestSwarm(t *testing.T) { +func TestLargeSwarm(t *testing.T) { if testing.Short() { t.SkipNow() } - net := tn.VirtualNetwork() - rs := mock.VirtualRoutingServer() - sg := NewSessionGenerator(net, rs) - bg := NewBlockGenerator() - - t.Log("Create a ton of instances, and just a few blocks") - + t.Parallel() numInstances := 500 numBlocks := 2 - - instances := sg.Instances(numInstances) - blocks := bg.Blocks(numBlocks) - - t.Log("Give the blocks to the first instance") - - first := instances[0] - for _, b := range blocks { - first.blockstore.Put(b) - first.exchange.HasBlock(context.Background(), *b) - rs.Announce(first.peer, b.Key()) - } - - t.Log("Distribute!") - - var wg sync.WaitGroup - - for _, inst := range instances { - for _, b := range blocks { - wg.Add(1) - // NB: executing getOrFail concurrently puts tremendous pressure on - // the goroutine scheduler - getOrFail(inst, b, t, &wg) - } - } - wg.Wait() - - t.Log("Verify!") - - for _, inst := range instances { - for _, b := range blocks { - if _, err := inst.blockstore.Get(b.Key()); err != nil { - t.Fatal(err) - } - } - } + PerformDistributionTest(t, numInstances, numBlocks) } func TestLargeFile(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + t.Parallel() + numInstances := 10 + numBlocks := 100 + PerformDistributionTest(t, numInstances, numBlocks) +} + +func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { if testing.Short() { t.SkipNow() } @@ -149,9 +118,6 @@ func TestLargeFile(t *testing.T) { t.Log("Test a few nodes trying to get one file with a lot of blocks") - numInstances := 10 - numBlocks := 100 - instances := sg.Instances(numInstances) blocks := bg.Blocks(numBlocks) From 99ae432021fd2987d9d7e018676605962eca0975 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 01:15:32 +0000 Subject: [PATCH 28/90] start working getBlocks up the call chain --- blockservice/blockservice.go | 26 +++++++++++++-- merkledag/merkledag.go | 62 +++++++++++++++++++++++++++++------- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 4413eee16..279e3ffd9 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -96,9 +96,29 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er } } -func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) (<-chan blocks.Block, error) { - // TODO: - return nil, nil +func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { + out := make(chan *blocks.Block, 32) + go func() { + var toFetch []u.Key + for _, k := range ks { + datai, err := s.Datastore.Get(k.DsKey()) + if err == nil { + log.Debug("Blockservice: Got data in datastore.") + bdata, ok := datai.([]byte) + if !ok { + log.Criticalf("data associated with %s is not a []byte", k) + continue + } + out <- &blocks.Block{ + Multihash: mh.Multihash(k), + Data: bdata, + } + } else { + toFetch = append(toFetch, k) + } + } + }() + return out } // DeleteBlock deletes a block in the blockservice from the datastore diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 9d3e6ce95..23eaf8881 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -289,18 +289,56 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} // Take advantage of blockservice/bitswap batched requests to fetch all // child nodes of a given node // TODO: finish this -func (ds *dagService) BatchFetch(ctx context.Context, root *Node) error { - var keys []u.Key - for _, lnk := range root.Links { - keys = append(keys, u.Key(lnk.Hash)) - } +func (ds *dagService) BatchFetch(ctx context.Context, root *Node) chan struct{} { + sig := make(chan struct{}) + go func() { + var keys []u.Key + for _, lnk := range root.Links { + keys = append(keys, u.Key(lnk.Hash)) + } - blocks, err := ds.Blocks.GetBlocks(ctx, keys) - if err != nil { - return err - } + blkchan := ds.Blocks.GetBlocks(ctx, keys) - _ = blocks - //what do i do with blocks? - return nil + // + next := 0 + seen := make(map[int]struct{}) + // + + for blk := range blkchan { + for i, lnk := range root.Links { + + // + seen[i] = struct{}{} + // + + if u.Key(lnk.Hash) != blk.Key() { + continue + } + nd, err := Decoded(blk.Data) + if err != nil { + log.Error("Got back bad block!") + break + } + lnk.Node = nd + + // + if next == i { + sig <- struct{}{} + next++ + for { + if _, ok := seen[next]; ok { + sig <- struct{}{} + next++ + } else { + break + } + } + } + // + } + } + }() + + // TODO: return a channel, and signal when the 'Next' readable block is available + return sig } From b1b42a560910e60eceb047f56676851e5693fb38 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 01:20:29 +0000 Subject: [PATCH 29/90] change BatchFetch to return indices --- merkledag/merkledag.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 23eaf8881..2483ce4d9 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -289,8 +289,8 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} // Take advantage of blockservice/bitswap batched requests to fetch all // child nodes of a given node // TODO: finish this -func (ds *dagService) BatchFetch(ctx context.Context, root *Node) chan struct{} { - sig := make(chan struct{}) +func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan int { + sig := make(chan int) go func() { var keys []u.Key for _, lnk := range root.Links { @@ -323,11 +323,11 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) chan struct{} // if next == i { - sig <- struct{}{} + sig <- next next++ for { if _, ok := seen[next]; ok { - sig <- struct{}{} + sig <- next next++ } else { break From 918c8e274e7bfc3dcb3f5575f8cca0d73b9039e1 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 17:27:48 -0800 Subject: [PATCH 30/90] refactor(blockstore) mv under blocks/ @jbenet @whyrusleeping the pyramids were built one brick at a time addresses: https://github.com/jbenet/go-ipfs/issues/370 License: MIT Signed-off-by: Brian Tiger Chow --- {blockstore => blocks/blockstore}/blockstore.go | 0 {blockstore => blocks/blockstore}/blockstore_test.go | 0 core/core.go | 2 +- exchange/bitswap/bitswap.go | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) rename {blockstore => blocks/blockstore}/blockstore.go (100%) rename {blockstore => blocks/blockstore}/blockstore_test.go (100%) diff --git a/blockstore/blockstore.go b/blocks/blockstore/blockstore.go similarity index 100% rename from blockstore/blockstore.go rename to blocks/blockstore/blockstore.go diff --git a/blockstore/blockstore_test.go b/blocks/blockstore/blockstore_test.go similarity index 100% rename from blockstore/blockstore_test.go rename to blocks/blockstore/blockstore_test.go diff --git a/core/core.go b/core/core.go index 4553a980d..a0d07d9af 100644 --- a/core/core.go +++ b/core/core.go @@ -8,8 +8,8 @@ import ( b58 "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-base58" ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" + blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" bserv "github.com/jbenet/go-ipfs/blockservice" - blockstore "github.com/jbenet/go-ipfs/blockstore" config "github.com/jbenet/go-ipfs/config" diag "github.com/jbenet/go-ipfs/diagnostics" exchange "github.com/jbenet/go-ipfs/exchange" diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 97fd0576f..d47ea81a4 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -9,7 +9,7 @@ import ( context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" blocks "github.com/jbenet/go-ipfs/blocks" - blockstore "github.com/jbenet/go-ipfs/blockstore" + blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" bsnet "github.com/jbenet/go-ipfs/exchange/bitswap/network" From eb0bde052e7b66e98ce26e77394cc54127470252 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 17:41:22 -0800 Subject: [PATCH 31/90] rename License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index ce881f846..52dad14b5 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -11,8 +11,7 @@ import ( ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" - blockstore "github.com/jbenet/go-ipfs/blockstore" - bstore "github.com/jbenet/go-ipfs/blockstore" + blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" peer "github.com/jbenet/go-ipfs/peer" @@ -286,7 +285,7 @@ func (g *SessionGenerator) Instances(n int) []instance { type instance struct { peer peer.Peer exchange exchange.Interface - blockstore bstore.Blockstore + blockstore blockstore.Blockstore } // session creates a test bitswap session. From 04a8a6133c2ab7a0221a6761256c772f255991ab Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 17:41:47 -0800 Subject: [PATCH 32/90] rename exchange License: MIT Signed-off-by: Brian Tiger Chow --- exchange/offline/offline.go | 2 +- exchange/offline/offline_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/exchange/offline/offline.go b/exchange/offline/offline.go index 37d672f73..fbb3485c0 100644 --- a/exchange/offline/offline.go +++ b/exchange/offline/offline.go @@ -14,7 +14,7 @@ import ( var OfflineMode = errors.New("Block unavailable. Operating in offline mode") -func NewOfflineExchange() exchange.Interface { +func Exchange() exchange.Interface { return &offlineExchange{} } diff --git a/exchange/offline/offline_test.go b/exchange/offline/offline_test.go index ae3fdaa0a..98b6e1a8c 100644 --- a/exchange/offline/offline_test.go +++ b/exchange/offline/offline_test.go @@ -10,7 +10,7 @@ import ( ) func TestBlockReturnsErr(t *testing.T) { - off := NewOfflineExchange() + off := Exchange() _, err := off.GetBlock(context.Background(), u.Key("foo")) if err != nil { return // as desired @@ -19,7 +19,7 @@ func TestBlockReturnsErr(t *testing.T) { } func TestHasBlockReturnsNil(t *testing.T) { - off := NewOfflineExchange() + off := Exchange() block := blocks.NewBlock([]byte("data")) err := off.HasBlock(context.Background(), *block) if err != nil { From d0304def6beec57e6e5bc2c8f5325f8f2a4ef71b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:08:43 -0800 Subject: [PATCH 33/90] refactor(blockstore, blockservice) use Blockstore and offline.Exchange License: MIT Signed-off-by: Brian Tiger Chow --- blocks/blockstore/blockstore.go | 10 ++++++ blockservice/blocks_test.go | 7 ++-- blockservice/blockservice.go | 59 ++++++++++++++------------------- core/core.go | 5 ++- core/mock.go | 8 ++--- pin/pin_test.go | 7 +++- unixfs/io/dagmodifier_test.go | 7 +++- util/testutil/gen.go | 6 +++- 8 files changed, 65 insertions(+), 44 deletions(-) diff --git a/blocks/blockstore/blockstore.go b/blocks/blockstore/blockstore.go index b4c0fd7fb..68ccc7c74 100644 --- a/blocks/blockstore/blockstore.go +++ b/blocks/blockstore/blockstore.go @@ -15,6 +15,8 @@ import ( var ValueTypeMismatch = errors.New("The retrieved value is not a Block") type Blockstore interface { + DeleteBlock(u.Key) error + Has(u.Key) (bool, error) Get(u.Key) (*blocks.Block, error) Put(*blocks.Block) error } @@ -45,3 +47,11 @@ func (bs *blockstore) Get(k u.Key) (*blocks.Block, error) { func (bs *blockstore) Put(block *blocks.Block) error { return bs.datastore.Put(block.Key().DsKey(), block.Data) } + +func (bs *blockstore) Has(k u.Key) (bool, error) { + return bs.datastore.Has(k.DsKey()) +} + +func (s *blockstore) DeleteBlock(k u.Key) error { + return s.datastore.Delete(k.DsKey()) +} diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index 1e837eb5d..9f579c530 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -6,15 +6,18 @@ import ( "time" "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" + dssync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" + blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" + offline "github.com/jbenet/go-ipfs/exchange/offline" u "github.com/jbenet/go-ipfs/util" ) func TestBlocks(t *testing.T) { d := ds.NewMapDatastore() - bs, err := NewBlockService(d, nil) + tsds := dssync.MutexWrap(d) + bs, err := New(blockstore.NewBlockstore(tsds), offline.Exchange()) if err != nil { t.Error("failed to construct block service", err) return diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 279e3ffd9..86e0c776b 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -9,9 +9,9 @@ import ( context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" - mh "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multihash" blocks "github.com/jbenet/go-ipfs/blocks" + "github.com/jbenet/go-ipfs/blocks/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" u "github.com/jbenet/go-ipfs/util" ) @@ -19,25 +19,28 @@ import ( var log = u.Logger("blockservice") var ErrNotFound = errors.New("blockservice: key not found") -// BlockService is a block datastore. +// BlockService is a hybrid block datastore. It stores data in a local +// datastore and may retrieve data from a remote Exchange. // It uses an internal `datastore.Datastore` instance to store values. type BlockService struct { - Datastore ds.Datastore - Remote exchange.Interface + // TODO don't expose underlying impl details + Blockstore blockstore.Blockstore + Remote exchange.Interface } // NewBlockService creates a BlockService with given datastore instance. -func NewBlockService(d ds.Datastore, rem exchange.Interface) (*BlockService, error) { - if d == nil { - return nil, fmt.Errorf("BlockService requires valid datastore") +func New(bs blockstore.Blockstore, rem exchange.Interface) (*BlockService, error) { + if bs == nil { + return nil, fmt.Errorf("BlockService requires valid blockstore") } if rem == nil { log.Warning("blockservice running in local (offline) mode.") } - return &BlockService{Datastore: d, Remote: rem}, nil + return &BlockService{Blockstore: bs, Remote: rem}, nil } // AddBlock adds a particular block to the service, Putting it into the datastore. +// TODO pass a context into this if the remote.HasBlock is going to remain here. func (s *BlockService) AddBlock(b *blocks.Block) (u.Key, error) { k := b.Key() log.Debugf("blockservice: storing [%s] in datastore", k) @@ -47,7 +50,7 @@ func (s *BlockService) AddBlock(b *blocks.Block) (u.Key, error) { // check if we have it before adding. this is an extra read, but large writes // are more expensive. // TODO(jbenet) cheaper has. https://github.com/jbenet/go-datastore/issues/6 - has, err := s.Datastore.Has(k.DsKey()) + has, err := s.Blockstore.Has(k) if err != nil { return k, err } @@ -55,12 +58,14 @@ func (s *BlockService) AddBlock(b *blocks.Block) (u.Key, error) { log.Debugf("blockservice: storing [%s] in datastore (already stored)", k) } else { log.Debugf("blockservice: storing [%s] in datastore", k) - err := s.Datastore.Put(k.DsKey(), b.Data) + err := s.Blockstore.Put(b) if err != nil { return k, err } } + // TODO this operation rate-limits blockservice operations, we should + // consider moving this to an sync process. if s.Remote != nil { ctx := context.TODO() err = s.Remote.HasBlock(ctx, *b) @@ -72,17 +77,11 @@ func (s *BlockService) AddBlock(b *blocks.Block) (u.Key, error) { // Getting it from the datastore using the key (hash). func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, error) { log.Debugf("BlockService GetBlock: '%s'", k) - datai, err := s.Datastore.Get(k.DsKey()) + block, err := s.Blockstore.Get(k) if err == nil { - log.Debug("Blockservice: Got data in datastore.") - bdata, ok := datai.([]byte) - if !ok { - return nil, fmt.Errorf("data associated with %s is not a []byte", k) - } - return &blocks.Block{ - Multihash: mh.Multihash(k), - Data: bdata, - }, nil + return block, nil + // TODO be careful checking ErrNotFound. If the underlying + // implementation changes, this will break. } else if err == ds.ErrNotFound && s.Remote != nil { log.Debug("Blockservice: Searching bitswap.") blk, err := s.Remote.GetBlock(ctx, k) @@ -101,21 +100,13 @@ func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks go func() { var toFetch []u.Key for _, k := range ks { - datai, err := s.Datastore.Get(k.DsKey()) - if err == nil { - log.Debug("Blockservice: Got data in datastore.") - bdata, ok := datai.([]byte) - if !ok { - log.Criticalf("data associated with %s is not a []byte", k) - continue - } - out <- &blocks.Block{ - Multihash: mh.Multihash(k), - Data: bdata, - } - } else { + block, err := s.Blockstore.Get(k) + if err != nil { toFetch = append(toFetch, k) + continue } + log.Debug("Blockservice: Got data in datastore.") + out <- block } }() return out @@ -123,5 +114,5 @@ func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks // DeleteBlock deletes a block in the blockservice from the datastore func (s *BlockService) DeleteBlock(k u.Key) error { - return s.Datastore.Delete(k.DsKey()) + return s.Blockstore.DeleteBlock(k) } diff --git a/core/core.go b/core/core.go index a0d07d9af..fcd421fc6 100644 --- a/core/core.go +++ b/core/core.go @@ -15,6 +15,7 @@ import ( exchange "github.com/jbenet/go-ipfs/exchange" bitswap "github.com/jbenet/go-ipfs/exchange/bitswap" bsnet "github.com/jbenet/go-ipfs/exchange/bitswap/network" + "github.com/jbenet/go-ipfs/exchange/offline" mount "github.com/jbenet/go-ipfs/fuse/mount" merkledag "github.com/jbenet/go-ipfs/merkledag" namesys "github.com/jbenet/go-ipfs/namesys" @@ -127,6 +128,8 @@ func NewIpfsNode(cfg *config.Config, online bool) (n *IpfsNode, err error) { return nil, debugerror.Wrap(err) } + n.Exchange = offline.Exchange() + // setup online services if online { @@ -178,7 +181,7 @@ func NewIpfsNode(cfg *config.Config, online bool) (n *IpfsNode, err error) { // TODO(brian): when offline instantiate the BlockService with a bitswap // session that simply doesn't return blocks - n.Blocks, err = bserv.NewBlockService(n.Datastore, n.Exchange) + n.Blocks, err = bserv.New(blockstore.NewBlockstore(n.Datastore), n.Exchange) if err != nil { return nil, debugerror.Wrap(err) } diff --git a/core/mock.go b/core/mock.go index 92ffcb57d..2d2359277 100644 --- a/core/mock.go +++ b/core/mock.go @@ -3,8 +3,10 @@ package core import ( ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" syncds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" - bs "github.com/jbenet/go-ipfs/blockservice" + "github.com/jbenet/go-ipfs/blocks/blockstore" + blockservice "github.com/jbenet/go-ipfs/blockservice" ci "github.com/jbenet/go-ipfs/crypto" + "github.com/jbenet/go-ipfs/exchange/offline" mdag "github.com/jbenet/go-ipfs/merkledag" nsys "github.com/jbenet/go-ipfs/namesys" path "github.com/jbenet/go-ipfs/path" @@ -43,9 +45,7 @@ func NewMockNode() (*IpfsNode, error) { nd.Routing = dht // Bitswap - //?? - - bserv, err := bs.NewBlockService(nd.Datastore, nil) + bserv, err := blockservice.New(blockstore.NewBlockstore(nd.Datastore), offline.Exchange()) if err != nil { return nil, err } diff --git a/pin/pin_test.go b/pin/pin_test.go index 1ea302823..fc9dc215d 100644 --- a/pin/pin_test.go +++ b/pin/pin_test.go @@ -4,7 +4,10 @@ import ( "testing" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" + dssync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" + "github.com/jbenet/go-ipfs/blocks/blockstore" bs "github.com/jbenet/go-ipfs/blockservice" + "github.com/jbenet/go-ipfs/exchange/offline" mdag "github.com/jbenet/go-ipfs/merkledag" "github.com/jbenet/go-ipfs/util" ) @@ -19,13 +22,15 @@ func randNode() (*mdag.Node, util.Key) { func TestPinnerBasic(t *testing.T) { dstore := ds.NewMapDatastore() - bserv, err := bs.NewBlockService(dstore, nil) + bstore := blockstore.NewBlockstore(dssync.MutexWrap(dstore)) + bserv, err := bs.New(bstore, offline.Exchange()) if err != nil { t.Fatal(err) } dserv := mdag.NewDAGService(bserv) + // TODO does pinner need to share datastore with blockservice? p := NewPinner(dstore, dserv) a, ak := randNode() diff --git a/unixfs/io/dagmodifier_test.go b/unixfs/io/dagmodifier_test.go index 822c87471..d0aa83795 100644 --- a/unixfs/io/dagmodifier_test.go +++ b/unixfs/io/dagmodifier_test.go @@ -6,7 +6,10 @@ import ( "io/ioutil" "testing" + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" + "github.com/jbenet/go-ipfs/blocks/blockstore" bs "github.com/jbenet/go-ipfs/blockservice" + "github.com/jbenet/go-ipfs/exchange/offline" imp "github.com/jbenet/go-ipfs/importer" "github.com/jbenet/go-ipfs/importer/chunk" mdag "github.com/jbenet/go-ipfs/merkledag" @@ -19,7 +22,9 @@ import ( func getMockDagServ(t *testing.T) mdag.DAGService { dstore := ds.NewMapDatastore() - bserv, err := bs.NewBlockService(dstore, nil) + tsds := sync.MutexWrap(dstore) + bstore := blockstore.NewBlockstore(tsds) + bserv, err := bs.New(bstore, offline.Exchange()) if err != nil { t.Fatal(err) } diff --git a/util/testutil/gen.go b/util/testutil/gen.go index be2fe5988..c7f310c2a 100644 --- a/util/testutil/gen.go +++ b/util/testutil/gen.go @@ -4,9 +4,12 @@ import ( crand "crypto/rand" "testing" + dssync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" + "github.com/jbenet/go-ipfs/exchange/offline" "github.com/jbenet/go-ipfs/peer" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" + "github.com/jbenet/go-ipfs/blocks/blockstore" bsrv "github.com/jbenet/go-ipfs/blockservice" dag "github.com/jbenet/go-ipfs/merkledag" u "github.com/jbenet/go-ipfs/util" @@ -14,7 +17,8 @@ import ( func GetDAGServ(t testing.TB) dag.DAGService { dstore := ds.NewMapDatastore() - bserv, err := bsrv.NewBlockService(dstore, nil) + tsds := dssync.MutexWrap(dstore) + bserv, err := bsrv.New(blockstore.NewBlockstore(tsds), offline.Exchange()) if err != nil { t.Fatal(err) } From 7c2053c3c84439c351591934f2928ecc2db5a701 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:14:36 -0800 Subject: [PATCH 34/90] fix(bitswap/loop) add to wantlist just once oops set Add is idempotent but it's a waste of resources License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index d47ea81a4..3ff301448 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -188,9 +188,6 @@ func (bs *bitswap) run(ctx context.Context) { } case ks := <-bs.batchRequests: // TODO: implement batching on len(ks) > X for some X - for _, k := range ks { - bs.wantlist.Add(k) - } if len(ks) == 0 { log.Warning("Received batch request for zero blocks") continue From ecf62dbf3a52f3742bd62cf2394fc77513e3c85b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:19:48 -0800 Subject: [PATCH 35/90] feat(bitswap) find providers for all keys on wantlist @jbenet @whyrusleeping this addresses a failure case where 1) bitswap wants blocks A and B 2) partner 1 has A and partner 2 has B 3) We choose a key at random, drawing A. 4) Then, we request A, neglecting to find a provider for B. Sending the full wantlist is meant to be used as a helpful additional piece of data, but... unless our hunch is support by statistical inference at runtime, it's not safe to assume that a peer will have blocks for related keys. Routing must be the source of truth. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 3ff301448..3c0f93119 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -3,7 +3,6 @@ package bitswap import ( - "math/rand" "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" @@ -175,16 +174,12 @@ func (bs *bitswap) run(ctx context.Context) { for { select { case <-broadcastSignal.C: - wantlist := bs.wantlist.Keys() - if len(wantlist) == 0 { - continue - } - n := rand.Intn(len(wantlist)) - providers := bs.routing.FindProvidersAsync(ctx, wantlist[n], maxProvidersPerRequest) - - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) + for _, k := range bs.wantlist.Keys() { + providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + err := bs.sendWantListTo(ctx, providers) + if err != nil { + log.Errorf("error sending wantlist: %s", err) + } } case ks := <-bs.batchRequests: // TODO: implement batching on len(ks) > X for some X From f6cb4ab9a2ddf06411d1f2d2e9b1858d3d732fd9 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:25:56 -0800 Subject: [PATCH 36/90] feat(bitswap) loop over all provided keys License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 3c0f93119..ed7155b6d 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -182,19 +182,13 @@ func (bs *bitswap) run(ctx context.Context) { } } case ks := <-bs.batchRequests: - // TODO: implement batching on len(ks) > X for some X - if len(ks) == 0 { - log.Warning("Received batch request for zero blocks") - continue - } for _, k := range ks { bs.wantlist.Add(k) - } - providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) - - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) + providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + err := bs.sendWantListTo(ctx, providers) + if err != nil { + log.Errorf("error sending wantlist: %s", err) + } } case <-ctx.Done(): return From 5bd3b178b665600db57019acf5a610073ed34435 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:27:05 -0800 Subject: [PATCH 37/90] style(bitswap) name -> loop eh? License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index ed7155b6d..1c1982edc 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -44,7 +44,7 @@ func New(ctx context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing batchRequests: make(chan []u.Key, 32), } network.SetDelegate(bs) - go bs.run(ctx) + go bs.loop(ctx) return bs } @@ -163,7 +163,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } // TODO ensure only one active request per key -func (bs *bitswap) run(ctx context.Context) { +func (bs *bitswap) loop(ctx context.Context) { // Every so often, we should resend out our current want list rebroadcastTime := time.Second * 5 From 4ef780a9c96547fa10423cef8c7f4c7630cf3852 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:28:29 -0800 Subject: [PATCH 38/90] fix(bitswap) signal termination to async'ly spawned workers License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 1c1982edc..6bfcb4800 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -163,7 +163,10 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } // TODO ensure only one active request per key -func (bs *bitswap) loop(ctx context.Context) { +func (bs *bitswap) loop(parent context.Context) { + + ctx, cancel := context.WithCancel(parent) + defer cancel() // signal termination // Every so often, we should resend out our current want list rebroadcastTime := time.Second * 5 @@ -190,7 +193,7 @@ func (bs *bitswap) loop(ctx context.Context) { log.Errorf("error sending wantlist: %s", err) } } - case <-ctx.Done(): + case <-parent.Done(): return } } From c7c085970ef325099fd7398a18f78b837c728f39 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 18:34:42 -0800 Subject: [PATCH 39/90] fix(exchange) allow exchange to be closed License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 13 ++++++++++++- exchange/interface.go | 5 ++++- exchange/offline/offline.go | 8 ++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 6bfcb4800..cb5db26f3 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -25,9 +25,11 @@ var log = eventlog.Logger("bitswap") // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. // Runs until context is cancelled -func New(ctx context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing bsnet.Routing, +func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing bsnet.Routing, bstore blockstore.Blockstore, nice bool) exchange.Interface { + ctx, cancelFunc := context.WithCancel(parent) + notif := notifications.New() go func() { <-ctx.Done() @@ -36,6 +38,7 @@ func New(ctx context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing bs := &bitswap{ blockstore: bstore, + cancelFunc: cancelFunc, notifications: notif, strategy: strategy.New(nice), routing: routing, @@ -75,6 +78,9 @@ type bitswap struct { strategy strategy.Strategy wantlist u.KeySet + + // cancelFunc signals cancellation to the bitswap event loop + cancelFunc func() } // GetBlock attempts to retrieve a particular block from peers within the @@ -295,3 +301,8 @@ func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block blocks.Block) } } } + +func (bs *bitswap) Close() error { + bs.cancelFunc() + return nil // to conform to Closer interface +} diff --git a/exchange/interface.go b/exchange/interface.go index b62a47957..1f126eed3 100644 --- a/exchange/interface.go +++ b/exchange/interface.go @@ -2,6 +2,8 @@ package exchange import ( + "io" + context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" blocks "github.com/jbenet/go-ipfs/blocks" @@ -11,11 +13,12 @@ import ( // Any type that implements exchange.Interface may be used as an IPFS block // exchange protocol. type Interface interface { - // GetBlock returns the block associated with a given key. GetBlock(context.Context, u.Key) (*blocks.Block, error) // TODO Should callers be concerned with whether the block was made // available on the network? HasBlock(context.Context, blocks.Block) error + + io.Closer } diff --git a/exchange/offline/offline.go b/exchange/offline/offline.go index fbb3485c0..893f546a9 100644 --- a/exchange/offline/offline.go +++ b/exchange/offline/offline.go @@ -20,8 +20,7 @@ func Exchange() exchange.Interface { // offlineExchange implements the Exchange interface but doesn't return blocks. // For use in offline mode. -type offlineExchange struct { -} +type offlineExchange struct{} // GetBlock returns nil to signal that a block could not be retrieved for the // given key. @@ -34,3 +33,8 @@ func (_ *offlineExchange) GetBlock(context.Context, u.Key) (*blocks.Block, error func (_ *offlineExchange) HasBlock(context.Context, blocks.Block) error { return nil } + +// Close always returns nil. +func (_ *offlineExchange) Close() error { + return nil +} From 0ce6071fea1ddd86aac327f47b2961f517cc3eaf Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 05:38:13 +0000 Subject: [PATCH 40/90] revamp BatchFetch a bit --- merkledag/merkledag.go | 71 ++++++++++++++++++++---------------------- unixfs/io/dagreader.go | 34 +++++++++++--------- 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 2483ce4d9..bdb258444 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -22,6 +22,19 @@ var ErrNotFound = fmt.Errorf("merkledag: not found") // so have to convert Multihash bytes to string (u.Key) type NodeMap map[u.Key]*Node +// DAGService is an IPFS Merkle DAG service. +type DAGService interface { + Add(*Node) (u.Key, error) + AddRecursive(*Node) error + Get(u.Key) (*Node, error) + Remove(*Node) error + BatchFetch(context.Context, *Node) <-chan *Node +} + +func NewDAGService(bs *bserv.BlockService) DAGService { + return &dagService{bs} +} + // Node represents a node in the IPFS Merkle DAG. // nodes have opaque data and a set of navigable links. type Node struct { @@ -156,18 +169,6 @@ func (n *Node) Key() (u.Key, error) { return u.Key(h), err } -// DAGService is an IPFS Merkle DAG service. -type DAGService interface { - Add(*Node) (u.Key, error) - AddRecursive(*Node) error - Get(u.Key) (*Node, error) - Remove(*Node) error -} - -func NewDAGService(bs *bserv.BlockService) DAGService { - return &dagService{bs} -} - // dagService is an IPFS Merkle DAG service. // - the root is virtual (like a forest) // - stores nodes' data in a BlockService @@ -286,59 +287,55 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} return done } -// Take advantage of blockservice/bitswap batched requests to fetch all -// child nodes of a given node -// TODO: finish this -func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan int { - sig := make(chan int) +// BatchFetch will fill out all of the links of the given Node. +// It returns a channel of indicies, which will be returned in order +// from 0 to len(root.Links) - 1, signalling that the link specified by +// the index has been filled out. +func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { + sig := make(chan *Node) go func() { var keys []u.Key - for _, lnk := range root.Links { - keys = append(keys, u.Key(lnk.Hash)) - } - - blkchan := ds.Blocks.GetBlocks(ctx, keys) + nodes := make([]*Node, len(root.Links)) // next := 0 seen := make(map[int]struct{}) // + for _, lnk := range root.Links { + keys = append(keys, u.Key(lnk.Hash)) + } + + blkchan := ds.Blocks.GetBlocks(ctx, keys) + for blk := range blkchan { for i, lnk := range root.Links { + if u.Key(lnk.Hash) != blk.Key() { + continue + } // seen[i] = struct{}{} // - if u.Key(lnk.Hash) != blk.Key() { - continue - } nd, err := Decoded(blk.Data) if err != nil { log.Error("Got back bad block!") break } - lnk.Node = nd + nodes[i] = nd - // if next == i { - sig <- next + sig <- nd next++ - for { - if _, ok := seen[next]; ok { - sig <- next - next++ - } else { - break - } + for ; nodes[next] != nil; next++ { + sig <- nodes[next] } } - // } } + close(sig) }() - // TODO: return a channel, and signal when the 'Next' readable block is available return sig } diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index ea33c3540..ec1b21bfe 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -5,6 +5,8 @@ import ( "errors" "io" + "code.google.com/p/go.net/context" + proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" mdag "github.com/jbenet/go-ipfs/merkledag" ft "github.com/jbenet/go-ipfs/unixfs" @@ -15,10 +17,10 @@ var ErrIsDir = errors.New("this dag node is a directory") // DagReader provides a way to easily read the data contained in a dag. type DagReader struct { - serv mdag.DAGService - node *mdag.Node - position int - buf io.Reader + serv mdag.DAGService + node *mdag.Node + buf io.Reader + fetchChan <-chan *mdag.Node } // NewDagReader creates a new reader object that reads the data represented by the given @@ -36,9 +38,10 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { return nil, ErrIsDir case ftpb.Data_File: return &DagReader{ - node: n, - serv: serv, - buf: bytes.NewBuffer(pb.GetData()), + node: n, + serv: serv, + buf: bytes.NewBuffer(pb.GetData()), + fetchChan: serv.BatchFetch(context.TODO(), n), }, nil case ftpb.Data_Raw: // Raw block will just be a single level, return a byte buffer @@ -51,19 +54,20 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { // precalcNextBuf follows the next link in line and loads it from the DAGService, // setting the next buffer to read from func (dr *DagReader) precalcNextBuf() error { - if dr.position >= len(dr.node.Links) { - return io.EOF - } - nxt, err := dr.node.Links[dr.position].GetNode(dr.serv) - if err != nil { - return err + var nxt *mdag.Node + var ok bool + select { + case nxt, ok = <-dr.fetchChan: + if !ok { + return io.EOF + } } + pb := new(ftpb.Data) - err = proto.Unmarshal(nxt.Data, pb) + err := proto.Unmarshal(nxt.Data, pb) if err != nil { return err } - dr.position++ switch pb.GetType() { case ftpb.Data_Directory: From 59a32b1d0fdfac5724b4e1a9f88c1f7b664344e2 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 19:14:16 -0800 Subject: [PATCH 41/90] refactor(bitswap) group the deferreds License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index cb5db26f3..05ed27eb3 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -92,11 +92,14 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err // functions. This is difficult to enforce. May this comment keep you safe. ctx, cancelFunc := context.WithCancel(parent) - defer cancelFunc() ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("GetBlockRequest")) log.Event(ctx, "GetBlockRequestBegin", &k) - defer log.Event(ctx, "GetBlockRequestEnd", &k) + + defer func() { + cancelFunc() + log.Event(ctx, "GetBlockRequestEnd", &k) + }() promise, err := bs.GetBlocks(parent, []u.Key{k}) if err != nil { @@ -109,6 +112,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err case <-parent.Done(): return nil, parent.Err() } + } // GetBlocks returns a channel where the caller may receive blocks that @@ -172,13 +176,15 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e func (bs *bitswap) loop(parent context.Context) { ctx, cancel := context.WithCancel(parent) - defer cancel() // signal termination // Every so often, we should resend out our current want list rebroadcastTime := time.Second * 5 broadcastSignal := time.NewTicker(bs.strategy.GetRebroadcastDelay()) - defer broadcastSignal.Stop() + defer func() { + cancel() // signal to derived async functions + broadcastSignal.Stop() + }() for { select { From ab201c15bb4ac20870f9b473be91947930f979cf Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 20 Nov 2014 19:12:02 -0800 Subject: [PATCH 42/90] test(bitswap) Close (but skip for now) License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 52dad14b5..a8483c3bd 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -18,6 +18,21 @@ import ( mock "github.com/jbenet/go-ipfs/routing/mock" ) +func TestClose(t *testing.T) { + // TODO + t.Skip("TODO Bitswap's Close implementation is a WIP") + vnet := tn.VirtualNetwork() + rout := mock.VirtualRoutingServer() + sesgen := NewSessionGenerator(vnet, rout) + bgen := NewBlockGenerator() + + block := bgen.Next() + bitswap := sesgen.Next() + + bitswap.exchange.Close() + bitswap.exchange.GetBlock(context.Background(), block.Key()) +} + func TestGetBlockTimeout(t *testing.T) { net := tn.VirtualNetwork() From d53deebada5f4ff2012f4fe9403862186200e28e Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 06:40:34 +0000 Subject: [PATCH 43/90] wire GetBlocks into blockservice --- blockservice/blockservice.go | 12 ++++++++- exchange/bitswap/bitswap.go | 12 ++++----- exchange/bitswap/bitswap_test.go | 14 +++++----- exchange/bitswap/message/message.go | 20 +++++++------- exchange/bitswap/message/message_test.go | 14 +++++----- .../bitswap/notifications/notifications.go | 12 ++++----- .../notifications/notifications_test.go | 8 +++--- exchange/bitswap/strategy/strategy_test.go | 2 +- exchange/bitswap/testnet/network_test.go | 8 +++--- exchange/interface.go | 4 ++- exchange/offline/offline.go | 6 ++++- exchange/offline/offline_test.go | 2 +- importer/importer_test.go | 1 + merkledag/merkledag.go | 2 +- unixfs/io/dagreader.go | 26 +++++++++++++++---- 15 files changed, 88 insertions(+), 55 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 86e0c776b..96234c12a 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -68,7 +68,7 @@ func (s *BlockService) AddBlock(b *blocks.Block) (u.Key, error) { // consider moving this to an sync process. if s.Remote != nil { ctx := context.TODO() - err = s.Remote.HasBlock(ctx, *b) + err = s.Remote.HasBlock(ctx, b) } return k, err } @@ -98,6 +98,7 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { out := make(chan *blocks.Block, 32) go func() { + defer close(out) var toFetch []u.Key for _, k := range ks { block, err := s.Blockstore.Get(k) @@ -108,6 +109,15 @@ func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks log.Debug("Blockservice: Got data in datastore.") out <- block } + + nblocks, err := s.Remote.GetBlocks(ctx, toFetch) + if err != nil { + log.Errorf("Error with GetBlocks: %s", err) + return + } + for blk := range nblocks { + out <- blk + } }() return out } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 05ed27eb3..604cfa21a 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -108,7 +108,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err select { case block := <-promise: - return &block, nil + return block, nil case <-parent.Done(): return nil, parent.Err() } @@ -122,7 +122,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err // NB: Your request remains open until the context expires. To conserve // resources, provide a context with a reasonably short deadline (ie. not one // that lasts throughout the lifetime of the server) -func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan blocks.Block, error) { +func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan *blocks.Block, error) { // TODO log the request promise := bs.notifications.Subscribe(ctx, keys...) @@ -213,7 +213,7 @@ func (bs *bitswap) loop(parent context.Context) { // HasBlock announces the existance of a block to this bitswap service. The // service will potentially notify its peers. -func (bs *bitswap) HasBlock(ctx context.Context, blk blocks.Block) error { +func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { log.Debugf("Has Block %v", blk.Key()) bs.wantlist.Remove(blk.Key()) bs.sendToPeersThatWant(ctx, blk) @@ -244,7 +244,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm for _, block := range incoming.Blocks() { // TODO verify blocks? - if err := bs.blockstore.Put(&block); err != nil { + if err := bs.blockstore.Put(block); err != nil { log.Criticalf("error putting block: %s", err) continue // FIXME(brian): err ignored } @@ -267,7 +267,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm if block, errBlockNotFound := bs.blockstore.Get(key); errBlockNotFound != nil { continue } else { - message.AddBlock(*block) + message.AddBlock(block) } } } @@ -290,7 +290,7 @@ func (bs *bitswap) send(ctx context.Context, p peer.Peer, m bsmsg.BitSwapMessage bs.strategy.MessageSent(p, m) } -func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block blocks.Block) { +func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block *blocks.Block) { log.Debugf("Sending %v to peers that want it", block.Key()) for _, p := range bs.strategy.Peers() { diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index a8483c3bd..4f5755ae0 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -83,7 +83,7 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { if err := hasBlock.blockstore.Put(block); err != nil { t.Fatal(err) } - if err := hasBlock.exchange.HasBlock(context.Background(), *block); err != nil { + if err := hasBlock.exchange.HasBlock(context.Background(), block); err != nil { t.Fatal(err) } @@ -140,7 +140,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { first := instances[0] for _, b := range blocks { first.blockstore.Put(b) - first.exchange.HasBlock(context.Background(), *b) + first.exchange.HasBlock(context.Background(), b) rs.Announce(first.peer, b.Key()) } @@ -212,7 +212,7 @@ func TestSendToWantingPeer(t *testing.T) { beta := bg.Next() t.Logf("Peer %v announes availability of %v\n", w.peer, beta.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := w.blockstore.Put(&beta); err != nil { + if err := w.blockstore.Put(beta); err != nil { t.Fatal(err) } w.exchange.HasBlock(ctx, beta) @@ -225,7 +225,7 @@ func TestSendToWantingPeer(t *testing.T) { t.Logf("%v announces availability of %v\n", o.peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := o.blockstore.Put(&alpha); err != nil { + if err := o.blockstore.Put(alpha); err != nil { t.Fatal(err) } o.exchange.HasBlock(ctx, alpha) @@ -254,16 +254,16 @@ type BlockGenerator struct { seq int } -func (bg *BlockGenerator) Next() blocks.Block { +func (bg *BlockGenerator) Next() *blocks.Block { bg.seq++ - return *blocks.NewBlock([]byte(string(bg.seq))) + return blocks.NewBlock([]byte(string(bg.seq))) } func (bg *BlockGenerator) Blocks(n int) []*blocks.Block { blocks := make([]*blocks.Block, 0) for i := 0; i < n; i++ { b := bg.Next() - blocks = append(blocks, &b) + blocks = append(blocks, b) } return blocks } diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index e0aea227d..b69450a6f 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -19,7 +19,7 @@ type BitSwapMessage interface { Wantlist() []u.Key // Blocks returns a slice of unique blocks - Blocks() []blocks.Block + Blocks() []*blocks.Block // AddWanted adds the key to the Wantlist. // @@ -32,7 +32,7 @@ type BitSwapMessage interface { // implies Priority(A) > Priority(B) AddWanted(u.Key) - AddBlock(blocks.Block) + AddBlock(*blocks.Block) Exportable } @@ -42,14 +42,14 @@ type Exportable interface { } type impl struct { - existsInWantlist map[u.Key]struct{} // map to detect duplicates - wantlist []u.Key // slice to preserve ordering - blocks map[u.Key]blocks.Block // map to detect duplicates + existsInWantlist map[u.Key]struct{} // map to detect duplicates + wantlist []u.Key // slice to preserve ordering + blocks map[u.Key]*blocks.Block // map to detect duplicates } func New() BitSwapMessage { return &impl{ - blocks: make(map[u.Key]blocks.Block), + blocks: make(map[u.Key]*blocks.Block), existsInWantlist: make(map[u.Key]struct{}), wantlist: make([]u.Key, 0), } @@ -62,7 +62,7 @@ func newMessageFromProto(pbm pb.Message) BitSwapMessage { } for _, d := range pbm.GetBlocks() { b := blocks.NewBlock(d) - m.AddBlock(*b) + m.AddBlock(b) } return m } @@ -71,8 +71,8 @@ func (m *impl) Wantlist() []u.Key { return m.wantlist } -func (m *impl) Blocks() []blocks.Block { - bs := make([]blocks.Block, 0) +func (m *impl) Blocks() []*blocks.Block { + bs := make([]*blocks.Block, 0) for _, block := range m.blocks { bs = append(bs, block) } @@ -88,7 +88,7 @@ func (m *impl) AddWanted(k u.Key) { m.wantlist = append(m.wantlist, k) } -func (m *impl) AddBlock(b blocks.Block) { +func (m *impl) AddBlock(b *blocks.Block) { m.blocks[b.Key()] = b } diff --git a/exchange/bitswap/message/message_test.go b/exchange/bitswap/message/message_test.go index 9c69136cd..de64b7925 100644 --- a/exchange/bitswap/message/message_test.go +++ b/exchange/bitswap/message/message_test.go @@ -42,7 +42,7 @@ func TestAppendBlock(t *testing.T) { m := New() for _, str := range strs { block := blocks.NewBlock([]byte(str)) - m.AddBlock(*block) + m.AddBlock(block) } // assert strings are in proto message @@ -133,10 +133,10 @@ func TestToNetFromNetPreservesWantList(t *testing.T) { func TestToAndFromNetMessage(t *testing.T) { original := New() - original.AddBlock(*blocks.NewBlock([]byte("W"))) - original.AddBlock(*blocks.NewBlock([]byte("E"))) - original.AddBlock(*blocks.NewBlock([]byte("F"))) - original.AddBlock(*blocks.NewBlock([]byte("M"))) + original.AddBlock(blocks.NewBlock([]byte("W"))) + original.AddBlock(blocks.NewBlock([]byte("E"))) + original.AddBlock(blocks.NewBlock([]byte("F"))) + original.AddBlock(blocks.NewBlock([]byte("M"))) p := peer.WithIDString("X") netmsg, err := original.ToNet(p) @@ -180,8 +180,8 @@ func TestDuplicates(t *testing.T) { t.Fatal("Duplicate in BitSwapMessage") } - msg.AddBlock(*b) - msg.AddBlock(*b) + msg.AddBlock(b) + msg.AddBlock(b) if len(msg.Blocks()) != 1 { t.Fatal("Duplicate in BitSwapMessage") } diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index a2646c814..2497f6316 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -11,8 +11,8 @@ import ( const bufferSize = 16 type PubSub interface { - Publish(block blocks.Block) - Subscribe(ctx context.Context, keys ...u.Key) <-chan blocks.Block + Publish(block *blocks.Block) + Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block Shutdown() } @@ -24,7 +24,7 @@ type impl struct { wrapped pubsub.PubSub } -func (ps *impl) Publish(block blocks.Block) { +func (ps *impl) Publish(block *blocks.Block) { topic := string(block.Key()) ps.wrapped.Pub(block, topic) } @@ -32,18 +32,18 @@ func (ps *impl) Publish(block blocks.Block) { // Subscribe returns a one-time use |blockChannel|. |blockChannel| returns nil // if the |ctx| times out or is cancelled. Then channel is closed after the // blocks given by |keys| are sent. -func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan blocks.Block { +func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { topics := make([]string, 0) for _, key := range keys { topics = append(topics, string(key)) } subChan := ps.wrapped.SubOnce(topics...) - blockChannel := make(chan blocks.Block, 1) // buffered so the sender doesn't wait on receiver + blockChannel := make(chan *blocks.Block, 1) // buffered so the sender doesn't wait on receiver go func() { defer close(blockChannel) select { case val := <-subChan: - block, ok := val.(blocks.Block) + block, ok := val.(*blocks.Block) if ok { blockChannel <- block } diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index 063634f61..ebbae2a51 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -16,13 +16,13 @@ func TestPublishSubscribe(t *testing.T) { defer n.Shutdown() ch := n.Subscribe(context.Background(), blockSent.Key()) - n.Publish(*blockSent) + n.Publish(blockSent) blockRecvd, ok := <-ch if !ok { t.Fail() } - assertBlocksEqual(t, blockRecvd, *blockSent) + assertBlocksEqual(t, blockRecvd, blockSent) } @@ -39,14 +39,14 @@ func TestCarryOnWhenDeadlineExpires(t *testing.T) { assertBlockChannelNil(t, blockChannel) } -func assertBlockChannelNil(t *testing.T, blockChannel <-chan blocks.Block) { +func assertBlockChannelNil(t *testing.T, blockChannel <-chan *blocks.Block) { _, ok := <-blockChannel if ok { t.Fail() } } -func assertBlocksEqual(t *testing.T, a, b blocks.Block) { +func assertBlocksEqual(t *testing.T, a, b *blocks.Block) { if !bytes.Equal(a.Data, b.Data) { t.Fail() } diff --git a/exchange/bitswap/strategy/strategy_test.go b/exchange/bitswap/strategy/strategy_test.go index ef93d9827..d07af601b 100644 --- a/exchange/bitswap/strategy/strategy_test.go +++ b/exchange/bitswap/strategy/strategy_test.go @@ -30,7 +30,7 @@ func TestConsistentAccounting(t *testing.T) { m := message.New() content := []string{"this", "is", "message", "i"} - m.AddBlock(*blocks.NewBlock([]byte(strings.Join(content, " ")))) + m.AddBlock(blocks.NewBlock([]byte(strings.Join(content, " ")))) sender.MessageSent(receiver.Peer, m) receiver.MessageReceived(sender.Peer, m) diff --git a/exchange/bitswap/testnet/network_test.go b/exchange/bitswap/testnet/network_test.go index 3930c2a8c..6f57aad50 100644 --- a/exchange/bitswap/testnet/network_test.go +++ b/exchange/bitswap/testnet/network_test.go @@ -33,7 +33,7 @@ func TestSendRequestToCooperativePeer(t *testing.T) { // TODO test contents of incoming message m := bsmsg.New() - m.AddBlock(*blocks.NewBlock([]byte(expectedStr))) + m.AddBlock(blocks.NewBlock([]byte(expectedStr))) return from, m })) @@ -41,7 +41,7 @@ func TestSendRequestToCooperativePeer(t *testing.T) { t.Log("Build a message and send a synchronous request to recipient") message := bsmsg.New() - message.AddBlock(*blocks.NewBlock([]byte("data"))) + message.AddBlock(blocks.NewBlock([]byte("data"))) response, err := initiator.SendRequest( context.Background(), peer.WithID(idOfRecipient), message) if err != nil { @@ -77,7 +77,7 @@ func TestSendMessageAsyncButWaitForResponse(t *testing.T) { peer.Peer, bsmsg.BitSwapMessage) { msgToWaiter := bsmsg.New() - msgToWaiter.AddBlock(*blocks.NewBlock([]byte(expectedStr))) + msgToWaiter.AddBlock(blocks.NewBlock([]byte(expectedStr))) return fromWaiter, msgToWaiter })) @@ -105,7 +105,7 @@ func TestSendMessageAsyncButWaitForResponse(t *testing.T) { })) messageSentAsync := bsmsg.New() - messageSentAsync.AddBlock(*blocks.NewBlock([]byte("data"))) + messageSentAsync.AddBlock(blocks.NewBlock([]byte("data"))) errSending := waiter.SendMessage( context.Background(), peer.WithID(idOfResponder), messageSentAsync) if errSending != nil { diff --git a/exchange/interface.go b/exchange/interface.go index 1f126eed3..aa2e2431c 100644 --- a/exchange/interface.go +++ b/exchange/interface.go @@ -16,9 +16,11 @@ type Interface interface { // GetBlock returns the block associated with a given key. GetBlock(context.Context, u.Key) (*blocks.Block, error) + GetBlocks(context.Context, []u.Key) (<-chan *blocks.Block, error) + // TODO Should callers be concerned with whether the block was made // available on the network? - HasBlock(context.Context, blocks.Block) error + HasBlock(context.Context, *blocks.Block) error io.Closer } diff --git a/exchange/offline/offline.go b/exchange/offline/offline.go index 893f546a9..24a89e038 100644 --- a/exchange/offline/offline.go +++ b/exchange/offline/offline.go @@ -30,7 +30,7 @@ func (_ *offlineExchange) GetBlock(context.Context, u.Key) (*blocks.Block, error } // HasBlock always returns nil. -func (_ *offlineExchange) HasBlock(context.Context, blocks.Block) error { +func (_ *offlineExchange) HasBlock(context.Context, *blocks.Block) error { return nil } @@ -38,3 +38,7 @@ func (_ *offlineExchange) HasBlock(context.Context, blocks.Block) error { func (_ *offlineExchange) Close() error { return nil } + +func (_ *offlineExchange) GetBlocks(context.Context, []u.Key) (<-chan *blocks.Block, error) { + return nil, OfflineMode +} diff --git a/exchange/offline/offline_test.go b/exchange/offline/offline_test.go index 98b6e1a8c..ac02d2101 100644 --- a/exchange/offline/offline_test.go +++ b/exchange/offline/offline_test.go @@ -21,7 +21,7 @@ func TestBlockReturnsErr(t *testing.T) { func TestHasBlockReturnsNil(t *testing.T) { off := Exchange() block := blocks.NewBlock([]byte("data")) - err := off.HasBlock(context.Background(), *block) + err := off.HasBlock(context.Background(), block) if err != nil { t.Fatal("") } diff --git a/importer/importer_test.go b/importer/importer_test.go index e8177aa91..21af40670 100644 --- a/importer/importer_test.go +++ b/importer/importer_test.go @@ -69,6 +69,7 @@ func testFileConsistency(t *testing.T, bs chunk.BlockSplitter, nbytes int) { if err != nil { t.Fatal(err) } + r, err := uio.NewDagReader(nd, nil) if err != nil { t.Fatal(err) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index bdb258444..2fba2f5fa 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -328,7 +328,7 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { if next == i { sig <- nd next++ - for ; nodes[next] != nil; next++ { + for ; next < len(nodes) && nodes[next] != nil; next++ { sig <- nodes[next] } } diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index ec1b21bfe..7f8720cf1 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -17,10 +17,11 @@ var ErrIsDir = errors.New("this dag node is a directory") // DagReader provides a way to easily read the data contained in a dag. type DagReader struct { - serv mdag.DAGService - node *mdag.Node - buf io.Reader - fetchChan <-chan *mdag.Node + serv mdag.DAGService + node *mdag.Node + buf io.Reader + fetchChan <-chan *mdag.Node + linkPosition int } // NewDagReader creates a new reader object that reads the data represented by the given @@ -37,11 +38,15 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { // Dont allow reading directories return nil, ErrIsDir case ftpb.Data_File: + var fetchChan <-chan *mdag.Node + if serv != nil { + fetchChan = serv.BatchFetch(context.TODO(), n) + } return &DagReader{ node: n, serv: serv, buf: bytes.NewBuffer(pb.GetData()), - fetchChan: serv.BatchFetch(context.TODO(), n), + fetchChan: fetchChan, }, nil case ftpb.Data_Raw: // Raw block will just be a single level, return a byte buffer @@ -61,6 +66,17 @@ func (dr *DagReader) precalcNextBuf() error { if !ok { return io.EOF } + default: + // Only used when fetchChan is nil, + // which only happens when passed in a nil dagservice + // TODO: this logic is hard to follow, do it better. + // NOTE: the only time this code is used, is during the + // importer tests, consider just changing those tests + if dr.linkPosition >= len(dr.node.Links) { + return io.EOF + } + nxt = dr.node.Links[dr.linkPosition].Node + dr.linkPosition++ } pb := new(ftpb.Data) From 15d4f82945bb8902cea34b4fe987b93de1f4c9ea Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 08:01:34 +0000 Subject: [PATCH 44/90] some cleanup, and fix minor bug in dagreader from previous commit --- blockservice/blockservice.go | 3 +++ merkledag/merkledag.go | 10 ++-------- unixfs/io/dagreader.go | 19 +++++++++++++------ 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 96234c12a..97c7dec90 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -95,6 +95,9 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er } } +// GetBlocks gets a list of blocks asynchronously and returns through +// the returned channel. +// NB: No guarantees are made about order. func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { out := make(chan *blocks.Block, 32) go func() { diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 2fba2f5fa..453f515e5 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -288,9 +288,8 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} } // BatchFetch will fill out all of the links of the given Node. -// It returns a channel of indicies, which will be returned in order -// from 0 to len(root.Links) - 1, signalling that the link specified by -// the index has been filled out. +// It returns a channel of nodes, which the caller can receive +// all the child nodes of 'root' on, in proper order. func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { sig := make(chan *Node) go func() { @@ -299,7 +298,6 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { // next := 0 - seen := make(map[int]struct{}) // for _, lnk := range root.Links { @@ -314,10 +312,6 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { continue } - // - seen[i] = struct{}{} - // - nd, err := Decoded(blk.Data) if err != nil { log.Error("Got back bad block!") diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index 7f8720cf1..892752c91 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -61,12 +61,8 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { func (dr *DagReader) precalcNextBuf() error { var nxt *mdag.Node var ok bool - select { - case nxt, ok = <-dr.fetchChan: - if !ok { - return io.EOF - } - default: + + if dr.serv == nil { // Only used when fetchChan is nil, // which only happens when passed in a nil dagservice // TODO: this logic is hard to follow, do it better. @@ -76,7 +72,18 @@ func (dr *DagReader) precalcNextBuf() error { return io.EOF } nxt = dr.node.Links[dr.linkPosition].Node + if nxt == nil { + return errors.New("Got nil node back from link! and no DAGService!") + } dr.linkPosition++ + + } else { + select { + case nxt, ok = <-dr.fetchChan: + if !ok { + return io.EOF + } + } } pb := new(ftpb.Data) From ed4509923cd80bd0b5cc98a32e61d77270fc1d62 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 18:14:28 +0000 Subject: [PATCH 45/90] tracking down a bug dhthell found, added asserts and better logging. --- blockservice/blockservice.go | 2 +- exchange/bitswap/bitswap.go | 16 +++++++++++----- net/swarm/conn.go | 2 +- unixfs/io/dagreader.go | 4 ++++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 97c7dec90..c4c90c88b 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -101,7 +101,6 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { out := make(chan *blocks.Block, 32) go func() { - defer close(out) var toFetch []u.Key for _, k := range ks { block, err := s.Blockstore.Get(k) @@ -121,6 +120,7 @@ func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks for blk := range nblocks { out <- blk } + close(out) }() return out } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 604cfa21a..6a6565d19 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -197,13 +197,19 @@ func (bs *bitswap) loop(parent context.Context) { } } case ks := <-bs.batchRequests: + // TODO: implement batching on len(ks) > X for some X + if len(ks) == 0 { + log.Warning("Received batch request for zero blocks") + continue + } for _, k := range ks { bs.wantlist.Add(k) - providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) - } + } + providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) + + err := bs.sendWantListTo(ctx, providers) + if err != nil { + log.Errorf("error sending wantlist: %s", err) } case <-parent.Done(): return diff --git a/net/swarm/conn.go b/net/swarm/conn.go index f95334d0d..ab6035917 100644 --- a/net/swarm/conn.go +++ b/net/swarm/conn.go @@ -182,7 +182,7 @@ func (s *Swarm) fanOut() { return } if len(msg.Data()) >= conn.MaxMessageSize { - log.Critical("Attempted to send message bigger than max size.") + log.Criticalf("Attempted to send message bigger than max size. (%d)", len(msg.Data())) } s.connsLock.RLock() diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index 892752c91..7373b94ae 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -68,6 +68,7 @@ func (dr *DagReader) precalcNextBuf() error { // TODO: this logic is hard to follow, do it better. // NOTE: the only time this code is used, is during the // importer tests, consider just changing those tests + log.Warning("Running DAGReader with nil DAGService!") if dr.linkPosition >= len(dr.node.Links) { return io.EOF } @@ -78,6 +79,9 @@ func (dr *DagReader) precalcNextBuf() error { dr.linkPosition++ } else { + if dr.fetchChan == nil { + panic("this is wrong.") + } select { case nxt, ok = <-dr.fetchChan: if !ok { From 438ffa1dd7a211c9b327f6759bc364406ba36a27 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 13:09:44 -0800 Subject: [PATCH 46/90] feat(util) ForwardNBlocks License: MIT Signed-off-by: Brian Tiger Chow --- util/async/forward.go | 30 +++++++++++++++++++++ util/async/forward_test.go | 55 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 util/async/forward.go create mode 100644 util/async/forward_test.go diff --git a/util/async/forward.go b/util/async/forward.go new file mode 100644 index 000000000..871b5a608 --- /dev/null +++ b/util/async/forward.go @@ -0,0 +1,30 @@ +package async + +import ( + context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + "github.com/jbenet/go-ipfs/blocks" +) + +// ForwardN forwards up to |num| blocks to the returned channel. +func ForwardN(ctx context.Context, in <-chan *blocks.Block, num int) <-chan *blocks.Block { + out := make(chan *blocks.Block) + go func() { + defer close(out) + for i := 0; i < num; i++ { + select { + case block, ok := <-in: + if !ok { + return + } + select { + case out <- block: + case <-ctx.Done(): + return + } + case <-ctx.Done(): + return + } + } + }() + return out +} diff --git a/util/async/forward_test.go b/util/async/forward_test.go new file mode 100644 index 000000000..42f5864b6 --- /dev/null +++ b/util/async/forward_test.go @@ -0,0 +1,55 @@ +package async + +import ( + "testing" + + "code.google.com/p/go.net/context" + "github.com/jbenet/go-ipfs/blocks" +) + +func TestForwardTwo(t *testing.T) { + const n = 2 + in := make(chan *blocks.Block, n) + ctx := context.Background() + out := ForwardN(ctx, in, n) + + in <- blocks.NewBlock([]byte("one")) + in <- blocks.NewBlock([]byte("two")) + + _ = <-out // 1 + _ = <-out // 2 + + _, ok := <-out // closed + if !ok { + return + } + t.Fail() +} + +func TestCloseInput(t *testing.T) { + const n = 2 + in := make(chan *blocks.Block, 0) + ctx := context.Background() + out := ForwardN(ctx, in, n) + + close(in) + _, ok := <-out // closed + if !ok { + return + } + t.Fatal("input channel closed, but output channel not") + +} + +func TestContextClosedWhenBlockingOnInput(t *testing.T) { + const n = 1 // but we won't ever send a block + ctx, cancel := context.WithCancel(context.Background()) + out := ForwardN(ctx, make(chan *blocks.Block), n) + + cancel() // before sending anything + _, ok := <-out + if !ok { + return + } + t.Fail() +} From be2678522e0f1779300cb9a6378b292bfe75aa3a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 13:11:02 -0800 Subject: [PATCH 47/90] fix(dep) License: MIT Signed-off-by: Brian Tiger Chow --- util/async/forward_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/async/forward_test.go b/util/async/forward_test.go index 42f5864b6..d7a13d339 100644 --- a/util/async/forward_test.go +++ b/util/async/forward_test.go @@ -3,7 +3,7 @@ package async import ( "testing" - "code.google.com/p/go.net/context" + context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" "github.com/jbenet/go-ipfs/blocks" ) From fc820a8110324653ac846de6af47ada8aff5e332 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 13:15:39 -0800 Subject: [PATCH 48/90] tests(forward) License: MIT Signed-off-by: Brian Tiger Chow --- util/async/forward_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/util/async/forward_test.go b/util/async/forward_test.go index d7a13d339..3933e4c58 100644 --- a/util/async/forward_test.go +++ b/util/async/forward_test.go @@ -7,23 +7,26 @@ import ( "github.com/jbenet/go-ipfs/blocks" ) -func TestForwardTwo(t *testing.T) { +func TestForwardNThenClose(t *testing.T) { const n = 2 - in := make(chan *blocks.Block, n) + const buf = 2 * n + in := make(chan *blocks.Block, buf) ctx := context.Background() out := ForwardN(ctx, in, n) - in <- blocks.NewBlock([]byte("one")) - in <- blocks.NewBlock([]byte("two")) + for i := 0; i < buf; i++ { + in <- blocks.NewBlock([]byte("")) + } - _ = <-out // 1 - _ = <-out // 2 + for i := 0; i < n; i++ { + _ = <-out + } _, ok := <-out // closed if !ok { return } - t.Fail() + t.Fatal("channel still open after receiving n blocks") } func TestCloseInput(t *testing.T) { From a932bfdfabfb5955bc3ca5c91eb74220364cf73d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 13:17:36 -0800 Subject: [PATCH 49/90] doc License: MIT Signed-off-by: Brian Tiger Chow --- util/async/forward.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/async/forward.go b/util/async/forward.go index 871b5a608..c1cb54a4f 100644 --- a/util/async/forward.go +++ b/util/async/forward.go @@ -14,7 +14,7 @@ func ForwardN(ctx context.Context, in <-chan *blocks.Block, num int) <-chan *blo select { case block, ok := <-in: if !ok { - return + return // otherwise nil value is forwarded to output } select { case out <- block: From 9120d107c33e1e2f365997ea3bc328506be5ae30 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 23:03:05 +0000 Subject: [PATCH 50/90] a little more correctness on the new bitswap impl --- blockservice/blockservice.go | 4 +- exchange/bitswap/bitswap.go | 47 ++++++++++++++++++----- exchange/bitswap/bitswap_test.go | 2 +- exchange/bitswap/strategy/ledger.go | 1 + exchange/bitswap/strategy/strategy.go | 2 + merkledag/merkledag.go | 55 +++++++++++++++++++-------- 6 files changed, 83 insertions(+), 28 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index c4c90c88b..214bd49fc 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -98,7 +98,7 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er // GetBlocks gets a list of blocks asynchronously and returns through // the returned channel. // NB: No guarantees are made about order. -func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { +func (s *BlockService) GetBlocks(parent context.Context, ks []u.Key) <-chan *blocks.Block { out := make(chan *blocks.Block, 32) go func() { var toFetch []u.Key @@ -112,11 +112,13 @@ func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks out <- block } + ctx, cancel := context.WithCancel(parent) nblocks, err := s.Remote.GetBlocks(ctx, toFetch) if err != nil { log.Errorf("Error with GetBlocks: %s", err) return } + for blk := range nblocks { out <- blk } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 6a6565d19..001f844b7 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -128,12 +128,35 @@ func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan *blocks. promise := bs.notifications.Subscribe(ctx, keys...) select { case bs.batchRequests <- keys: - return promise, nil + return pipeBlocks(ctx, promise, len(keys)), nil case <-ctx.Done(): return nil, ctx.Err() } } +func pipeBlocks(ctx context.Context, in <-chan *blocks.Block, count int) <-chan *blocks.Block { + out := make(chan *blocks.Block, 1) + go func() { + defer close(out) + for i := 0; i < count; i++ { + select { + case blk, ok := <-in: + if !ok { + return + } + select { + case out <- blk: + case <-ctx.Done(): + return + } + case <-ctx.Done(): + return + } + } + }() + return out +} + func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { if peers == nil { panic("Cant send wantlist to nil peerchan") @@ -220,7 +243,7 @@ func (bs *bitswap) loop(parent context.Context) { // HasBlock announces the existance of a block to this bitswap service. The // service will potentially notify its peers. func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { - log.Debugf("Has Block %v", blk.Key()) + log.Debugf("Has Block %s", blk.Key()) bs.wantlist.Remove(blk.Key()) bs.sendToPeersThatWant(ctx, blk) return bs.routing.Provide(ctx, blk.Key()) @@ -262,10 +285,6 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } - message := bsmsg.New() - for _, wanted := range bs.wantlist.Keys() { - message.AddWanted(wanted) - } for _, key := range incoming.Wantlist() { // TODO: might be better to check if we have the block before checking // if we should send it to someone @@ -273,14 +292,22 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm if block, errBlockNotFound := bs.blockstore.Get(key); errBlockNotFound != nil { continue } else { - message.AddBlock(block) + // Create a separate message to send this block in + blkmsg := bsmsg.New() + + // TODO: only send this the first time + for _, k := range bs.wantlist.Keys() { + blkmsg.AddWanted(k) + } + + blkmsg.AddBlock(block) + bs.strategy.MessageSent(p, blkmsg) + bs.send(ctx, p, blkmsg) } } } - bs.strategy.MessageSent(p, message) - log.Debug("Returning message.") - return p, message + return nil, nil } func (bs *bitswap) ReceiveError(err error) { diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 4f5755ae0..426c0a315 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -106,7 +106,7 @@ func TestLargeSwarm(t *testing.T) { t.SkipNow() } t.Parallel() - numInstances := 500 + numInstances := 5 numBlocks := 2 PerformDistributionTest(t, numInstances, numBlocks) } diff --git a/exchange/bitswap/strategy/ledger.go b/exchange/bitswap/strategy/ledger.go index 9f33b1aba..74feb3407 100644 --- a/exchange/bitswap/strategy/ledger.go +++ b/exchange/bitswap/strategy/ledger.go @@ -61,6 +61,7 @@ func (l *ledger) ReceivedBytes(n int) { // TODO: this needs to be different. We need timeouts. func (l *ledger) Wants(k u.Key) { + log.Debugf("peer %s wants %s", l.Partner, k) l.wantList[k] = struct{}{} } diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index ad69b841a..d86092da6 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -10,6 +10,8 @@ import ( u "github.com/jbenet/go-ipfs/util" ) +var log = u.Logger("strategy") + // TODO niceness should be on a per-peer basis. Use-case: Certain peers are // "trusted" and/or controlled by a single human user. The user may want for // these peers to exchange data freely diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 453f515e5..2673c59fc 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -163,6 +163,17 @@ func (n *Node) Multihash() (mh.Multihash, error) { return n.cached, nil } +// Searches this nodes links for one to the given key, +// returns the index of said link +func (n *Node) FindLink(k u.Key) (int, error) { + for i, lnk := range n.Links { + if u.Key(lnk.Hash) == k { + return i, nil + } + } + return -1, u.ErrNotFound +} + // Key returns the Multihash as a key, for maps. func (n *Node) Key() (u.Key, error) { h, err := n.Multihash() @@ -296,6 +307,10 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { var keys []u.Key nodes := make([]*Node, len(root.Links)) + //temp + recvd := []int{} + // + // next := 0 // @@ -306,28 +321,36 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { blkchan := ds.Blocks.GetBlocks(ctx, keys) + count := 0 for blk := range blkchan { - for i, lnk := range root.Links { - if u.Key(lnk.Hash) != blk.Key() { - continue - } + count++ + i, err := root.FindLink(blk.Key()) + if err != nil { + panic("Received block that wasnt in this nodes links!") + } - nd, err := Decoded(blk.Data) - if err != nil { - log.Error("Got back bad block!") - break - } - nodes[i] = nd + recvd = append(recvd, i) - if next == i { - sig <- nd - next++ - for ; next < len(nodes) && nodes[next] != nil; next++ { - sig <- nodes[next] - } + nd, err := Decoded(blk.Data) + if err != nil { + log.Error("Got back bad block!") + break + } + nodes[i] = nd + + if next == i { + sig <- nd + next++ + for ; next < len(nodes) && nodes[next] != nil; next++ { + sig <- nodes[next] } } } + if next < len(nodes) { + log.Errorf("count = %d, links = %d", count, len(nodes)) + log.Error(recvd) + panic("didnt receive all requested blocks!") + } close(sig) }() From d5e7fd67075d84e7f3d4cb488d7864d21302a08e Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 15:25:37 -0800 Subject: [PATCH 51/90] test(notifications) we expect this to fail. will be fixed in upcoming commit License: MIT Signed-off-by: Brian Tiger Chow --- .../notifications/notifications_test.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index ebbae2a51..5c51f322e 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -26,6 +26,29 @@ func TestPublishSubscribe(t *testing.T) { } +func TestSubscribeMany(t *testing.T) { + e1 := blocks.NewBlock([]byte("Greetings from The Interval")) + e2 := blocks.NewBlock([]byte("Greetings from The Interval")) + + n := New() + defer n.Shutdown() + ch := n.Subscribe(context.Background(), e1.Key(), e2.Key()) + + n.Publish(e1) + r1, ok := <-ch + if !ok { + t.Fatal("didn't receive first expected block") + } + assertBlocksEqual(t, e1, r1) + + n.Publish(e2) + r2, ok := <-ch + if !ok { + t.Fatal("didn't receive second expected block") + } + assertBlocksEqual(t, e2, r2) +} + func TestCarryOnWhenDeadlineExpires(t *testing.T) { impossibleDeadline := time.Nanosecond From 50b00eb90fc3f1d493f4c1c05592c09ae7958b8a Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 21 Nov 2014 23:33:33 +0000 Subject: [PATCH 52/90] use @maybebtc's ForwardBlocks function --- blockservice/blockservice.go | 3 +-- exchange/bitswap/bitswap.go | 26 ++------------------------ util/async/forward.go | 4 ++++ 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 214bd49fc..07e6d1092 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -98,7 +98,7 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er // GetBlocks gets a list of blocks asynchronously and returns through // the returned channel. // NB: No guarantees are made about order. -func (s *BlockService) GetBlocks(parent context.Context, ks []u.Key) <-chan *blocks.Block { +func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { out := make(chan *blocks.Block, 32) go func() { var toFetch []u.Key @@ -112,7 +112,6 @@ func (s *BlockService) GetBlocks(parent context.Context, ks []u.Key) <-chan *blo out <- block } - ctx, cancel := context.WithCancel(parent) nblocks, err := s.Remote.GetBlocks(ctx, toFetch) if err != nil { log.Errorf("Error with GetBlocks: %s", err) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 001f844b7..5ad3c8026 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -16,6 +16,7 @@ import ( strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" + async "github.com/jbenet/go-ipfs/util/async" "github.com/jbenet/go-ipfs/util/eventlog" ) @@ -128,35 +129,12 @@ func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan *blocks. promise := bs.notifications.Subscribe(ctx, keys...) select { case bs.batchRequests <- keys: - return pipeBlocks(ctx, promise, len(keys)), nil + return async.ForwardN(ctx, promise, len(keys)), nil case <-ctx.Done(): return nil, ctx.Err() } } -func pipeBlocks(ctx context.Context, in <-chan *blocks.Block, count int) <-chan *blocks.Block { - out := make(chan *blocks.Block, 1) - go func() { - defer close(out) - for i := 0; i < count; i++ { - select { - case blk, ok := <-in: - if !ok { - return - } - select { - case out <- blk: - case <-ctx.Done(): - return - } - case <-ctx.Done(): - return - } - } - }() - return out -} - func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { if peers == nil { panic("Cant send wantlist to nil peerchan") diff --git a/util/async/forward.go b/util/async/forward.go index c1cb54a4f..1e6e044a3 100644 --- a/util/async/forward.go +++ b/util/async/forward.go @@ -3,8 +3,11 @@ package async import ( context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" "github.com/jbenet/go-ipfs/blocks" + u "github.com/jbenet/go-ipfs/util" ) +var log = u.Logger("async") + // ForwardN forwards up to |num| blocks to the returned channel. func ForwardN(ctx context.Context, in <-chan *blocks.Block, num int) <-chan *blocks.Block { out := make(chan *blocks.Block) @@ -14,6 +17,7 @@ func ForwardN(ctx context.Context, in <-chan *blocks.Block, num int) <-chan *blo select { case block, ok := <-in: if !ok { + log.Error("Forwarder exiting early!") return // otherwise nil value is forwarded to output } select { From 1fb80330da0902456be129400c01eaa67bf141b1 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 15:41:04 -0800 Subject: [PATCH 53/90] docs(bitswap/notifications) License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/notifications/notifications.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index 2497f6316..1de7bf909 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -29,9 +29,9 @@ func (ps *impl) Publish(block *blocks.Block) { ps.wrapped.Pub(block, topic) } -// Subscribe returns a one-time use |blockChannel|. |blockChannel| returns nil -// if the |ctx| times out or is cancelled. Then channel is closed after the -// blocks given by |keys| are sent. +// Subscribe returns a channel of blocks for the given |keys|. |blockChannel| +// is closed if the |ctx| times out or is cancelled, or after sending len(keys) +// blocks. func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { topics := make([]string, 0) for _, key := range keys { From 03324f776517a681383eea2eae6dcf5b9dcd9a9b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:18:40 -0800 Subject: [PATCH 54/90] fix(bitswap/notifications) subscribe to many License: MIT Signed-off-by: Brian Tiger Chow --- .../bitswap/notifications/notifications.go | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index 1de7bf909..74833810a 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -29,10 +29,7 @@ func (ps *impl) Publish(block *blocks.Block) { ps.wrapped.Pub(block, topic) } -// Subscribe returns a channel of blocks for the given |keys|. |blockChannel| -// is closed if the |ctx| times out or is cancelled, or after sending len(keys) -// blocks. -func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { +func (ps *impl) SubscribeDeprec(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { topics := make([]string, 0) for _, key := range keys { topics = append(topics, string(key)) @@ -57,3 +54,49 @@ func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Blo func (ps *impl) Shutdown() { ps.wrapped.Shutdown() } + +// Subscribe returns a channel of blocks for the given |keys|. |blockChannel| +// is closed if the |ctx| times out or is cancelled, or after sending len(keys) +// blocks. +func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { + topics := toStrings(keys) + blocksCh := make(chan *blocks.Block, len(keys)) + valuesCh := make(chan interface{}, len(keys)) + ps.wrapped.AddSub(valuesCh, topics...) + + go func() { + defer func() { + ps.wrapped.Unsub(valuesCh, topics...) + close(blocksCh) + }() + for _, _ = range keys { + select { + case <-ctx.Done(): + return + case val, ok := <-valuesCh: + if !ok { + return + } + block, ok := val.(*blocks.Block) + if !ok { + return + } + select { + case <-ctx.Done(): + return + case blocksCh <- block: // continue + } + } + } + }() + + return blocksCh +} + +func toStrings(keys []u.Key) []string { + strs := make([]string, 0) + for _, key := range keys { + strs = append(strs, string(key)) + } + return strs +} From a5fccaccee4f5dbbc505be79624d24028697db5f Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:21:03 -0800 Subject: [PATCH 55/90] tests(bitswap/notifications) test niladic License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/notifications/notifications_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index 5c51f322e..7352320d9 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -49,6 +49,15 @@ func TestSubscribeMany(t *testing.T) { assertBlocksEqual(t, e2, r2) } +func TestSubscribeIsANoopWhenCalledWithNoKeys(t *testing.T) { + n := New() + defer n.Shutdown() + ch := n.Subscribe(context.TODO()) // no keys provided + if _, ok := <-ch; ok { + t.Fatal("should be closed if no keys provided") + } +} + func TestCarryOnWhenDeadlineExpires(t *testing.T) { impossibleDeadline := time.Nanosecond From b3d3b1dd301dfa1a745b045c11a0a13d987b5546 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:22:44 -0800 Subject: [PATCH 56/90] refactor(bitswap) forwardN no longer needed @whyrusleeping now, the pubsub channel closes after sending N blocks. we got this functionality for free from the fix. So, the forwardN wrap is no longer required! woohoo License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 5ad3c8026..95cb7ebf6 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -16,7 +16,6 @@ import ( strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" - async "github.com/jbenet/go-ipfs/util/async" "github.com/jbenet/go-ipfs/util/eventlog" ) @@ -129,7 +128,7 @@ func (bs *bitswap) GetBlocks(ctx context.Context, keys []u.Key) (<-chan *blocks. promise := bs.notifications.Subscribe(ctx, keys...) select { case bs.batchRequests <- keys: - return async.ForwardN(ctx, promise, len(keys)), nil + return promise, nil case <-ctx.Done(): return nil, ctx.Err() } From 134929ac6440dcf6fa6840291e8d9112c079c661 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:29:07 -0800 Subject: [PATCH 57/90] misc(bs/n) rm dead code License: MIT Signed-off-by: Brian Tiger Chow --- .../bitswap/notifications/notifications.go | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index 74833810a..ee82f0305 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -29,28 +29,6 @@ func (ps *impl) Publish(block *blocks.Block) { ps.wrapped.Pub(block, topic) } -func (ps *impl) SubscribeDeprec(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { - topics := make([]string, 0) - for _, key := range keys { - topics = append(topics, string(key)) - } - subChan := ps.wrapped.SubOnce(topics...) - blockChannel := make(chan *blocks.Block, 1) // buffered so the sender doesn't wait on receiver - go func() { - defer close(blockChannel) - select { - case val := <-subChan: - block, ok := val.(*blocks.Block) - if ok { - blockChannel <- block - } - case <-ctx.Done(): - ps.wrapped.Unsub(subChan, topics...) - } - }() - return blockChannel -} - func (ps *impl) Shutdown() { ps.wrapped.Shutdown() } From 6a5bc4b879676aeb71951f6f03365048e6d96fda Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:43:57 -0800 Subject: [PATCH 58/90] fix(bs/n) remove unnecessary variable to remove ambiguity (before it was possible to loop over either topics or keys by only keeping keys, there's no confusing about what to use for the loop range License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/notifications/notifications.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index ee82f0305..b07f3bf73 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -37,14 +37,14 @@ func (ps *impl) Shutdown() { // is closed if the |ctx| times out or is cancelled, or after sending len(keys) // blocks. func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { - topics := toStrings(keys) + blocksCh := make(chan *blocks.Block, len(keys)) valuesCh := make(chan interface{}, len(keys)) - ps.wrapped.AddSub(valuesCh, topics...) + ps.wrapped.AddSub(valuesCh, toStrings(keys)...) go func() { defer func() { - ps.wrapped.Unsub(valuesCh, topics...) + ps.wrapped.Unsub(valuesCh, toStrings(keys)...) close(blocksCh) }() for _, _ = range keys { From e4c97316430e208c81d24d1359cfc4c67671c859 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 17:53:53 -0800 Subject: [PATCH 59/90] chore(util) remove forward License: MIT Signed-off-by: Brian Tiger Chow --- util/async/forward.go | 34 ---------------------- util/async/forward_test.go | 58 -------------------------------------- 2 files changed, 92 deletions(-) delete mode 100644 util/async/forward.go delete mode 100644 util/async/forward_test.go diff --git a/util/async/forward.go b/util/async/forward.go deleted file mode 100644 index 1e6e044a3..000000000 --- a/util/async/forward.go +++ /dev/null @@ -1,34 +0,0 @@ -package async - -import ( - context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - "github.com/jbenet/go-ipfs/blocks" - u "github.com/jbenet/go-ipfs/util" -) - -var log = u.Logger("async") - -// ForwardN forwards up to |num| blocks to the returned channel. -func ForwardN(ctx context.Context, in <-chan *blocks.Block, num int) <-chan *blocks.Block { - out := make(chan *blocks.Block) - go func() { - defer close(out) - for i := 0; i < num; i++ { - select { - case block, ok := <-in: - if !ok { - log.Error("Forwarder exiting early!") - return // otherwise nil value is forwarded to output - } - select { - case out <- block: - case <-ctx.Done(): - return - } - case <-ctx.Done(): - return - } - } - }() - return out -} diff --git a/util/async/forward_test.go b/util/async/forward_test.go deleted file mode 100644 index 3933e4c58..000000000 --- a/util/async/forward_test.go +++ /dev/null @@ -1,58 +0,0 @@ -package async - -import ( - "testing" - - context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - "github.com/jbenet/go-ipfs/blocks" -) - -func TestForwardNThenClose(t *testing.T) { - const n = 2 - const buf = 2 * n - in := make(chan *blocks.Block, buf) - ctx := context.Background() - out := ForwardN(ctx, in, n) - - for i := 0; i < buf; i++ { - in <- blocks.NewBlock([]byte("")) - } - - for i := 0; i < n; i++ { - _ = <-out - } - - _, ok := <-out // closed - if !ok { - return - } - t.Fatal("channel still open after receiving n blocks") -} - -func TestCloseInput(t *testing.T) { - const n = 2 - in := make(chan *blocks.Block, 0) - ctx := context.Background() - out := ForwardN(ctx, in, n) - - close(in) - _, ok := <-out // closed - if !ok { - return - } - t.Fatal("input channel closed, but output channel not") - -} - -func TestContextClosedWhenBlockingOnInput(t *testing.T) { - const n = 1 // but we won't ever send a block - ctx, cancel := context.WithCancel(context.Background()) - out := ForwardN(ctx, make(chan *blocks.Block), n) - - cancel() // before sending anything - _, ok := <-out - if !ok { - return - } - t.Fail() -} From 044db5bee8e963e64ecb057b617781e84f06203f Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 18:01:24 -0800 Subject: [PATCH 60/90] hotfix(dep) duplicates TEMP DONT MERGE TO MASTER before merging, fork and send a PR to tuxy License: MIT Signed-off-by: Brian Tiger Chow --- .../src/github.com/tuxychandru/pubsub/pubsub.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go b/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go index 9cbf9cffa..aac7ad89f 100644 --- a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go +++ b/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go @@ -66,6 +66,14 @@ func (ps *PubSub) AddSub(ch chan interface{}, topics ...string) { ps.cmdChan <- cmd{op: sub, topics: topics, ch: ch} } +// AddSubOnce adds one-time subscriptions to an existing channel. +// For each topic, a message will be sent once. +func (ps *PubSub) AddSubOnce(ch chan interface{}, topics ...string) { + for _, t := range topics { + ps.cmdChan <- cmd{op: subOnce, topics: []string{t}, ch: ch} + } +} + // Pub publishes the given message to all subscribers of // the specified topics. func (ps *PubSub) Pub(msg interface{}, topics ...string) { From e393edcc2859aa3ae7d3dd99ab7584d11b5e59fc Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 18:23:47 -0800 Subject: [PATCH 61/90] Revert "hotfix(dep) duplicates TEMP DONT MERGE TO MASTER" This reverts commit 49004e9743110ba69c07852c3a105dac2e01e1c4. --- .../src/github.com/tuxychandru/pubsub/pubsub.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go b/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go index aac7ad89f..9cbf9cffa 100644 --- a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go +++ b/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go @@ -66,14 +66,6 @@ func (ps *PubSub) AddSub(ch chan interface{}, topics ...string) { ps.cmdChan <- cmd{op: sub, topics: topics, ch: ch} } -// AddSubOnce adds one-time subscriptions to an existing channel. -// For each topic, a message will be sent once. -func (ps *PubSub) AddSubOnce(ch chan interface{}, topics ...string) { - for _, t := range topics { - ps.cmdChan <- cmd{op: subOnce, topics: []string{t}, ch: ch} - } -} - // Pub publishes the given message to all subscribers of // the specified topics. func (ps *PubSub) Pub(msg interface{}, topics ...string) { From 20382340f58851bb46d59693003582e817f11bd3 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 18:48:15 -0800 Subject: [PATCH 62/90] test(bs/n) check for duplicates received License: MIT Signed-off-by: Brian Tiger Chow --- .../notifications/notifications_test.go | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index 7352320d9..2b2f769e6 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -9,6 +9,31 @@ import ( blocks "github.com/jbenet/go-ipfs/blocks" ) +func TestDuplicates(t *testing.T) { + b1 := blocks.NewBlock([]byte("1")) + b2 := blocks.NewBlock([]byte("2")) + + n := New() + defer n.Shutdown() + ch := n.Subscribe(context.Background(), b1.Key(), b2.Key()) + + n.Publish(b1) + blockRecvd, ok := <-ch + if !ok { + t.Fail() + } + assertBlocksEqual(t, b1, blockRecvd) + + n.Publish(b1) // ignored duplicate + + n.Publish(b2) + blockRecvd, ok = <-ch + if !ok { + t.Fail() + } + assertBlocksEqual(t, b2, blockRecvd) +} + func TestPublishSubscribe(t *testing.T) { blockSent := blocks.NewBlock([]byte("Greetings from The Interval")) @@ -80,9 +105,9 @@ func assertBlockChannelNil(t *testing.T, blockChannel <-chan *blocks.Block) { func assertBlocksEqual(t *testing.T, a, b *blocks.Block) { if !bytes.Equal(a.Data, b.Data) { - t.Fail() + t.Fatal("blocks aren't equal") } if a.Key() != b.Key() { - t.Fail() + t.Fatal("block keys aren't equal") } } From 9bf1ba6ab5db0ef92194a3b9e1d6b529865fa41a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Fri, 21 Nov 2014 19:11:09 -0800 Subject: [PATCH 63/90] fix(bs/notifications) prevent duplicates @whyrusleeping now notifications _guarantees_ there won't be any duplicates License: MIT Signed-off-by: Brian Tiger Chow --- .../bitswap/notifications/notifications.go | 19 ++++++++++++++++++- .../notifications/notifications_test.go | 4 ++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index b07f3bf73..20a0f623d 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -47,7 +47,12 @@ func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Blo ps.wrapped.Unsub(valuesCh, toStrings(keys)...) close(blocksCh) }() - for _, _ = range keys { + seen := make(map[u.Key]struct{}) + i := 0 // req'd because it only counts unique block sends + for { + if i >= len(keys) { + return + } select { case <-ctx.Done(): return @@ -59,10 +64,22 @@ func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Blo if !ok { return } + if _, ok := seen[block.Key()]; ok { + continue + } select { case <-ctx.Done(): return case blocksCh <- block: // continue + // Unsub alone is insufficient for keeping out duplicates. + // It's a race to unsubscribe before pubsub handles the + // next Publish call. Therefore, must also check for + // duplicates manually. Unsub is a performance + // consideration to avoid lots of unnecessary channel + // chatter. + ps.wrapped.Unsub(valuesCh, string(block.Key())) + i++ + seen[block.Key()] = struct{}{} } } } diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index 2b2f769e6..6467f3d4f 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -52,8 +52,8 @@ func TestPublishSubscribe(t *testing.T) { } func TestSubscribeMany(t *testing.T) { - e1 := blocks.NewBlock([]byte("Greetings from The Interval")) - e2 := blocks.NewBlock([]byte("Greetings from The Interval")) + e1 := blocks.NewBlock([]byte("1")) + e2 := blocks.NewBlock([]byte("2")) n := New() defer n.Shutdown() From 6d217b53114ec053b0a3df79c544e9d9468437cc Mon Sep 17 00:00:00 2001 From: Jeromy Date: Sat, 22 Nov 2014 22:27:19 +0000 Subject: [PATCH 64/90] ensure sending of wantlist to friendly peers --- blockservice/blocks_test.go | 4 ++++ exchange/bitswap/bitswap.go | 19 ++++++++++++++++--- exchange/bitswap/bitswap_test.go | 3 ++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index 9f579c530..1779c0c83 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -10,6 +10,7 @@ import ( dssync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" + bitswap "github.com/jbenet/go-ipfs/exchange/bitswap" offline "github.com/jbenet/go-ipfs/exchange/offline" u "github.com/jbenet/go-ipfs/util" ) @@ -58,3 +59,6 @@ func TestBlocks(t *testing.T) { t.Error("Block data is not equal.") } } + +func TestGetBlocks(t *testing.T) { +} diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 95cb7ebf6..b5edfcf27 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -262,6 +262,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } + first := true for _, key := range incoming.Wantlist() { // TODO: might be better to check if we have the block before checking // if we should send it to someone @@ -272,9 +273,11 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // Create a separate message to send this block in blkmsg := bsmsg.New() - // TODO: only send this the first time - for _, k := range bs.wantlist.Keys() { - blkmsg.AddWanted(k) + if first { + for _, k := range bs.wantlist.Keys() { + blkmsg.AddWanted(k) + } + first = false } blkmsg.AddBlock(block) @@ -284,6 +287,16 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } + // If they send us a block, we should guarantee that we send + // them our updated want list one way or another + if len(incoming.Blocks()) > 0 && first { + message := bsmsg.New() + for _, k := range bs.wantlist.Keys() { + message.AddWanted(k) + } + return p, message + } + return nil, nil } diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 426c0a315..0610164a0 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -1,4 +1,4 @@ -package bitswap +package bitswap_test import ( "bytes" @@ -7,6 +7,7 @@ import ( "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + . "github.com/jbenet/go-ipfs/exchange/bitswap" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" From 07bb901ed5ad0ad3fb98f6f82d2f1696317b0538 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Sun, 23 Nov 2014 19:14:06 +0000 Subject: [PATCH 65/90] add a test to blockservice to demonstate GetBlocks failure. --- blockservice/blocks_test.go | 40 ++++++++++++ exchange/bitswap/bitswap.go | 19 +----- exchange/bitswap/bitswap_test.go | 90 +-------------------------- exchange/bitswap/testutils.go | 101 +++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 104 deletions(-) create mode 100644 exchange/bitswap/testutils.go diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index 1779c0c83..fd4ac34c3 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -11,7 +11,9 @@ import ( blocks "github.com/jbenet/go-ipfs/blocks" blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" bitswap "github.com/jbenet/go-ipfs/exchange/bitswap" + tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" offline "github.com/jbenet/go-ipfs/exchange/offline" + "github.com/jbenet/go-ipfs/routing/mock" u "github.com/jbenet/go-ipfs/util" ) @@ -61,4 +63,42 @@ func TestBlocks(t *testing.T) { } func TestGetBlocks(t *testing.T) { + net := tn.VirtualNetwork() + rs := mock.VirtualRoutingServer() + sg := bitswap.NewSessionGenerator(net, rs) + bg := bitswap.NewBlockGenerator() + + instances := sg.Instances(4) + blks := bg.Blocks(50) + // TODO: verify no duplicates + + var servs []*BlockService + for _, i := range instances { + bserv, err := New(i.Blockstore, i.Exchange) + if err != nil { + t.Fatal(err) + } + servs = append(servs, bserv) + } + + var keys []u.Key + for _, blk := range blks { + keys = append(keys, blk.Key()) + servs[0].AddBlock(blk) + } + + for i := 1; i < 4; i++ { + ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) + out := servs[i].GetBlocks(ctx, keys) + gotten := make(map[u.Key]*blocks.Block) + for blk := range out { + if _, ok := gotten[blk.Key()]; ok { + t.Fatal("Got duplicate block!") + } + gotten[blk.Key()] = blk + } + if len(gotten) != len(blks) { + t.Fatalf("Didnt get enough blocks back: %d/%d", len(gotten), len(blks)) + } + } } diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index b5edfcf27..95cb7ebf6 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -262,7 +262,6 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } - first := true for _, key := range incoming.Wantlist() { // TODO: might be better to check if we have the block before checking // if we should send it to someone @@ -273,11 +272,9 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // Create a separate message to send this block in blkmsg := bsmsg.New() - if first { - for _, k := range bs.wantlist.Keys() { - blkmsg.AddWanted(k) - } - first = false + // TODO: only send this the first time + for _, k := range bs.wantlist.Keys() { + blkmsg.AddWanted(k) } blkmsg.AddBlock(block) @@ -287,16 +284,6 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } } - // If they send us a block, we should guarantee that we send - // them our updated want list one way or another - if len(incoming.Blocks()) > 0 && first { - message := bsmsg.New() - for _, k := range bs.wantlist.Keys() { - message.AddWanted(k) - } - return p, message - } - return nil, nil } diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 0610164a0..7cd1c22f9 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -1,4 +1,4 @@ -package bitswap_test +package bitswap import ( "bytes" @@ -7,13 +7,8 @@ import ( "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - . "github.com/jbenet/go-ipfs/exchange/bitswap" - ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" - ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" - blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" - exchange "github.com/jbenet/go-ipfs/exchange" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" peer "github.com/jbenet/go-ipfs/peer" mock "github.com/jbenet/go-ipfs/routing/mock" @@ -170,7 +165,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { } } -func getOrFail(bitswap instance, b *blocks.Block, t *testing.T, wg *sync.WaitGroup) { +func getOrFail(bitswap Instance, b *blocks.Block, t *testing.T, wg *sync.WaitGroup) { if _, err := bitswap.blockstore.Get(b.Key()); err != nil { _, err := bitswap.exchange.GetBlock(context.Background(), b.Key()) if err != nil { @@ -246,84 +241,3 @@ func TestSendToWantingPeer(t *testing.T) { t.Fatal("Expected to receive alpha from me") } } - -func NewBlockGenerator() BlockGenerator { - return BlockGenerator{} -} - -type BlockGenerator struct { - seq int -} - -func (bg *BlockGenerator) Next() *blocks.Block { - bg.seq++ - return blocks.NewBlock([]byte(string(bg.seq))) -} - -func (bg *BlockGenerator) Blocks(n int) []*blocks.Block { - blocks := make([]*blocks.Block, 0) - for i := 0; i < n; i++ { - b := bg.Next() - blocks = append(blocks, b) - } - return blocks -} - -func NewSessionGenerator( - net tn.Network, rs mock.RoutingServer) SessionGenerator { - return SessionGenerator{ - net: net, - rs: rs, - seq: 0, - } -} - -type SessionGenerator struct { - seq int - net tn.Network - rs mock.RoutingServer -} - -func (g *SessionGenerator) Next() instance { - g.seq++ - return session(g.net, g.rs, []byte(string(g.seq))) -} - -func (g *SessionGenerator) Instances(n int) []instance { - instances := make([]instance, 0) - for j := 0; j < n; j++ { - inst := g.Next() - instances = append(instances, inst) - } - return instances -} - -type instance struct { - peer peer.Peer - exchange exchange.Interface - blockstore blockstore.Blockstore -} - -// session creates a test bitswap session. -// -// NB: It's easy make mistakes by providing the same peer ID to two different -// sessions. To safeguard, use the SessionGenerator to generate sessions. It's -// just a much better idea. -func session(net tn.Network, rs mock.RoutingServer, id peer.ID) instance { - p := peer.WithID(id) - - adapter := net.Adapter(p) - htc := rs.Client(p) - bstore := blockstore.NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore())) - - const alwaysSendToPeer = true - ctx := context.TODO() - - bs := New(ctx, p, adapter, htc, bstore, alwaysSendToPeer) - - return instance{ - peer: p, - exchange: bs, - blockstore: bstore, - } -} diff --git a/exchange/bitswap/testutils.go b/exchange/bitswap/testutils.go new file mode 100644 index 000000000..c32cee6f9 --- /dev/null +++ b/exchange/bitswap/testutils.go @@ -0,0 +1,101 @@ +package bitswap + +import ( + "code.google.com/p/go.net/context" + ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" + ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" + "github.com/jbenet/go-ipfs/blocks" + "github.com/jbenet/go-ipfs/blocks/blockstore" + "github.com/jbenet/go-ipfs/exchange" + tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" + "github.com/jbenet/go-ipfs/peer" + "github.com/jbenet/go-ipfs/routing/mock" +) + +/* +TODO: This whole file needs somewhere better to live. +The issue is that its very difficult to move it somewhere else +without creating circular dependencies. +Additional thought required. +*/ + +func NewBlockGenerator() BlockGenerator { + return BlockGenerator{} +} + +type BlockGenerator struct { + seq int +} + +func (bg *BlockGenerator) Next() *blocks.Block { + bg.seq++ + return blocks.NewBlock([]byte(string(bg.seq))) +} + +func (bg *BlockGenerator) Blocks(n int) []*blocks.Block { + blocks := make([]*blocks.Block, 0) + for i := 0; i < n; i++ { + b := bg.Next() + blocks = append(blocks, b) + } + return blocks +} + +func NewSessionGenerator( + net tn.Network, rs mock.RoutingServer) SessionGenerator { + return SessionGenerator{ + net: net, + rs: rs, + seq: 0, + } +} + +type SessionGenerator struct { + seq int + net tn.Network + rs mock.RoutingServer +} + +func (g *SessionGenerator) Next() Instance { + g.seq++ + return session(g.net, g.rs, []byte(string(g.seq))) +} + +func (g *SessionGenerator) Instances(n int) []Instance { + instances := make([]Instance, 0) + for j := 0; j < n; j++ { + inst := g.Next() + instances = append(instances, inst) + } + return instances +} + +type Instance struct { + Peer peer.Peer + Exchange exchange.Interface + Blockstore blockstore.Blockstore +} + +// session creates a test bitswap session. +// +// NB: It's easy make mistakes by providing the same peer ID to two different +// sessions. To safeguard, use the SessionGenerator to generate sessions. It's +// just a much better idea. +func session(net tn.Network, rs mock.RoutingServer, id peer.ID) Instance { + p := peer.WithID(id) + + adapter := net.Adapter(p) + htc := rs.Client(p) + bstore := blockstore.NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore())) + + const alwaysSendToPeer = true + ctx := context.TODO() + + bs := New(ctx, p, adapter, htc, bstore, alwaysSendToPeer) + + return Instance{ + Peer: p, + Exchange: bs, + Blockstore: bstore, + } +} From aac3c6abbd6fc700f4a305ab56bccd15ac5ed4f0 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 18:27:02 -0800 Subject: [PATCH 66/90] fix(blockservice) test License: MIT Signed-off-by: Brian Tiger Chow --- blockservice/blocks_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index fd4ac34c3..d9b2facb1 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -87,9 +87,14 @@ func TestGetBlocks(t *testing.T) { servs[0].AddBlock(blk) } + var chans []<-chan *blocks.Block for i := 1; i < 4; i++ { ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) - out := servs[i].GetBlocks(ctx, keys) + ch := servs[i].GetBlocks(ctx, keys) + chans = append(chans, ch) + } + + for _, out := range chans { gotten := make(map[u.Key]*blocks.Block) for blk := range out { if _, ok := gotten[blk.Key()]; ok { From 19de3041fb0c03e597acc1afea8382064358e044 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 22:52:43 -0800 Subject: [PATCH 67/90] fix(bitswap) build-breaking compilation errors License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap_test.go | 60 ++++++++++++++++---------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 7cd1c22f9..1da69560e 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -25,8 +25,8 @@ func TestClose(t *testing.T) { block := bgen.Next() bitswap := sesgen.Next() - bitswap.exchange.Close() - bitswap.exchange.GetBlock(context.Background(), block.Key()) + bitswap.Exchange.Close() + bitswap.Exchange.GetBlock(context.Background(), block.Key()) } func TestGetBlockTimeout(t *testing.T) { @@ -39,7 +39,7 @@ func TestGetBlockTimeout(t *testing.T) { ctx, _ := context.WithTimeout(context.Background(), time.Nanosecond) block := blocks.NewBlock([]byte("block")) - _, err := self.exchange.GetBlock(ctx, block.Key()) + _, err := self.Exchange.GetBlock(ctx, block.Key()) if err != context.DeadlineExceeded { t.Fatal("Expected DeadlineExceeded error") @@ -58,7 +58,7 @@ func TestProviderForKeyButNetworkCannotFind(t *testing.T) { solo := g.Next() ctx, _ := context.WithTimeout(context.Background(), time.Nanosecond) - _, err := solo.exchange.GetBlock(ctx, block.Key()) + _, err := solo.Exchange.GetBlock(ctx, block.Key()) if err != context.DeadlineExceeded { t.Fatal("Expected DeadlineExceeded error") @@ -76,17 +76,17 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { hasBlock := g.Next() - if err := hasBlock.blockstore.Put(block); err != nil { + if err := hasBlock.Blockstore.Put(block); err != nil { t.Fatal(err) } - if err := hasBlock.exchange.HasBlock(context.Background(), block); err != nil { + if err := hasBlock.Exchange.HasBlock(context.Background(), block); err != nil { t.Fatal(err) } wantsBlock := g.Next() ctx, _ := context.WithTimeout(context.Background(), time.Second) - received, err := wantsBlock.exchange.GetBlock(ctx, block.Key()) + received, err := wantsBlock.Exchange.GetBlock(ctx, block.Key()) if err != nil { t.Log(err) t.Fatal("Expected to succeed") @@ -135,9 +135,9 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { first := instances[0] for _, b := range blocks { - first.blockstore.Put(b) - first.exchange.HasBlock(context.Background(), b) - rs.Announce(first.peer, b.Key()) + first.Blockstore.Put(b) + first.Exchange.HasBlock(context.Background(), b) + rs.Announce(first.Peer, b.Key()) } t.Log("Distribute!") @@ -158,7 +158,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { for _, inst := range instances { for _, b := range blocks { - if _, err := inst.blockstore.Get(b.Key()); err != nil { + if _, err := inst.Blockstore.Get(b.Key()); err != nil { t.Fatal(err) } } @@ -166,8 +166,8 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { } func getOrFail(bitswap Instance, b *blocks.Block, t *testing.T, wg *sync.WaitGroup) { - if _, err := bitswap.blockstore.Get(b.Key()); err != nil { - _, err := bitswap.exchange.GetBlock(context.Background(), b.Key()) + if _, err := bitswap.Blockstore.Get(b.Key()); err != nil { + _, err := bitswap.Exchange.GetBlock(context.Background(), b.Key()) if err != nil { t.Fatal(err) } @@ -190,50 +190,50 @@ func TestSendToWantingPeer(t *testing.T) { w := sg.Next() o := sg.Next() - t.Logf("Session %v\n", me.peer) - t.Logf("Session %v\n", w.peer) - t.Logf("Session %v\n", o.peer) + t.Logf("Session %v\n", me.Peer) + t.Logf("Session %v\n", w.Peer) + t.Logf("Session %v\n", o.Peer) alpha := bg.Next() const timeout = 100 * time.Millisecond // FIXME don't depend on time - t.Logf("Peer %v attempts to get %v. NB: not available\n", w.peer, alpha.Key()) + t.Logf("Peer %v attempts to get %v. NB: not available\n", w.Peer, alpha.Key()) ctx, _ := context.WithTimeout(context.Background(), timeout) - _, err := w.exchange.GetBlock(ctx, alpha.Key()) + _, err := w.Exchange.GetBlock(ctx, alpha.Key()) if err == nil { t.Fatalf("Expected %v to NOT be available", alpha.Key()) } beta := bg.Next() - t.Logf("Peer %v announes availability of %v\n", w.peer, beta.Key()) + t.Logf("Peer %v announes availability of %v\n", w.Peer, beta.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := w.blockstore.Put(beta); err != nil { + if err := w.Blockstore.Put(beta); err != nil { t.Fatal(err) } - w.exchange.HasBlock(ctx, beta) + w.Exchange.HasBlock(ctx, beta) - t.Logf("%v gets %v from %v and discovers it wants %v\n", me.peer, beta.Key(), w.peer, alpha.Key()) + t.Logf("%v gets %v from %v and discovers it wants %v\n", me.Peer, beta.Key(), w.Peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if _, err := me.exchange.GetBlock(ctx, beta.Key()); err != nil { + if _, err := me.Exchange.GetBlock(ctx, beta.Key()); err != nil { t.Fatal(err) } - t.Logf("%v announces availability of %v\n", o.peer, alpha.Key()) + t.Logf("%v announces availability of %v\n", o.Peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := o.blockstore.Put(alpha); err != nil { + if err := o.Blockstore.Put(alpha); err != nil { t.Fatal(err) } - o.exchange.HasBlock(ctx, alpha) + o.Exchange.HasBlock(ctx, alpha) - t.Logf("%v requests %v\n", me.peer, alpha.Key()) + t.Logf("%v requests %v\n", me.Peer, alpha.Key()) ctx, _ = context.WithTimeout(context.Background(), timeout) - if _, err := me.exchange.GetBlock(ctx, alpha.Key()); err != nil { + if _, err := me.Exchange.GetBlock(ctx, alpha.Key()); err != nil { t.Fatal(err) } - t.Logf("%v should now have %v\n", w.peer, alpha.Key()) - block, err := w.blockstore.Get(alpha.Key()) + t.Logf("%v should now have %v\n", w.Peer, alpha.Key()) + block, err := w.Blockstore.Get(alpha.Key()) if err != nil { t.Fatal("Should not have received an error") } From fb5779661b40d2d2ac66e65feb36c7f606dce899 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 22:46:11 -0800 Subject: [PATCH 68/90] fix(bs/notifications) use SubOnceEach to provide uniqueness guarantee License: MIT Signed-off-by: Brian Tiger Chow vendor forked pubsub to get SubOnceEach License: MIT Signed-off-by: Brian Tiger Chow --- Godeps/Godeps.json | 8 ++-- .../maybebtc/pubsub/Godeps/Godeps.json | 13 ++++++ .../github.com/maybebtc/pubsub/Godeps/Readme | 5 +++ .../src/github.com/maybebtc/pubsub/Makefile | 2 + .../pubsub/README.md | 2 +- .../pubsub/pubsub.go | 45 +++++++++++++++---- .../pubsub/pubsub_test.go | 19 +++++++- .../bitswap/notifications/notifications.go | 28 +++--------- 8 files changed, 85 insertions(+), 37 deletions(-) create mode 100644 Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Godeps.json create mode 100644 Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Readme create mode 100644 Godeps/_workspace/src/github.com/maybebtc/pubsub/Makefile rename Godeps/_workspace/src/github.com/{tuxychandru => maybebtc}/pubsub/README.md (97%) rename Godeps/_workspace/src/github.com/{tuxychandru => maybebtc}/pubsub/pubsub.go (81%) rename Godeps/_workspace/src/github.com/{tuxychandru => maybebtc}/pubsub/pubsub_test.go (92%) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index b37ae027b..f487713e7 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -136,6 +136,10 @@ "Comment": "v0.6.0-5-gf92b795", "Rev": "f92b7950b372b1db80bd3527e4d40e42555fe6c2" }, + { + "ImportPath": "github.com/maybebtc/pubsub", + "Rev": "39ce5f556423a4c7223b370fa17a3bbd75b2d197" + }, { "ImportPath": "github.com/mitchellh/go-homedir", "Rev": "7d2d8c8a4e078ce3c58736ab521a40b37a504c52" @@ -144,10 +148,6 @@ "ImportPath": "github.com/syndtr/goleveldb/leveldb", "Rev": "99056d50e56252fbe0021d5c893defca5a76baf8" }, - { - "ImportPath": "github.com/tuxychandru/pubsub", - "Rev": "02de8aa2db3d570c5ab1be5ba67b456fd0fb7c4e" - }, { "ImportPath": "gopkg.in/natefinch/lumberjack.v2", "Comment": "v1.0-12-gd28785c", diff --git a/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Godeps.json b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Godeps.json new file mode 100644 index 000000000..3a0925c00 --- /dev/null +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Godeps.json @@ -0,0 +1,13 @@ +{ + "ImportPath": "github.com/maybebtc/pubsub", + "GoVersion": "go1.3.3", + "Packages": [ + "./..." + ], + "Deps": [ + { + "ImportPath": "gopkg.in/check.v1", + "Rev": "64131543e7896d5bcc6bd5a76287eb75ea96c673" + } + ] +} diff --git a/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Readme b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Readme new file mode 100644 index 000000000..4cdaa53d5 --- /dev/null +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Godeps/Readme @@ -0,0 +1,5 @@ +This directory tree is generated automatically by godep. + +Please do not edit. + +See https://github.com/tools/godep for more information. diff --git a/Godeps/_workspace/src/github.com/maybebtc/pubsub/Makefile b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Makefile new file mode 100644 index 000000000..ed3992384 --- /dev/null +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/Makefile @@ -0,0 +1,2 @@ +vendor: + godep save -r ./... diff --git a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/README.md b/Godeps/_workspace/src/github.com/maybebtc/pubsub/README.md similarity index 97% rename from Godeps/_workspace/src/github.com/tuxychandru/pubsub/README.md rename to Godeps/_workspace/src/github.com/maybebtc/pubsub/README.md index c1aab80b5..e176dab68 100644 --- a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/README.md +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/README.md @@ -6,7 +6,7 @@ View the [API Documentation](http://godoc.org/github.com/tuxychandru/pubsub). ## License -Copyright (c) 2013, Chandra Sekar S +Copyright (c) 2013, Chandra Sekar S All rights reserved. Redistribution and use in source and binary forms, with or without diff --git a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go b/Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub.go similarity index 81% rename from Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go rename to Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub.go index 9cbf9cffa..f42587d07 100644 --- a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub.go +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub.go @@ -15,6 +15,7 @@ type operation int const ( sub operation = iota subOnce + subOnceEach pub unsub unsubAll @@ -55,6 +56,12 @@ func (ps *PubSub) SubOnce(topics ...string) chan interface{} { return ps.sub(subOnce, topics...) } +// SubOnceEach returns a channel on which callers receive, at most, one message +// for each topic. +func (ps *PubSub) SubOnceEach(topics ...string) chan interface{} { + return ps.sub(subOnceEach, topics...) +} + func (ps *PubSub) sub(op operation, topics ...string) chan interface{} { ch := make(chan interface{}, ps.capacity) ps.cmdChan <- cmd{op: op, topics: topics, ch: ch} @@ -66,6 +73,12 @@ func (ps *PubSub) AddSub(ch chan interface{}, topics ...string) { ps.cmdChan <- cmd{op: sub, topics: topics, ch: ch} } +// AddSubOnceEach adds subscriptions to an existing channel with SubOnceEach +// behavior. +func (ps *PubSub) AddSubOnceEach(ch chan interface{}, topics ...string) { + ps.cmdChan <- cmd{op: subOnceEach, topics: topics, ch: ch} +} + // Pub publishes the given message to all subscribers of // the specified topics. func (ps *PubSub) Pub(msg interface{}, topics ...string) { @@ -98,7 +111,7 @@ func (ps *PubSub) Shutdown() { func (ps *PubSub) start() { reg := registry{ - topics: make(map[string]map[chan interface{}]bool), + topics: make(map[string]map[chan interface{}]subtype), revTopics: make(map[chan interface{}]map[string]bool), } @@ -119,10 +132,13 @@ loop: for _, topic := range cmd.topics { switch cmd.op { case sub: - reg.add(topic, cmd.ch, false) + reg.add(topic, cmd.ch, stNorm) case subOnce: - reg.add(topic, cmd.ch, true) + reg.add(topic, cmd.ch, stOnceAny) + + case subOnceEach: + reg.add(topic, cmd.ch, stOnceEach) case pub: reg.send(topic, cmd.msg) @@ -146,15 +162,23 @@ loop: // registry maintains the current subscription state. It's not // safe to access a registry from multiple goroutines simultaneously. type registry struct { - topics map[string]map[chan interface{}]bool + topics map[string]map[chan interface{}]subtype revTopics map[chan interface{}]map[string]bool } -func (reg *registry) add(topic string, ch chan interface{}, once bool) { +type subtype int + +const ( + stOnceAny = iota + stOnceEach + stNorm +) + +func (reg *registry) add(topic string, ch chan interface{}, st subtype) { if reg.topics[topic] == nil { - reg.topics[topic] = make(map[chan interface{}]bool) + reg.topics[topic] = make(map[chan interface{}]subtype) } - reg.topics[topic][ch] = once + reg.topics[topic][ch] = st if reg.revTopics[ch] == nil { reg.revTopics[ch] = make(map[string]bool) @@ -163,12 +187,15 @@ func (reg *registry) add(topic string, ch chan interface{}, once bool) { } func (reg *registry) send(topic string, msg interface{}) { - for ch, once := range reg.topics[topic] { + for ch, st := range reg.topics[topic] { ch <- msg - if once { + switch st { + case stOnceAny: for topic := range reg.revTopics[ch] { reg.remove(topic, ch) } + case stOnceEach: + reg.remove(topic, ch) } } } diff --git a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub_test.go b/Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub_test.go similarity index 92% rename from Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub_test.go rename to Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub_test.go index 16392d33b..f4a80ab62 100644 --- a/Godeps/_workspace/src/github.com/tuxychandru/pubsub/pubsub_test.go +++ b/Godeps/_workspace/src/github.com/maybebtc/pubsub/pubsub_test.go @@ -5,7 +5,7 @@ package pubsub import ( - check "launchpad.net/gocheck" + check "gopkg.in/check.v1" "runtime" "testing" "time" @@ -181,6 +181,23 @@ func (s *Suite) TestMultiSubOnce(c *check.C) { ps.Shutdown() } +func (s *Suite) TestMultiSubOnceEach(c *check.C) { + ps := New(1) + ch := ps.SubOnceEach("t1", "t2") + + ps.Pub("hi", "t1") + c.Check(<-ch, check.Equals, "hi") + + ps.Pub("hi!", "t1") // ignored + + ps.Pub("hello", "t2") + c.Check(<-ch, check.Equals, "hello") + + _, ok := <-ch + c.Check(ok, check.Equals, false) + ps.Shutdown() +} + func (s *Suite) TestMultiPub(c *check.C) { ps := New(1) ch1 := ps.Sub("t1") diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index 20a0f623d..e9aac629c 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -2,7 +2,7 @@ package notifications import ( context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - pubsub "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/tuxychandru/pubsub" + pubsub "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/maybebtc/pubsub" blocks "github.com/jbenet/go-ipfs/blocks" u "github.com/jbenet/go-ipfs/util" @@ -39,20 +39,16 @@ func (ps *impl) Shutdown() { func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { blocksCh := make(chan *blocks.Block, len(keys)) - valuesCh := make(chan interface{}, len(keys)) - ps.wrapped.AddSub(valuesCh, toStrings(keys)...) - + if len(keys) == 0 { + close(blocksCh) + return blocksCh + } + valuesCh := ps.wrapped.SubOnceEach(toStrings(keys)...) go func() { defer func() { - ps.wrapped.Unsub(valuesCh, toStrings(keys)...) close(blocksCh) }() - seen := make(map[u.Key]struct{}) - i := 0 // req'd because it only counts unique block sends for { - if i >= len(keys) { - return - } select { case <-ctx.Done(): return @@ -64,22 +60,10 @@ func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Blo if !ok { return } - if _, ok := seen[block.Key()]; ok { - continue - } select { case <-ctx.Done(): return case blocksCh <- block: // continue - // Unsub alone is insufficient for keeping out duplicates. - // It's a race to unsubscribe before pubsub handles the - // next Publish call. Therefore, must also check for - // duplicates manually. Unsub is a performance - // consideration to avoid lots of unnecessary channel - // chatter. - ps.wrapped.Unsub(valuesCh, string(block.Key())) - i++ - seen[block.Key()] = struct{}{} } } } From bef75d5061d66f4ec10a492b2669bea6c54cab99 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 22:47:06 -0800 Subject: [PATCH 69/90] fix(bitswap/testutils) vendor License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/testutils.go | 2 +- unixfs/io/dagreader.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/testutils.go b/exchange/bitswap/testutils.go index c32cee6f9..d0064173f 100644 --- a/exchange/bitswap/testutils.go +++ b/exchange/bitswap/testutils.go @@ -1,7 +1,7 @@ package bitswap import ( - "code.google.com/p/go.net/context" + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" "github.com/jbenet/go-ipfs/blocks" diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index 7373b94ae..55e677386 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -5,7 +5,7 @@ import ( "errors" "io" - "code.google.com/p/go.net/context" + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" mdag "github.com/jbenet/go-ipfs/merkledag" From e27de2bfa2369b4629749d718182fdfdff0bb61e Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 22:47:53 -0800 Subject: [PATCH 70/90] fix(blockservice) respect context in GetBlocks License: MIT Signed-off-by: Brian Tiger Chow --- blockservice/blockservice.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 07e6d1092..0ddec7955 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -99,29 +99,37 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er // the returned channel. // NB: No guarantees are made about order. func (s *BlockService) GetBlocks(ctx context.Context, ks []u.Key) <-chan *blocks.Block { - out := make(chan *blocks.Block, 32) + out := make(chan *blocks.Block, 0) go func() { - var toFetch []u.Key + defer close(out) + var misses []u.Key for _, k := range ks { - block, err := s.Blockstore.Get(k) + hit, err := s.Blockstore.Get(k) if err != nil { - toFetch = append(toFetch, k) + misses = append(misses, k) continue } log.Debug("Blockservice: Got data in datastore.") - out <- block + select { + case out <- hit: + case <-ctx.Done(): + return + } } - nblocks, err := s.Remote.GetBlocks(ctx, toFetch) + rblocks, err := s.Remote.GetBlocks(ctx, misses) if err != nil { log.Errorf("Error with GetBlocks: %s", err) return } - for blk := range nblocks { - out <- blk + for b := range rblocks { + select { + case out <- b: + case <-ctx.Done(): + return + } } - close(out) }() return out } From d721c448af1770db259b7c274ac882bf921a2a60 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sun, 23 Nov 2014 22:59:22 -0800 Subject: [PATCH 71/90] reset test to the way it ways before @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- blockservice/blocks_test.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index d9b2facb1..9fd85725e 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -62,7 +62,7 @@ func TestBlocks(t *testing.T) { } } -func TestGetBlocks(t *testing.T) { +func TestGetBlocksSequential(t *testing.T) { net := tn.VirtualNetwork() rs := mock.VirtualRoutingServer() sg := bitswap.NewSessionGenerator(net, rs) @@ -87,14 +87,11 @@ func TestGetBlocks(t *testing.T) { servs[0].AddBlock(blk) } - var chans []<-chan *blocks.Block - for i := 1; i < 4; i++ { - ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) - ch := servs[i].GetBlocks(ctx, keys) - chans = append(chans, ch) - } + t.Log("one instance at a time, get blocks concurrently") - for _, out := range chans { + for i := 1; i < len(instances); i++ { + ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) + out := servs[i].GetBlocks(ctx, keys) gotten := make(map[u.Key]*blocks.Block) for blk := range out { if _, ok := gotten[blk.Key()]; ok { From 4cc1780705c3c627a2feee8be12731ebf1b287df Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 24 Nov 2014 08:28:48 +0000 Subject: [PATCH 72/90] fix issues in merkledag --- core/core.go | 1 + exchange/bitswap/network/ipfs_impl.go | 3 --- merkledag/merkledag.go | 35 ++++++++++++++++++--------- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/core/core.go b/core/core.go index fcd421fc6..b3f858ddd 100644 --- a/core/core.go +++ b/core/core.go @@ -115,6 +115,7 @@ func NewIpfsNode(cfg *config.Config, online bool) (n *IpfsNode, err error) { Config: cfg, } n.ContextCloser = ctxc.NewContextCloser(ctx, n.teardown) + ctx = n.Context() // setup datastore. if n.Datastore, err = makeDatastore(cfg.Datastore); err != nil { diff --git a/exchange/bitswap/network/ipfs_impl.go b/exchange/bitswap/network/ipfs_impl.go index c94a4859f..1a3c11b44 100644 --- a/exchange/bitswap/network/ipfs_impl.go +++ b/exchange/bitswap/network/ipfs_impl.go @@ -1,8 +1,6 @@ package network import ( - "errors" - context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" @@ -54,7 +52,6 @@ func (bsnet *impl) HandleMessage( // TODO(brian): put this in a helper function if bsmsg == nil || p == nil { - bsnet.receiver.ReceiveError(errors.New("ReceiveMessage returned nil peer or message")) return nil } diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 2673c59fc..06381bacf 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -163,17 +163,6 @@ func (n *Node) Multihash() (mh.Multihash, error) { return n.cached, nil } -// Searches this nodes links for one to the given key, -// returns the index of said link -func (n *Node) FindLink(k u.Key) (int, error) { - for i, lnk := range n.Links { - if u.Key(lnk.Hash) == k { - return i, nil - } - } - return -1, u.ErrNotFound -} - // Key returns the Multihash as a key, for maps. func (n *Node) Key() (u.Key, error) { h, err := n.Multihash() @@ -298,6 +287,17 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} return done } +// Searches this nodes links for one to the given key, +// returns the index of said link +func FindLink(n *Node, k u.Key, found []*Node) (int, error) { + for i, lnk := range n.Links { + if u.Key(lnk.Hash) == k && found[i] == nil { + return i, nil + } + } + return -1, u.ErrNotFound +} + // BatchFetch will fill out all of the links of the given Node. // It returns a channel of nodes, which the caller can receive // all the child nodes of 'root' on, in proper order. @@ -324,7 +324,7 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { count := 0 for blk := range blkchan { count++ - i, err := root.FindLink(blk.Key()) + i, err := FindLink(root, blk.Key(), nodes) if err != nil { panic("Received block that wasnt in this nodes links!") } @@ -356,3 +356,14 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { return sig } + +func checkForDupes(ks []u.Key) bool { + seen := make(map[u.Key]struct{}) + for _, k := range ks { + if _, ok := seen[k]; ok { + return true + } + seen[k] = struct{}{} + } + return false +} From d06343083e0466878c339c64699e1235a51071dd Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 25 Nov 2014 18:45:44 +0000 Subject: [PATCH 73/90] add a test in merkledag to exercise GetBlocks --- merkledag/merkledag_test.go | 94 ++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/merkledag/merkledag_test.go b/merkledag/merkledag_test.go index 2db166beb..0f628e6c1 100644 --- a/merkledag/merkledag_test.go +++ b/merkledag/merkledag_test.go @@ -1,9 +1,20 @@ -package merkledag +package merkledag_test import ( + "bytes" "fmt" + "io" + "io/ioutil" "testing" + bserv "github.com/jbenet/go-ipfs/blockservice" + bs "github.com/jbenet/go-ipfs/exchange/bitswap" + tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" + imp "github.com/jbenet/go-ipfs/importer" + chunk "github.com/jbenet/go-ipfs/importer/chunk" + . "github.com/jbenet/go-ipfs/merkledag" + "github.com/jbenet/go-ipfs/routing/mock" + uio "github.com/jbenet/go-ipfs/unixfs/io" u "github.com/jbenet/go-ipfs/util" ) @@ -56,3 +67,84 @@ func TestNode(t *testing.T) { printn("boop", n2) printn("beep boop", n3) } + +func makeTestDag(t *testing.T) *Node { + read := io.LimitReader(u.NewTimeSeededRand(), 1024*32) + spl := &chunk.SizeSplitter{512} + root, err := imp.NewDagFromReaderWithSplitter(read, spl) + if err != nil { + t.Fatal(err) + } + return root +} + +func TestBatchFetch(t *testing.T) { + net := tn.VirtualNetwork() + rs := mock.VirtualRoutingServer() + sg := bs.NewSessionGenerator(net, rs) + + instances := sg.Instances(5) + + var servs []*bserv.BlockService + var dagservs []DAGService + for _, i := range instances { + bsi, err := bserv.New(i.Blockstore, i.Exchange) + if err != nil { + t.Fatal(err) + } + servs = append(servs, bsi) + dagservs = append(dagservs, NewDAGService(bsi)) + } + t.Log("finished setup.") + + root := makeTestDag(t) + read, err := uio.NewDagReader(root, nil) + if err != nil { + t.Fatal(err) + } + expected, err := ioutil.ReadAll(read) + if err != nil { + t.Fatal(err) + } + + err = dagservs[0].AddRecursive(root) + if err != nil { + t.Fatal(err) + } + + t.Log("Added file to first node.") + + k, err := root.Key() + if err != nil { + t.Fatal(err) + } + + done := make(chan struct{}) + for i := 1; i < len(dagservs); i++ { + go func(i int) { + first, err := dagservs[i].Get(k) + if err != nil { + t.Fatal(err) + } + fmt.Println("Got first node back.") + + read, err := uio.NewDagReader(first, dagservs[i]) + if err != nil { + t.Fatal(err) + } + datagot, err := ioutil.ReadAll(read) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(datagot, expected) { + t.Fatal("Got bad data back!") + } + done <- struct{}{} + }(i) + } + + for i := 1; i < len(dagservs); i++ { + <-done + } +} From 7a3819a52809d0f4612accb6a0144f31f6b813b7 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 26 Nov 2014 12:08:40 -0800 Subject: [PATCH 74/90] refactor(util) move block generator @whyrusleeping @jbenet Putting the block generator in a util dir until blocks. Can't put it in util/testutil because the util/testutil/dag-generator imports blockservice and blockservice uses the generator. Tough problem. This'll do for now. License: MIT Signed-off-by: Brian Tiger Chow --- blocks/blocksutil/block_generator.go | 25 +++++++++++++++++++++++ blockservice/blocks_test.go | 3 ++- exchange/bitswap/bitswap_test.go | 8 ++++---- exchange/bitswap/testutils.go | 30 ---------------------------- 4 files changed, 31 insertions(+), 35 deletions(-) create mode 100644 blocks/blocksutil/block_generator.go diff --git a/blocks/blocksutil/block_generator.go b/blocks/blocksutil/block_generator.go new file mode 100644 index 000000000..20f6a3edc --- /dev/null +++ b/blocks/blocksutil/block_generator.go @@ -0,0 +1,25 @@ +package blocksutil + +import "github.com/jbenet/go-ipfs/blocks" + +func NewBlockGenerator() BlockGenerator { + return BlockGenerator{} +} + +type BlockGenerator struct { + seq int +} + +func (bg *BlockGenerator) Next() *blocks.Block { + bg.seq++ + return blocks.NewBlock([]byte(string(bg.seq))) +} + +func (bg *BlockGenerator) Blocks(n int) []*blocks.Block { + blocks := make([]*blocks.Block, 0) + for i := 0; i < n; i++ { + b := bg.Next() + blocks = append(blocks, b) + } + return blocks +} diff --git a/blockservice/blocks_test.go b/blockservice/blocks_test.go index 9fd85725e..1a75723e2 100644 --- a/blockservice/blocks_test.go +++ b/blockservice/blocks_test.go @@ -10,6 +10,7 @@ import ( dssync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" blocks "github.com/jbenet/go-ipfs/blocks" blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" + blocksutil "github.com/jbenet/go-ipfs/blocks/blocksutil" bitswap "github.com/jbenet/go-ipfs/exchange/bitswap" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" offline "github.com/jbenet/go-ipfs/exchange/offline" @@ -66,7 +67,7 @@ func TestGetBlocksSequential(t *testing.T) { net := tn.VirtualNetwork() rs := mock.VirtualRoutingServer() sg := bitswap.NewSessionGenerator(net, rs) - bg := bitswap.NewBlockGenerator() + bg := blocksutil.NewBlockGenerator() instances := sg.Instances(4) blks := bg.Blocks(50) diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 1da69560e..ede87c474 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -7,8 +7,8 @@ import ( "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - blocks "github.com/jbenet/go-ipfs/blocks" + blocksutil "github.com/jbenet/go-ipfs/blocks/blocksutil" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" peer "github.com/jbenet/go-ipfs/peer" mock "github.com/jbenet/go-ipfs/routing/mock" @@ -20,7 +20,7 @@ func TestClose(t *testing.T) { vnet := tn.VirtualNetwork() rout := mock.VirtualRoutingServer() sesgen := NewSessionGenerator(vnet, rout) - bgen := NewBlockGenerator() + bgen := blocksutil.NewBlockGenerator() block := bgen.Next() bitswap := sesgen.Next() @@ -124,7 +124,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { net := tn.VirtualNetwork() rs := mock.VirtualRoutingServer() sg := NewSessionGenerator(net, rs) - bg := NewBlockGenerator() + bg := blocksutil.NewBlockGenerator() t.Log("Test a few nodes trying to get one file with a lot of blocks") @@ -184,7 +184,7 @@ func TestSendToWantingPeer(t *testing.T) { net := tn.VirtualNetwork() rs := mock.VirtualRoutingServer() sg := NewSessionGenerator(net, rs) - bg := NewBlockGenerator() + bg := blocksutil.NewBlockGenerator() me := sg.Next() w := sg.Next() diff --git a/exchange/bitswap/testutils.go b/exchange/bitswap/testutils.go index d0064173f..402a5b1d2 100644 --- a/exchange/bitswap/testutils.go +++ b/exchange/bitswap/testutils.go @@ -4,7 +4,6 @@ import ( "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" ds "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" ds_sync "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" - "github.com/jbenet/go-ipfs/blocks" "github.com/jbenet/go-ipfs/blocks/blockstore" "github.com/jbenet/go-ipfs/exchange" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" @@ -12,35 +11,6 @@ import ( "github.com/jbenet/go-ipfs/routing/mock" ) -/* -TODO: This whole file needs somewhere better to live. -The issue is that its very difficult to move it somewhere else -without creating circular dependencies. -Additional thought required. -*/ - -func NewBlockGenerator() BlockGenerator { - return BlockGenerator{} -} - -type BlockGenerator struct { - seq int -} - -func (bg *BlockGenerator) Next() *blocks.Block { - bg.seq++ - return blocks.NewBlock([]byte(string(bg.seq))) -} - -func (bg *BlockGenerator) Blocks(n int) []*blocks.Block { - blocks := make([]*blocks.Block, 0) - for i := 0; i < n; i++ { - b := bg.Next() - blocks = append(blocks, b) - } - return blocks -} - func NewSessionGenerator( net tn.Network, rs mock.RoutingServer) SessionGenerator { return SessionGenerator{ From bb0b5f7c0b682a36fedb6fe4f0d1e82797d877a3 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 26 Nov 2014 12:06:39 -0800 Subject: [PATCH 75/90] fix(notifications) prevent deadlock when context cancelled early + test(notifications) cc @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- .../bitswap/notifications/notifications.go | 8 ++--- .../notifications/notifications_test.go | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/notifications/notifications.go b/exchange/bitswap/notifications/notifications.go index e9aac629c..4616ac735 100644 --- a/exchange/bitswap/notifications/notifications.go +++ b/exchange/bitswap/notifications/notifications.go @@ -39,15 +39,15 @@ func (ps *impl) Shutdown() { func (ps *impl) Subscribe(ctx context.Context, keys ...u.Key) <-chan *blocks.Block { blocksCh := make(chan *blocks.Block, len(keys)) + valuesCh := make(chan interface{}, len(keys)) // provide our own channel to control buffer, prevent blocking if len(keys) == 0 { close(blocksCh) return blocksCh } - valuesCh := ps.wrapped.SubOnceEach(toStrings(keys)...) + ps.wrapped.AddSubOnceEach(valuesCh, toStrings(keys)...) go func() { - defer func() { - close(blocksCh) - }() + defer close(blocksCh) + defer ps.wrapped.Unsub(valuesCh) // with a len(keys) buffer, this is an optimization for { select { case <-ctx.Done(): diff --git a/exchange/bitswap/notifications/notifications_test.go b/exchange/bitswap/notifications/notifications_test.go index 6467f3d4f..3a6ada1ea 100644 --- a/exchange/bitswap/notifications/notifications_test.go +++ b/exchange/bitswap/notifications/notifications_test.go @@ -7,6 +7,8 @@ import ( context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" blocks "github.com/jbenet/go-ipfs/blocks" + blocksutil "github.com/jbenet/go-ipfs/blocks/blocksutil" + "github.com/jbenet/go-ipfs/util" ) func TestDuplicates(t *testing.T) { @@ -96,6 +98,34 @@ func TestCarryOnWhenDeadlineExpires(t *testing.T) { assertBlockChannelNil(t, blockChannel) } +func TestDoesNotDeadLockIfContextCancelledBeforePublish(t *testing.T) { + + g := blocksutil.NewBlockGenerator() + ctx, cancel := context.WithCancel(context.Background()) + n := New() + defer n.Shutdown() + + t.Log("generate a large number of blocks. exceed default buffer") + bs := g.Blocks(1000) + ks := func() []util.Key { + var keys []util.Key + for _, b := range bs { + keys = append(keys, b.Key()) + } + return keys + }() + + _ = n.Subscribe(ctx, ks...) // ignore received channel + + t.Log("cancel context before any blocks published") + cancel() + for _, b := range bs { + n.Publish(b) + } + + t.Log("publishing the large number of blocks to the ignored channel must not deadlock") +} + func assertBlockChannelNil(t *testing.T, blockChannel <-chan *blocks.Block) { _, ok := <-blockChannel if ok { From 829eac3012ab958914626d1963ced83a875d68f4 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 26 Nov 2014 12:58:35 -0800 Subject: [PATCH 76/90] fix(bitswap) pass derived context to called functions @whyrusleeping @jbenet License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 95cb7ebf6..125561889 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -87,9 +87,13 @@ type bitswap struct { // deadline enforced by the context. func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, error) { - // make sure to derive a new |ctx| and pass it to children. It's correct to - // listen on |parent| here, but incorrect to pass |parent| to new async - // functions. This is difficult to enforce. May this comment keep you safe. + // Any async work initiated by this function must end when this function + // returns. To ensure this, derive a new context. Note that it is okay to + // listen on parent in this scope, but NOT okay to pass |parent| to + // functions called by this one. Otherwise those functions won't return + // when this context Otherwise those functions won't return when this + // context's cancel func is executed. This is difficult to enforce. May + // this comment keep you safe. ctx, cancelFunc := context.WithCancel(parent) @@ -101,7 +105,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err log.Event(ctx, "GetBlockRequestEnd", &k) }() - promise, err := bs.GetBlocks(parent, []u.Key{k}) + promise, err := bs.GetBlocks(ctx, []u.Key{k}) if err != nil { return nil, err } From 1e7b7efa7692f4d179fd669bcef535cdb6062a1a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 26 Nov 2014 14:22:10 -0800 Subject: [PATCH 77/90] refactor(bitswap) perform Publish in HasBlock License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 125561889..490ae0d47 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -224,8 +224,10 @@ func (bs *bitswap) loop(parent context.Context) { // HasBlock announces the existance of a block to this bitswap service. The // service will potentially notify its peers. func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { + // TODO check all errors log.Debugf("Has Block %s", blk.Key()) bs.wantlist.Remove(blk.Key()) + bs.notifications.Publish(blk) bs.sendToPeersThatWant(ctx, blk) return bs.routing.Provide(ctx, blk.Key()) } @@ -258,8 +260,6 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm log.Criticalf("error putting block: %s", err) continue // FIXME(brian): err ignored } - bs.notifications.Publish(block) - bs.wantlist.Remove(block.Key()) err := bs.HasBlock(ctx, block) if err != nil { log.Warningf("HasBlock errored: %s", err) From f0a4fdad59418ec631d23843ed1f23c8351c1c1e Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 26 Nov 2014 22:50:41 +0000 Subject: [PATCH 78/90] some bitswap cleanup --- core/core.go | 1 + exchange/bitswap/bitswap.go | 63 ++++++++++++++++++--------- exchange/bitswap/strategy/strategy.go | 2 +- merkledag/merkledag.go | 38 ++++------------ unixfs/io/dagreader.go | 2 +- 5 files changed, 54 insertions(+), 52 deletions(-) diff --git a/core/core.go b/core/core.go index b3f858ddd..148853bfc 100644 --- a/core/core.go +++ b/core/core.go @@ -31,6 +31,7 @@ import ( u "github.com/jbenet/go-ipfs/util" ctxc "github.com/jbenet/go-ipfs/util/ctxcloser" debugerror "github.com/jbenet/go-ipfs/util/debugerror" + "github.com/jbenet/go-ipfs/util/eventlog" ) const IpnsValidatorTag = "ipns" diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 490ae0d47..9cfe5875d 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -16,11 +16,14 @@ import ( strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" - "github.com/jbenet/go-ipfs/util/eventlog" + eventlog "github.com/jbenet/go-ipfs/util/eventlog" ) var log = eventlog.Logger("bitswap") +// Number of providers to request for sending a wantlist to +const maxProvidersPerRequest = 6 + // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. @@ -97,7 +100,7 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err ctx, cancelFunc := context.WithCancel(parent) - ctx = eventlog.ContextWithMetadata(ctx, eventlog.Uuid("GetBlockRequest")) + ctx = eventlog.ContextWithLoggable(ctx, eventlog.Uuid("GetBlockRequest")) log.Event(ctx, "GetBlockRequestBegin", &k) defer func() { @@ -176,14 +179,29 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e return nil } +func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { + done := make(chan struct{}) + for _, k := range ks { + go func(k u.Key) { + providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + + err := bs.sendWantListTo(ctx, providers) + if err != nil { + log.Errorf("error sending wantlist: %s", err) + } + done <- struct{}{} + }(k) + } + for _ = range ks { + <-done + } +} + // TODO ensure only one active request per key func (bs *bitswap) loop(parent context.Context) { ctx, cancel := context.WithCancel(parent) - // Every so often, we should resend out our current want list - rebroadcastTime := time.Second * 5 - broadcastSignal := time.NewTicker(bs.strategy.GetRebroadcastDelay()) defer func() { cancel() // signal to derived async functions @@ -193,15 +211,12 @@ func (bs *bitswap) loop(parent context.Context) { for { select { case <-broadcastSignal.C: - for _, k := range bs.wantlist.Keys() { - providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) - } - } + bs.sendWantlistToProviders(ctx, bs.wantlist.Keys()) case ks := <-bs.batchRequests: // TODO: implement batching on len(ks) > X for some X + // i.e. if given 20 keys, fetch first five, then next + // five, and so on, so we are more likely to be able to + // effectively stream the data if len(ks) == 0 { log.Warning("Received batch request for zero blocks") continue @@ -232,6 +247,18 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { return bs.routing.Provide(ctx, blk.Key()) } +func (bs *bitswap) receiveBlock(ctx context.Context, block *blocks.Block) { + // TODO verify blocks? + if err := bs.blockstore.Put(block); err != nil { + log.Criticalf("error putting block: %s", err) + return + } + err := bs.HasBlock(ctx, block) + if err != nil { + log.Warningf("HasBlock errored: %s", err) + } +} + // TODO(brian): handle errors func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsmsg.BitSwapMessage) ( peer.Peer, bsmsg.BitSwapMessage) { @@ -255,15 +282,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm bs.strategy.MessageReceived(p, incoming) // FIRST for _, block := range incoming.Blocks() { - // TODO verify blocks? - if err := bs.blockstore.Put(block); err != nil { - log.Criticalf("error putting block: %s", err) - continue // FIXME(brian): err ignored - } - err := bs.HasBlock(ctx, block) - if err != nil { - log.Warningf("HasBlock errored: %s", err) - } + go bs.receiveBlock(ctx, block) } for _, key := range incoming.Wantlist() { @@ -277,6 +296,8 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm blkmsg := bsmsg.New() // TODO: only send this the first time + // no sense in sending our wantlist to the + // same peer multiple times for _, k := range bs.wantlist.Keys() { blkmsg.AddWanted(k) } diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index d86092da6..fb353d84a 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -148,5 +148,5 @@ func (s *strategist) GetBatchSize() int { } func (s *strategist) GetRebroadcastDelay() time.Duration { - return time.Second * 2 + return time.Second * 5 } diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 06381bacf..7dadf722d 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -28,7 +28,7 @@ type DAGService interface { AddRecursive(*Node) error Get(u.Key) (*Node, error) Remove(*Node) error - BatchFetch(context.Context, *Node) <-chan *Node + GetKeysAsync(context.Context, *Node) <-chan *Node } func NewDAGService(bs *bserv.BlockService) DAGService { @@ -298,41 +298,33 @@ func FindLink(n *Node, k u.Key, found []*Node) (int, error) { return -1, u.ErrNotFound } -// BatchFetch will fill out all of the links of the given Node. +// GetKeysAsync will fill out all of the links of the given Node. // It returns a channel of nodes, which the caller can receive // all the child nodes of 'root' on, in proper order. -func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { +func (ds *dagService) GetKeysAsync(ctx context.Context, root *Node) <-chan *Node { sig := make(chan *Node) go func() { var keys []u.Key nodes := make([]*Node, len(root.Links)) - //temp - recvd := []int{} - // - - // - next := 0 - // - for _, lnk := range root.Links { keys = append(keys, u.Key(lnk.Hash)) } blkchan := ds.Blocks.GetBlocks(ctx, keys) - count := 0 + next := 0 for blk := range blkchan { - count++ i, err := FindLink(root, blk.Key(), nodes) if err != nil { + // NB: can only occur as a result of programmer error panic("Received block that wasnt in this nodes links!") } - recvd = append(recvd, i) - nd, err := Decoded(blk.Data) if err != nil { + // NB: can occur in normal situations, with improperly formatted + // input data log.Error("Got back bad block!") break } @@ -347,23 +339,11 @@ func (ds *dagService) BatchFetch(ctx context.Context, root *Node) <-chan *Node { } } if next < len(nodes) { - log.Errorf("count = %d, links = %d", count, len(nodes)) - log.Error(recvd) - panic("didnt receive all requested blocks!") + // TODO: bubble errors back up. + log.Errorf("Did not receive correct number of nodes!") } close(sig) }() return sig } - -func checkForDupes(ks []u.Key) bool { - seen := make(map[u.Key]struct{}) - for _, k := range ks { - if _, ok := seen[k]; ok { - return true - } - seen[k] = struct{}{} - } - return false -} diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index 55e677386..b41ac3daa 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -40,7 +40,7 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { case ftpb.Data_File: var fetchChan <-chan *mdag.Node if serv != nil { - fetchChan = serv.BatchFetch(context.TODO(), n) + fetchChan = serv.GetKeysAsync(context.TODO(), n) } return &DagReader{ node: n, From bc02b77b47f34586026563e241fb24914b36f90f Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 26 Nov 2014 23:48:43 +0000 Subject: [PATCH 79/90] document bitswap more --- exchange/bitswap/bitswap.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 9cfe5875d..94c4cde88 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -22,7 +22,8 @@ import ( var log = eventlog.Logger("bitswap") // Number of providers to request for sending a wantlist to -const maxProvidersPerRequest = 6 +// TODO: if a 'non-nice' strategy is implemented, consider increasing this value +const maxProvidersPerRequest = 3 // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as @@ -211,6 +212,7 @@ func (bs *bitswap) loop(parent context.Context) { for { select { case <-broadcastSignal.C: + // Resend unfulfilled wantlist keys bs.sendWantlistToProviders(ctx, bs.wantlist.Keys()) case ks := <-bs.batchRequests: // TODO: implement batching on len(ks) > X for some X @@ -224,6 +226,13 @@ func (bs *bitswap) loop(parent context.Context) { for _, k := range ks { bs.wantlist.Add(k) } + // NB: send want list to providers for the first peer in this list. + // the assumption is made that the providers of the first key in + // the set are likely to have others as well. + // This currently holds true in most every situation, since when + // pinning a file, you store and provide all blocks associated with + // it. Later, this assumption may not hold as true if we implement + // newer bitswap strategies. providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) err := bs.sendWantListTo(ctx, providers) @@ -263,7 +272,6 @@ func (bs *bitswap) receiveBlock(ctx context.Context, block *blocks.Block) { func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsmsg.BitSwapMessage) ( peer.Peer, bsmsg.BitSwapMessage) { log.Debugf("ReceiveMessage from %s", p) - log.Debugf("Message wantlist: %v", incoming.Wantlist()) if p == nil { log.Error("Received message from nil peer!") @@ -279,15 +287,17 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // Record message bytes in ledger // TODO: this is bad, and could be easily abused. // Should only track *useful* messages in ledger - bs.strategy.MessageReceived(p, incoming) // FIRST + // This call records changes to wantlists, blocks received, + // and number of bytes transfered. + bs.strategy.MessageReceived(p, incoming) - for _, block := range incoming.Blocks() { - go bs.receiveBlock(ctx, block) - } + go func() { + for _, block := range incoming.Blocks() { + bs.receiveBlock(ctx, block) + } + }() for _, key := range incoming.Wantlist() { - // TODO: might be better to check if we have the block before checking - // if we should send it to someone if bs.strategy.ShouldSendBlockToPeer(key, p) { if block, errBlockNotFound := bs.blockstore.Get(key); errBlockNotFound != nil { continue @@ -303,12 +313,12 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm } blkmsg.AddBlock(block) - bs.strategy.MessageSent(p, blkmsg) bs.send(ctx, p, blkmsg) } } } + // TODO: consider changing this function to not return anything return nil, nil } @@ -326,7 +336,7 @@ func (bs *bitswap) send(ctx context.Context, p peer.Peer, m bsmsg.BitSwapMessage } func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block *blocks.Block) { - log.Debugf("Sending %v to peers that want it", block.Key()) + log.Debugf("Sending %s to peers that want it", block) for _, p := range bs.strategy.Peers() { if bs.strategy.BlockIsWantedByPeer(block.Key(), p) { From 9835c1e335ac4205f1fc18269cc392b4c1a25683 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Thu, 27 Nov 2014 16:01:25 -0800 Subject: [PATCH 80/90] doc(bitswap) fix duplicaduplication @whyrusleeping https://github.com/jbenet/go-ipfs/commit/ada571425bc688b459cd34810fd398e5547b48a0#commitcomment-8753622 License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 94c4cde88..00b08a323 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -95,9 +95,8 @@ func (bs *bitswap) GetBlock(parent context.Context, k u.Key) (*blocks.Block, err // returns. To ensure this, derive a new context. Note that it is okay to // listen on parent in this scope, but NOT okay to pass |parent| to // functions called by this one. Otherwise those functions won't return - // when this context Otherwise those functions won't return when this - // context's cancel func is executed. This is difficult to enforce. May - // this comment keep you safe. + // when this context's cancel func is executed. This is difficult to + // enforce. May this comment keep you safe. ctx, cancelFunc := context.WithCancel(parent) From 3a6b6c697482fc5160584342a28f3c5d80c7d075 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 1 Dec 2014 02:15:04 +0000 Subject: [PATCH 81/90] cleanup, use a workgroup over channels --- exchange/bitswap/bitswap.go | 11 ++++++----- merkledag/merkledag.go | 9 ++++++--- unixfs/io/dagreader.go | 3 ++- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 00b08a323..debfd5f69 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -3,6 +3,7 @@ package bitswap import ( + "sync" "time" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" @@ -180,8 +181,9 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { - done := make(chan struct{}) + wg := sync.WaitGroup{} for _, k := range ks { + wg.Add(1) go func(k u.Key) { providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) @@ -189,12 +191,10 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { if err != nil { log.Errorf("error sending wantlist: %s", err) } - done <- struct{}{} + wg.Done() }(k) } - for _ = range ks { - <-done - } + wg.Wait() } // TODO ensure only one active request per key @@ -255,6 +255,7 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { return bs.routing.Provide(ctx, blk.Key()) } +// receiveBlock handles storing the block in the blockstore and calling HasBlock func (bs *bitswap) receiveBlock(ctx context.Context, block *blocks.Block) { // TODO verify blocks? if err := bs.blockstore.Put(block); err != nil { diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 7dadf722d..fbb07c9ee 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -28,7 +28,10 @@ type DAGService interface { AddRecursive(*Node) error Get(u.Key) (*Node, error) Remove(*Node) error - GetKeysAsync(context.Context, *Node) <-chan *Node + + // GetDAG returns, in order, all the single leve child + // nodes of the passed in node. + GetDAG(context.Context, *Node) <-chan *Node } func NewDAGService(bs *bserv.BlockService) DAGService { @@ -298,10 +301,10 @@ func FindLink(n *Node, k u.Key, found []*Node) (int, error) { return -1, u.ErrNotFound } -// GetKeysAsync will fill out all of the links of the given Node. +// GetDAG will fill out all of the links of the given Node. // It returns a channel of nodes, which the caller can receive // all the child nodes of 'root' on, in proper order. -func (ds *dagService) GetKeysAsync(ctx context.Context, root *Node) <-chan *Node { +func (ds *dagService) GetDAG(ctx context.Context, root *Node) <-chan *Node { sig := make(chan *Node) go func() { var keys []u.Key diff --git a/unixfs/io/dagreader.go b/unixfs/io/dagreader.go index b41ac3daa..f4290dd4b 100644 --- a/unixfs/io/dagreader.go +++ b/unixfs/io/dagreader.go @@ -40,7 +40,7 @@ func NewDagReader(n *mdag.Node, serv mdag.DAGService) (io.Reader, error) { case ftpb.Data_File: var fetchChan <-chan *mdag.Node if serv != nil { - fetchChan = serv.GetKeysAsync(context.TODO(), n) + fetchChan = serv.GetDAG(context.TODO(), n) } return &DagReader{ node: n, @@ -62,6 +62,7 @@ func (dr *DagReader) precalcNextBuf() error { var nxt *mdag.Node var ok bool + // TODO: require non-nil dagservice, use offline bitswap exchange if dr.serv == nil { // Only used when fetchChan is nil, // which only happens when passed in a nil dagservice From c2b497e3157222187a4f65b4fec03ce6f26bf2c4 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 1 Dec 2014 21:38:16 +0000 Subject: [PATCH 82/90] switch over to using sendMessage vs sendRequest --- exchange/bitswap/bitswap.go | 10 +++------- exchange/bitswap/network/ipfs_impl.go | 17 ++--------------- routing/dht/routing.go | 1 + 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index debfd5f69..44d51cde2 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -151,6 +151,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e message.AddWanted(wanted) } for peerToQuery := range peers { + log.Debug("sending query to: %s", peerToQuery) log.Event(ctx, "PeerToQuery", peerToQuery) go func(p peer.Peer) { @@ -161,20 +162,15 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e return } - response, err := bs.sender.SendRequest(ctx, p, message) + err = bs.sender.SendMessage(ctx, p, message) if err != nil { - log.Errorf("Error sender.SendRequest(%s) = %s", p, err) + log.Errorf("Error sender.SendMessage(%s) = %s", p, err) return } // FIXME ensure accounting is handled correctly when // communication fails. May require slightly different API to // get better guarantees. May need shared sequence numbers. bs.strategy.MessageSent(p, message) - - if response == nil { - return - } - bs.ReceiveMessage(ctx, p, response) }(peerToQuery) } return nil diff --git a/exchange/bitswap/network/ipfs_impl.go b/exchange/bitswap/network/ipfs_impl.go index 1a3c11b44..f356285ef 100644 --- a/exchange/bitswap/network/ipfs_impl.go +++ b/exchange/bitswap/network/ipfs_impl.go @@ -48,21 +48,8 @@ func (bsnet *impl) HandleMessage( return nil } - p, bsmsg := bsnet.receiver.ReceiveMessage(ctx, incoming.Peer(), received) - - // TODO(brian): put this in a helper function - if bsmsg == nil || p == nil { - return nil - } - - outgoing, err := bsmsg.ToNet(p) - if err != nil { - go bsnet.receiver.ReceiveError(err) - return nil - } - - log.Debugf("Message size: %d", len(outgoing.Data())) - return outgoing + bsnet.receiver.ReceiveMessage(ctx, incoming.Peer(), received) + return nil } func (bsnet *impl) DialPeer(ctx context.Context, p peer.Peer) error { diff --git a/routing/dht/routing.go b/routing/dht/routing.go index fedf281d3..b1644d116 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -126,6 +126,7 @@ func (dht *IpfsDHT) Provide(ctx context.Context, key u.Key) error { } func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int) <-chan peer.Peer { + log.Debug("Find Providers: %s", key) peerOut := make(chan peer.Peer, count) go func() { ps := newPeerSet() From 514b26e2b87c9cdc3506e1d8c817d8e6bb57c50d Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 1 Dec 2014 22:14:57 +0000 Subject: [PATCH 83/90] remove sigquit from handled signals --- cmd/ipfs/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ipfs/main.go b/cmd/ipfs/main.go index 20278ab31..8212cc63b 100644 --- a/cmd/ipfs/main.go +++ b/cmd/ipfs/main.go @@ -510,6 +510,6 @@ func (i *cmdInvocation) setupInterruptHandler() { func allInterruptSignals() chan os.Signal { sigc := make(chan os.Signal, 1) signal.Notify(sigc, syscall.SIGHUP, syscall.SIGINT, - syscall.SIGTERM, syscall.SIGQUIT) + syscall.SIGTERM) return sigc } From e8536db3510a1554f5ed7c9704e1a937bc923cb9 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 2 Dec 2014 07:34:39 +0000 Subject: [PATCH 84/90] make bitswap sub-RPC's timeout (slowly for now) --- cmd/ipfs/main.go | 2 ++ exchange/bitswap/bitswap.go | 26 ++++++++++++++++++++++---- routing/dht/routing.go | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/cmd/ipfs/main.go b/cmd/ipfs/main.go index 8212cc63b..18923a64a 100644 --- a/cmd/ipfs/main.go +++ b/cmd/ipfs/main.go @@ -6,6 +6,7 @@ import ( "io" "os" "os/signal" + "runtime" "runtime/pprof" "syscall" @@ -54,6 +55,7 @@ type cmdInvocation struct { // - output the response // - if anything fails, print error, maybe with help func main() { + runtime.GOMAXPROCS(3) ctx := context.Background() var err error var invoc cmdInvocation diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 44d51cde2..ac9224228 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -26,6 +26,9 @@ var log = eventlog.Logger("bitswap") // TODO: if a 'non-nice' strategy is implemented, consider increasing this value const maxProvidersPerRequest = 3 +const providerRequestTimeout = time.Second * 10 +const hasBlockTimeout = time.Second * 15 + // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. @@ -181,7 +184,8 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { for _, k := range ks { wg.Add(1) go func(k u.Key) { - providers := bs.routing.FindProvidersAsync(ctx, k, maxProvidersPerRequest) + child, _ := context.WithTimeout(ctx, providerRequestTimeout) + providers := bs.routing.FindProvidersAsync(child, k, maxProvidersPerRequest) err := bs.sendWantListTo(ctx, providers) if err != nil { @@ -228,7 +232,8 @@ func (bs *bitswap) loop(parent context.Context) { // pinning a file, you store and provide all blocks associated with // it. Later, this assumption may not hold as true if we implement // newer bitswap strategies. - providers := bs.routing.FindProvidersAsync(ctx, ks[0], maxProvidersPerRequest) + child, _ := context.WithTimeout(ctx, providerRequestTimeout) + providers := bs.routing.FindProvidersAsync(child, ks[0], maxProvidersPerRequest) err := bs.sendWantListTo(ctx, providers) if err != nil { @@ -247,8 +252,21 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { log.Debugf("Has Block %s", blk.Key()) bs.wantlist.Remove(blk.Key()) bs.notifications.Publish(blk) - bs.sendToPeersThatWant(ctx, blk) - return bs.routing.Provide(ctx, blk.Key()) + + var err error + wg := &sync.WaitGroup{} + wg.Add(2) + child, _ := context.WithTimeout(ctx, hasBlockTimeout) + go func() { + bs.sendToPeersThatWant(child, blk) + wg.Done() + }() + go func() { + err = bs.routing.Provide(child, blk.Key()) + wg.Done() + }() + wg.Wait() + return err } // receiveBlock handles storing the block in the blockstore and calling HasBlock diff --git a/routing/dht/routing.go b/routing/dht/routing.go index b1644d116..f504b9bb4 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -126,7 +126,7 @@ func (dht *IpfsDHT) Provide(ctx context.Context, key u.Key) error { } func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int) <-chan peer.Peer { - log.Debug("Find Providers: %s", key) + log.Debugf("Find Providers: %s", key) peerOut := make(chan peer.Peer, count) go func() { ps := newPeerSet() From 432eee651fc82ff5e9161254410662542e8e1659 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 2 Dec 2014 08:03:00 +0000 Subject: [PATCH 85/90] remove unnecessary concurrency in last commit --- exchange/bitswap/bitswap.go | 16 +++------------- exchange/bitswap/bitswap_test.go | 2 +- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index ac9224228..e00b23f91 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -253,20 +253,10 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { bs.wantlist.Remove(blk.Key()) bs.notifications.Publish(blk) - var err error - wg := &sync.WaitGroup{} - wg.Add(2) child, _ := context.WithTimeout(ctx, hasBlockTimeout) - go func() { - bs.sendToPeersThatWant(child, blk) - wg.Done() - }() - go func() { - err = bs.routing.Provide(child, blk.Key()) - wg.Done() - }() - wg.Wait() - return err + bs.sendToPeersThatWant(child, blk) + child, _ = context.WithTimeout(ctx, hasBlockTimeout) + return bs.routing.Provide(child, blk.Key()) } // receiveBlock handles storing the block in the blockstore and calling HasBlock diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index ede87c474..d26a8ffc9 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -235,7 +235,7 @@ func TestSendToWantingPeer(t *testing.T) { t.Logf("%v should now have %v\n", w.Peer, alpha.Key()) block, err := w.Blockstore.Get(alpha.Key()) if err != nil { - t.Fatal("Should not have received an error") + t.Fatalf("Should not have received an error: %s", err) } if block.Key() != alpha.Key() { t.Fatal("Expected to receive alpha from me") From 655216374098dec7781899d3e0dbc9cba8035a1d Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 3 Dec 2014 19:46:01 +0000 Subject: [PATCH 86/90] add readme for bitswap --- exchange/bitswap/README.md | 24 ++++++++++++++++++++++++ routing/dht/routing.go | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 exchange/bitswap/README.md diff --git a/exchange/bitswap/README.md b/exchange/bitswap/README.md new file mode 100644 index 000000000..86b29e090 --- /dev/null +++ b/exchange/bitswap/README.md @@ -0,0 +1,24 @@ +#Welcome to Bitswap + +Bitswap is the module that is responsible for requesting blocks over the +network from other ipfs peers. + +##Main Operations +Bitswap has three main operations: + +###GetBlocks +`GetBlocks` is a bitswap method used to request multiple blocks that are likely to all be provided by the same peer (part of a single file, for example). + +###GetBlock +`GetBlock` is a special case of `GetBlocks` that just requests a single block. + +###HasBlock +`HasBlock` registers a local block with bitswap. Bitswap will then send that block to any connected peers who want it (strategy allowing), and announce to the DHT that the block is being provided. + +##Internal Details +All `GetBlock` requests are relayed into a single for-select loop via channels. Calls to `GetBlocks` will have `FindProviders` called for only the first key in the set initially, This is an optimization attempting to cut down on the number of RPCs required. After a timeout (specified by the strategies `GetRebroadcastDelay`) Bitswap will iterate through all keys still in the local wantlist, perform a find providers call for each, and sent the wantlist out to those providers. This is the fallback behaviour for cases where our initial assumption about one peer potentially having multiple blocks in a set does not hold true. + +When receiving messages, Bitswaps `ReceiveMessage` method is called. A bitswap message may contain the wantlist of the peer who sent the message, and an array of blocks that were on our local wantlist. Any blocks we receive in a bitswap message will be passed to `HasBlock`, and the other peers wantlist gets updated in the strategy by `bs.strategy.MessageReceived`. + +##Outstanding TODOs: +- Ensure only one request active per key diff --git a/routing/dht/routing.go b/routing/dht/routing.go index f504b9bb4..102acd5a4 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -126,7 +126,7 @@ func (dht *IpfsDHT) Provide(ctx context.Context, key u.Key) error { } func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int) <-chan peer.Peer { - log.Debugf("Find Providers: %s", key) + log.Event(ctx, "findProviders", key) peerOut := make(chan peer.Peer, count) go func() { ps := newPeerSet() From 260ace7f8134365b158fe000da45f4b380709c2c Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 3 Dec 2014 19:51:17 +0000 Subject: [PATCH 87/90] util keys need to be pointers for loggable --- routing/dht/routing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routing/dht/routing.go b/routing/dht/routing.go index 102acd5a4..f0bfbe485 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -126,7 +126,7 @@ func (dht *IpfsDHT) Provide(ctx context.Context, key u.Key) error { } func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int) <-chan peer.Peer { - log.Event(ctx, "findProviders", key) + log.Event(ctx, "findProviders", &key) peerOut := make(chan peer.Peer, count) go func() { ps := newPeerSet() From f054be9e1fa5befaf9251e5e0996731aa845b064 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 3 Dec 2014 23:48:38 +0000 Subject: [PATCH 88/90] update bitswap readme --- exchange/bitswap/README.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/README.md b/exchange/bitswap/README.md index 86b29e090..5f55c6ee3 100644 --- a/exchange/bitswap/README.md +++ b/exchange/bitswap/README.md @@ -7,18 +7,36 @@ network from other ipfs peers. Bitswap has three main operations: ###GetBlocks -`GetBlocks` is a bitswap method used to request multiple blocks that are likely to all be provided by the same peer (part of a single file, for example). +`GetBlocks` is a bitswap method used to request multiple blocks that are likely +to all be provided by the same peer (part of a single file, for example). ###GetBlock `GetBlock` is a special case of `GetBlocks` that just requests a single block. ###HasBlock -`HasBlock` registers a local block with bitswap. Bitswap will then send that block to any connected peers who want it (strategy allowing), and announce to the DHT that the block is being provided. +`HasBlock` registers a local block with bitswap. Bitswap will then send that +block to any connected peers who want it (strategy allowing), and announce to +the DHT that the block is being provided. ##Internal Details -All `GetBlock` requests are relayed into a single for-select loop via channels. Calls to `GetBlocks` will have `FindProviders` called for only the first key in the set initially, This is an optimization attempting to cut down on the number of RPCs required. After a timeout (specified by the strategies `GetRebroadcastDelay`) Bitswap will iterate through all keys still in the local wantlist, perform a find providers call for each, and sent the wantlist out to those providers. This is the fallback behaviour for cases where our initial assumption about one peer potentially having multiple blocks in a set does not hold true. +All `GetBlock` requests are relayed into a single for-select loop via channels. +Calls to `GetBlocks` will have `FindProviders` called for only the first key in +the set initially, This is an optimization attempting to cut down on the number +of RPCs required. After a timeout (specified by the strategies +`GetRebroadcastDelay`) Bitswap will iterate through all keys still in the local +wantlist, perform a find providers call for each, and sent the wantlist out to +those providers. This is the fallback behaviour for cases where our initial +assumption about one peer potentially having multiple blocks in a set does not +hold true. -When receiving messages, Bitswaps `ReceiveMessage` method is called. A bitswap message may contain the wantlist of the peer who sent the message, and an array of blocks that were on our local wantlist. Any blocks we receive in a bitswap message will be passed to `HasBlock`, and the other peers wantlist gets updated in the strategy by `bs.strategy.MessageReceived`. +When receiving messages, Bitswaps `ReceiveMessage` method is called. A bitswap +message may contain the wantlist of the peer who sent the message, and an array +of blocks that were on our local wantlist. Any blocks we receive in a bitswap +message will be passed to `HasBlock`, and the other peers wantlist gets updated +in the strategy by `bs.strategy.MessageReceived`. +If another peers wantlist is received, Bitswap will call its strategies +`ShouldSendBlockToPeer` method to determine whether or not the other peer will +be sent the block they are requesting (if we even have it). ##Outstanding TODOs: - Ensure only one request active per key From afa28dc67234dcad6e5328219023273e8292d51f Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 4 Dec 2014 21:38:40 +0000 Subject: [PATCH 89/90] update bitswap readme --- exchange/bitswap/README.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/exchange/bitswap/README.md b/exchange/bitswap/README.md index 5f55c6ee3..991d17213 100644 --- a/exchange/bitswap/README.md +++ b/exchange/bitswap/README.md @@ -1,22 +1,24 @@ -#Welcome to Bitswap +#Welcome to Bitswap (The data trading engine) -Bitswap is the module that is responsible for requesting blocks over the -network from other ipfs peers. +Bitswap is the module that is responsible for requesting and providing data +blocks over the network to and from other ipfs peers. The role of bitswap is +to be a merchant in the large global marketplace of data. ##Main Operations -Bitswap has three main operations: +Bitswap has three high level operations: ###GetBlocks `GetBlocks` is a bitswap method used to request multiple blocks that are likely -to all be provided by the same peer (part of a single file, for example). +to all be provided by the same set of peers (part of a single file, for example). ###GetBlock `GetBlock` is a special case of `GetBlocks` that just requests a single block. ###HasBlock `HasBlock` registers a local block with bitswap. Bitswap will then send that -block to any connected peers who want it (strategy allowing), and announce to -the DHT that the block is being provided. +block to any connected peers who want it (with the strategies approval), record +that transaction in the ledger and announce to the DHT that the block is being +provided. ##Internal Details All `GetBlock` requests are relayed into a single for-select loop via channels. @@ -39,4 +41,6 @@ If another peers wantlist is received, Bitswap will call its strategies be sent the block they are requesting (if we even have it). ##Outstanding TODOs: -- Ensure only one request active per key +[] Ensure only one request active per key +[] More involved strategies +[] Ensure only wanted blocks are counted in ledgers From 260ac9617585e47fde73835729ea5e9716c3d16f Mon Sep 17 00:00:00 2001 From: Jeromy Johnson Date: Thu, 4 Dec 2014 21:48:11 +0000 Subject: [PATCH 90/90] Update README.md --- exchange/bitswap/README.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/exchange/bitswap/README.md b/exchange/bitswap/README.md index 991d17213..bfa0aaa86 100644 --- a/exchange/bitswap/README.md +++ b/exchange/bitswap/README.md @@ -1,4 +1,5 @@ -#Welcome to Bitswap (The data trading engine) +#Welcome to Bitswap +###(The data trading engine) Bitswap is the module that is responsible for requesting and providing data blocks over the network to and from other ipfs peers. The role of bitswap is @@ -7,15 +8,15 @@ to be a merchant in the large global marketplace of data. ##Main Operations Bitswap has three high level operations: -###GetBlocks -`GetBlocks` is a bitswap method used to request multiple blocks that are likely +- **GetBlocks** + - `GetBlocks` is a bitswap method used to request multiple blocks that are likely to all be provided by the same set of peers (part of a single file, for example). -###GetBlock -`GetBlock` is a special case of `GetBlocks` that just requests a single block. +- **GetBlock** + - `GetBlock` is a special case of `GetBlocks` that just requests a single block. -###HasBlock -`HasBlock` registers a local block with bitswap. Bitswap will then send that +- **HasBlock** + - `HasBlock` registers a local block with bitswap. Bitswap will then send that block to any connected peers who want it (with the strategies approval), record that transaction in the ledger and announce to the DHT that the block is being provided. @@ -41,6 +42,6 @@ If another peers wantlist is received, Bitswap will call its strategies be sent the block they are requesting (if we even have it). ##Outstanding TODOs: -[] Ensure only one request active per key -[] More involved strategies -[] Ensure only wanted blocks are counted in ledgers +- [ ] Ensure only one request active per key +- [ ] More involved strategies +- [ ] Ensure only wanted blocks are counted in ledgers