From c4a935c3e3ffcf3ecfe69444dbd2bda9add9d01f Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Wed, 17 Dec 2014 23:43:58 -0800 Subject: [PATCH 01/64] go complains about lack of buildable file --- net/backpressure/backpressure.go | 1 + 1 file changed, 1 insertion(+) create mode 100644 net/backpressure/backpressure.go diff --git a/net/backpressure/backpressure.go b/net/backpressure/backpressure.go new file mode 100644 index 000000000..19950a728 --- /dev/null +++ b/net/backpressure/backpressure.go @@ -0,0 +1 @@ +package backpressure_tests From 12b296ee1a494fb2d74db7ceebb513e5b4479d39 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 02:00:37 +0000 Subject: [PATCH 02/64] create wantlist object --- exchange/bitswap/wantlist/wantlist.go | 56 +++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 exchange/bitswap/wantlist/wantlist.go diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go new file mode 100644 index 000000000..041064901 --- /dev/null +++ b/exchange/bitswap/wantlist/wantlist.go @@ -0,0 +1,56 @@ +package wantlist + +import ( + u "github.com/jbenet/go-ipfs/util" + "sort" +) + +type Wantlist struct { + set map[u.Key]*Entry +} + +func NewWantlist() *Wantlist { + return &Wantlist{ + set: make(map[u.Key]*Entry), + } +} + +type Entry struct { + Value u.Key + Priority int +} + +func (w *Wantlist) Add(k u.Key, priority int) { + if _, ok := w.set[k]; ok { + return + } + w.set[k] = &Entry{ + Value: k, + Priority: priority, + } +} + +func (w *Wantlist) Remove(k u.Key) { + delete(w.set, k) +} + +func (w *Wantlist) Contains(k u.Key) bool { + _, ok := w.set[k] + return ok +} + +type entrySlice []*Entry + +func (es entrySlice) Len() int { return len(es) } +func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } +func (es entrySlice) Less(i, j int) bool { return es[i].Priority < es[j].Priority } + +func (w *Wantlist) Entries() []*Entry { + var es entrySlice + + for _, e := range w.set { + es = append(es, e) + } + sort.Sort(es) + return es +} From 5b6a5e807f43775ec18dbcc7fa3eafaf71c88678 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 02:03:20 +0000 Subject: [PATCH 03/64] implement bitswap roundWorker make vendor --- Godeps/Godeps.json | 2 +- exchange/bitswap/bitswap.go | 104 ++++++++++++------ exchange/bitswap/bitswap_test.go | 65 +++++++++-- .../bitswap/message/internal/pb/message.pb.go | 64 ++++++++++- .../bitswap/message/internal/pb/message.proto | 17 ++- exchange/bitswap/message/message.go | 92 ++++++++++------ exchange/bitswap/message/message_test.go | 59 ++++++---- exchange/bitswap/strategy/interface.go | 3 + exchange/bitswap/strategy/ledger.go | 16 ++- exchange/bitswap/strategy/strategy.go | 70 +++++++++++- exchange/bitswap/strategy/strategy_test.go | 2 +- 11 files changed, 379 insertions(+), 115 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 38b0ec272..84f4b4c60 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/jbenet/go-ipfs", - "GoVersion": "go1.3", + "GoVersion": "devel +ffe33f1f1f17 Tue Nov 25 15:41:33 2014 +1100", "Packages": [ "./..." ], diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 64f293528..1e0e86b61 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -15,6 +15,7 @@ import ( bsnet "github.com/jbenet/go-ipfs/exchange/bitswap/network" notifications "github.com/jbenet/go-ipfs/exchange/bitswap/notifications" strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" + wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" eventlog "github.com/jbenet/go-ipfs/util/eventlog" @@ -29,6 +30,8 @@ const maxProvidersPerRequest = 3 const providerRequestTimeout = time.Second * 10 const hasBlockTimeout = time.Second * 15 +const roundTime = time.Second / 2 + // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. @@ -41,6 +44,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout notif := notifications.New() go func() { <-ctx.Done() + cancelFunc() notif.Shutdown() }() @@ -51,11 +55,12 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout strategy: strategy.New(nice), routing: routing, sender: network, - wantlist: u.NewKeySet(), + wantlist: wl.NewWantlist(), batchRequests: make(chan []u.Key, 32), } network.SetDelegate(bs) go bs.loop(ctx) + go bs.roundWorker(ctx) return bs } @@ -85,7 +90,7 @@ type bitswap struct { // TODO(brian): save the strategy's state to the datastore strategy strategy.Strategy - wantlist u.KeySet + wantlist *wl.Wantlist // cancelFunc signals cancellation to the bitswap event loop cancelFunc func() @@ -166,8 +171,8 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e panic("Cant send wantlist to nil peerchan") } message := bsmsg.New() - for _, wanted := range bs.wantlist.Keys() { - message.AddWanted(wanted) + for _, wanted := range bs.wantlist.Entries() { + message.AddEntry(wanted.Value, wanted.Priority, false) } for peerToQuery := range peers { log.Debug("sending query to: %s", peerToQuery) @@ -195,9 +200,9 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e return nil } -func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { +func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wantlist) { wg := sync.WaitGroup{} - for _, k := range ks { + for _, e := range wantlist.Entries() { wg.Add(1) go func(k u.Key) { child, _ := context.WithTimeout(ctx, providerRequestTimeout) @@ -208,11 +213,44 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, ks []u.Key) { log.Errorf("error sending wantlist: %s", err) } wg.Done() - }(k) + }(e.Value) } wg.Wait() } +func (bs *bitswap) roundWorker(ctx context.Context) { + roundTicker := time.NewTicker(roundTime) + bandwidthPerRound := 500000 + for { + select { + case <-ctx.Done(): + return + case <-roundTicker.C: + alloc, err := bs.strategy.GetAllocation(bandwidthPerRound, bs.blockstore) + if err != nil { + log.Critical("%s", err) + } + //log.Errorf("Allocation: %v", alloc) + bs.processStrategyAllocation(ctx, alloc) + } + } +} + +func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strategy.Task) { + for _, t := range alloc { + for _, block := range t.Blocks { + message := bsmsg.New() + message.AddBlock(block) + for _, wanted := range bs.wantlist.Entries() { + message.AddEntry(wanted.Value, wanted.Priority, false) + } + if err := bs.send(ctx, t.Peer, message); err != nil { + log.Errorf("Message Send Failed: %s", err) + } + } + } +} + // TODO ensure only one active request per key func (bs *bitswap) loop(parent context.Context) { @@ -228,7 +266,7 @@ func (bs *bitswap) loop(parent context.Context) { select { case <-broadcastSignal.C: // Resend unfulfilled wantlist keys - bs.sendWantlistToProviders(ctx, bs.wantlist.Keys()) + bs.sendWantlistToProviders(ctx, bs.wantlist) 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 @@ -239,7 +277,7 @@ func (bs *bitswap) loop(parent context.Context) { continue } for _, k := range ks { - bs.wantlist.Add(k) + bs.wantlist.Add(k, 1) } // 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 @@ -277,45 +315,41 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm return nil, nil } - // Record message bytes in ledger - // TODO: this is bad, and could be easily abused. - // Should only track *useful* messages in ledger // This call records changes to wantlists, blocks received, // and number of bytes transfered. bs.strategy.MessageReceived(p, incoming) + // TODO: this is bad, and could be easily abused. + // Should only track *useful* messages in ledger + var blkeys []u.Key for _, block := range incoming.Blocks() { + blkeys = append(blkeys, block.Key()) if err := bs.HasBlock(ctx, block); err != nil { log.Error(err) } } - - for _, key := range incoming.Wantlist() { - if bs.strategy.ShouldSendBlockToPeer(key, p) { - if block, errBlockNotFound := bs.blockstore.Get(key); errBlockNotFound != nil { - continue - } else { - // Create a separate message to send this block in - 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) - } - - blkmsg.AddBlock(block) - bs.send(ctx, p, blkmsg) - bs.strategy.BlockSentToPeer(block.Key(), p) - } - } + if len(blkeys) > 0 { + bs.cancelBlocks(ctx, blkeys) } // TODO: consider changing this function to not return anything return nil, nil } +func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { + message := bsmsg.New() + message.SetFull(false) + for _, k := range bkeys { + message.AddEntry(k, 0, true) + } + for _, p := range bs.strategy.Peers() { + err := bs.send(ctx, p, message) + if err != nil { + log.Errorf("Error sending message: %s", err) + } + } +} + func (bs *bitswap) ReceiveError(err error) { log.Errorf("Bitswap ReceiveError: %s", err) // TODO log the network error @@ -337,8 +371,8 @@ func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block *blocks.Block) if bs.strategy.ShouldSendBlockToPeer(block.Key(), p) { message := bsmsg.New() message.AddBlock(block) - for _, wanted := range bs.wantlist.Keys() { - message.AddWanted(wanted) + for _, wanted := range bs.wantlist.Entries() { + message.AddEntry(wanted.Value, wanted.Priority, false) } if err := bs.send(ctx, p, message); err != nil { return err diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index d58ff596a..0e72883cc 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -11,6 +11,7 @@ import ( blocksutil "github.com/jbenet/go-ipfs/blocks/blocksutil" tn "github.com/jbenet/go-ipfs/exchange/bitswap/testnet" mockrouting "github.com/jbenet/go-ipfs/routing/mock" + u "github.com/jbenet/go-ipfs/util" delay "github.com/jbenet/go-ipfs/util/delay" testutil "github.com/jbenet/go-ipfs/util/testutil" ) @@ -25,6 +26,7 @@ func TestClose(t *testing.T) { vnet := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rout := mockrouting.NewServer() sesgen := NewSessionGenerator(vnet, rout) + defer sesgen.Stop() bgen := blocksutil.NewBlockGenerator() block := bgen.Next() @@ -39,6 +41,7 @@ func TestGetBlockTimeout(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() g := NewSessionGenerator(net, rs) + defer g.Stop() self := g.Next() @@ -56,11 +59,13 @@ func TestProviderForKeyButNetworkCannotFind(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() g := NewSessionGenerator(net, rs) + defer g.Stop() block := blocks.NewBlock([]byte("block")) rs.Client(testutil.NewPeerWithIDString("testing")).Provide(context.Background(), block.Key()) // but not on network solo := g.Next() + defer solo.Exchange.Close() ctx, _ := context.WithTimeout(context.Background(), time.Nanosecond) _, err := solo.Exchange.GetBlock(ctx, block.Key()) @@ -78,8 +83,10 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { rs := mockrouting.NewServer() block := blocks.NewBlock([]byte("block")) g := NewSessionGenerator(net, rs) + defer g.Stop() hasBlock := g.Next() + defer hasBlock.Exchange.Close() if err := hasBlock.Blockstore().Put(block); err != nil { t.Fatal(err) @@ -89,6 +96,7 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { } wantsBlock := g.Next() + defer wantsBlock.Exchange.Close() ctx, _ := context.WithTimeout(context.Background(), time.Second) received, err := wantsBlock.Exchange.GetBlock(ctx, block.Key()) @@ -107,7 +115,7 @@ func TestLargeSwarm(t *testing.T) { t.SkipNow() } t.Parallel() - numInstances := 5 + numInstances := 500 numBlocks := 2 PerformDistributionTest(t, numInstances, numBlocks) } @@ -129,6 +137,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() sg := NewSessionGenerator(net, rs) + defer sg.Stop() bg := blocksutil.NewBlockGenerator() t.Log("Test a few nodes trying to get one file with a lot of blocks") @@ -138,24 +147,29 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { t.Log("Give the blocks to the first instance") + var blkeys []u.Key first := instances[0] for _, b := range blocks { first.Blockstore().Put(b) + blkeys = append(blkeys, b.Key()) first.Exchange.HasBlock(context.Background(), b) rs.Client(first.Peer).Provide(context.Background(), b.Key()) } t.Log("Distribute!") - var wg sync.WaitGroup - + 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.Add(1) + go func(inst Instance) { + defer wg.Done() + outch, err := inst.Exchange.GetBlocks(context.TODO(), blkeys) + if err != nil { + t.Fatal(err) + } + for _ = range outch { + } + }(inst) } wg.Wait() @@ -189,6 +203,7 @@ func TestSendToWantingPeer(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() sg := NewSessionGenerator(net, rs) + defer sg.Stop() bg := blocksutil.NewBlockGenerator() me := sg.Next() @@ -201,7 +216,7 @@ func TestSendToWantingPeer(t *testing.T) { alpha := bg.Next() - const timeout = 100 * time.Millisecond // FIXME don't depend on time + const timeout = 1000 * 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) @@ -246,3 +261,33 @@ func TestSendToWantingPeer(t *testing.T) { t.Fatal("Expected to receive alpha from me") } } + +func TestBasicBitswap(t *testing.T) { + net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) + rs := mockrouting.NewServer() + sg := NewSessionGenerator(net, rs) + bg := blocksutil.NewBlockGenerator() + + t.Log("Test a few nodes trying to get one file with a lot of blocks") + + instances := sg.Instances(2) + blocks := bg.Blocks(1) + err := instances[0].Exchange.HasBlock(context.TODO(), blocks[0]) + if err != nil { + t.Fatal(err) + } + + ctx, _ := context.WithTimeout(context.TODO(), time.Second*5) + blk, err := instances[1].Exchange.GetBlock(ctx, blocks[0].Key()) + if err != nil { + t.Fatal(err) + } + + t.Log(blk) + for _, inst := range instances { + err := inst.Exchange.Close() + if err != nil { + t.Fatal(err) + } + } +} diff --git a/exchange/bitswap/message/internal/pb/message.pb.go b/exchange/bitswap/message/internal/pb/message.pb.go index f6f8a9bbc..4ddfc56f7 100644 --- a/exchange/bitswap/message/internal/pb/message.pb.go +++ b/exchange/bitswap/message/internal/pb/message.pb.go @@ -21,16 +21,16 @@ var _ = proto.Marshal var _ = math.Inf type Message struct { - Wantlist []string `protobuf:"bytes,1,rep,name=wantlist" json:"wantlist,omitempty"` - Blocks [][]byte `protobuf:"bytes,2,rep,name=blocks" json:"blocks,omitempty"` - XXX_unrecognized []byte `json:"-"` + Wantlist *Message_Wantlist `protobuf:"bytes,1,opt,name=wantlist" json:"wantlist,omitempty"` + Blocks [][]byte `protobuf:"bytes,2,rep,name=blocks" json:"blocks,omitempty"` + XXX_unrecognized []byte `json:"-"` } func (m *Message) Reset() { *m = Message{} } func (m *Message) String() string { return proto.CompactTextString(m) } func (*Message) ProtoMessage() {} -func (m *Message) GetWantlist() []string { +func (m *Message) GetWantlist() *Message_Wantlist { if m != nil { return m.Wantlist } @@ -44,5 +44,61 @@ func (m *Message) GetBlocks() [][]byte { return nil } +type Message_Wantlist struct { + Entries []*Message_Wantlist_Entry `protobuf:"bytes,1,rep,name=entries" json:"entries,omitempty"` + Full *bool `protobuf:"varint,2,opt,name=full" json:"full,omitempty"` + XXX_unrecognized []byte `json:"-"` +} + +func (m *Message_Wantlist) Reset() { *m = Message_Wantlist{} } +func (m *Message_Wantlist) String() string { return proto.CompactTextString(m) } +func (*Message_Wantlist) ProtoMessage() {} + +func (m *Message_Wantlist) GetEntries() []*Message_Wantlist_Entry { + if m != nil { + return m.Entries + } + return nil +} + +func (m *Message_Wantlist) GetFull() bool { + if m != nil && m.Full != nil { + return *m.Full + } + return false +} + +type Message_Wantlist_Entry struct { + Block *string `protobuf:"bytes,1,opt,name=block" json:"block,omitempty"` + Priority *int32 `protobuf:"varint,2,opt,name=priority" json:"priority,omitempty"` + Cancel *bool `protobuf:"varint,3,opt,name=cancel" json:"cancel,omitempty"` + XXX_unrecognized []byte `json:"-"` +} + +func (m *Message_Wantlist_Entry) Reset() { *m = Message_Wantlist_Entry{} } +func (m *Message_Wantlist_Entry) String() string { return proto.CompactTextString(m) } +func (*Message_Wantlist_Entry) ProtoMessage() {} + +func (m *Message_Wantlist_Entry) GetBlock() string { + if m != nil && m.Block != nil { + return *m.Block + } + return "" +} + +func (m *Message_Wantlist_Entry) GetPriority() int32 { + if m != nil && m.Priority != nil { + return *m.Priority + } + return 0 +} + +func (m *Message_Wantlist_Entry) GetCancel() bool { + if m != nil && m.Cancel != nil { + return *m.Cancel + } + return false +} + func init() { } diff --git a/exchange/bitswap/message/internal/pb/message.proto b/exchange/bitswap/message/internal/pb/message.proto index a8c6c7252..7c44f3a6b 100644 --- a/exchange/bitswap/message/internal/pb/message.proto +++ b/exchange/bitswap/message/internal/pb/message.proto @@ -1,6 +1,19 @@ package bitswap.message.pb; message Message { - repeated string wantlist = 1; - repeated bytes blocks = 2; + + message Wantlist { + + message Entry { + optional string block = 1; // the block key + optional int32 priority = 2; // the priority (normalized). default to 1 + optional bool cancel = 3; // whether this revokes an entry + } + + repeated Entry entries = 1; // a list of wantlist entries + optional bool full = 2; // whether this is the full wantlist. default to false + } + + optional Wantlist wantlist = 1; + repeated bytes blocks = 2; } diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index 62a39be91..288fc9da7 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -9,6 +9,7 @@ import ( u "github.com/jbenet/go-ipfs/util" ggio "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/io" + proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" ) // TODO move message.go into the bitswap package @@ -17,21 +18,21 @@ import ( type BitSwapMessage interface { // Wantlist returns a slice of unique keys that represent data wanted by // the sender. - Wantlist() []u.Key + Wantlist() []*Entry // Blocks returns a slice of unique blocks Blocks() []*blocks.Block - // AddWanted adds the key to the Wantlist. - // - // Insertion order determines priority. That is, earlier insertions are - // deemed higher priority than keys inserted later. - // - // t = 0, msg.AddWanted(A) - // t = 1, msg.AddWanted(B) - // - // implies Priority(A) > Priority(B) - AddWanted(u.Key) + // AddEntry adds an entry to the Wantlist. + AddEntry(u.Key, int, bool) + + // Sets whether or not the contained wantlist represents the entire wantlist + // true = full wantlist + // false = wantlist 'patch' + // default: true + SetFull(bool) + + Full() bool AddBlock(*blocks.Block) Exportable @@ -43,23 +44,30 @@ 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 + full bool + wantlist map[u.Key]*Entry + blocks map[u.Key]*blocks.Block // map to detect duplicates } func New() BitSwapMessage { return &impl{ - blocks: make(map[u.Key]*blocks.Block), - existsInWantlist: make(map[u.Key]struct{}), - wantlist: make([]u.Key, 0), + blocks: make(map[u.Key]*blocks.Block), + wantlist: make(map[u.Key]*Entry), + full: true, } } +type Entry struct { + Key u.Key + Priority int + Cancel bool +} + func newMessageFromProto(pbm pb.Message) BitSwapMessage { m := New() - for _, s := range pbm.GetWantlist() { - m.AddWanted(u.Key(s)) + m.SetFull(pbm.GetWantlist().GetFull()) + for _, e := range pbm.GetWantlist().GetEntries() { + m.AddEntry(u.Key(e.GetBlock()), int(e.GetPriority()), e.GetCancel()) } for _, d := range pbm.GetBlocks() { b := blocks.NewBlock(d) @@ -68,8 +76,20 @@ func newMessageFromProto(pbm pb.Message) BitSwapMessage { return m } -func (m *impl) Wantlist() []u.Key { - return m.wantlist +func (m *impl) SetFull(full bool) { + m.full = full +} + +func (m *impl) Full() bool { + return m.full +} + +func (m *impl) Wantlist() []*Entry { + var out []*Entry + for _, e := range m.wantlist { + out = append(out, e) + } + return out } func (m *impl) Blocks() []*blocks.Block { @@ -80,13 +100,18 @@ func (m *impl) Blocks() []*blocks.Block { return bs } -func (m *impl) AddWanted(k u.Key) { - _, exists := m.existsInWantlist[k] +func (m *impl) AddEntry(k u.Key, priority int, cancel bool) { + e, exists := m.wantlist[k] if exists { - return + e.Priority = priority + e.Cancel = cancel + } else { + m.wantlist[k] = &Entry{ + Key: k, + Priority: priority, + Cancel: cancel, + } } - m.existsInWantlist[k] = struct{}{} - m.wantlist = append(m.wantlist, k) } func (m *impl) AddBlock(b *blocks.Block) { @@ -106,14 +131,19 @@ func FromNet(r io.Reader) (BitSwapMessage, error) { } func (m *impl) ToProto() *pb.Message { - pb := new(pb.Message) - for _, k := range m.Wantlist() { - pb.Wantlist = append(pb.Wantlist, string(k)) + pbm := new(pb.Message) + pbm.Wantlist = new(pb.Message_Wantlist) + for _, e := range m.wantlist { + pbm.Wantlist.Entries = append(pbm.Wantlist.Entries, &pb.Message_Wantlist_Entry{ + Block: proto.String(string(e.Key)), + Priority: proto.Int32(int32(e.Priority)), + Cancel: &e.Cancel, + }) } for _, b := range m.Blocks() { - pb.Blocks = append(pb.Blocks, b.Data) + pbm.Blocks = append(pbm.Blocks, b.Data) } - return pb + return pbm } func (m *impl) ToNet(w io.Writer) error { diff --git a/exchange/bitswap/message/message_test.go b/exchange/bitswap/message/message_test.go index 681b60a6f..29eb6eb4e 100644 --- a/exchange/bitswap/message/message_test.go +++ b/exchange/bitswap/message/message_test.go @@ -4,6 +4,8 @@ import ( "bytes" "testing" + proto "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" + blocks "github.com/jbenet/go-ipfs/blocks" pb "github.com/jbenet/go-ipfs/exchange/bitswap/message/internal/pb" u "github.com/jbenet/go-ipfs/util" @@ -12,22 +14,26 @@ import ( func TestAppendWanted(t *testing.T) { const str = "foo" m := New() - m.AddWanted(u.Key(str)) + m.AddEntry(u.Key(str), 1, false) - if !contains(m.ToProto().GetWantlist(), str) { + if !wantlistContains(m.ToProto().GetWantlist(), str) { t.Fail() } + m.ToProto().GetWantlist().GetEntries() } func TestNewMessageFromProto(t *testing.T) { const str = "a_key" protoMessage := new(pb.Message) - protoMessage.Wantlist = []string{string(str)} - if !contains(protoMessage.Wantlist, str) { + protoMessage.Wantlist = new(pb.Message_Wantlist) + protoMessage.Wantlist.Entries = []*pb.Message_Wantlist_Entry{ + &pb.Message_Wantlist_Entry{Block: proto.String(str)}, + } + if !wantlistContains(protoMessage.Wantlist, str) { t.Fail() } m := newMessageFromProto(*protoMessage) - if !contains(m.ToProto().GetWantlist(), str) { + if !wantlistContains(m.ToProto().GetWantlist(), str) { t.Fail() } } @@ -57,7 +63,7 @@ func TestWantlist(t *testing.T) { keystrs := []string{"foo", "bar", "baz", "bat"} m := New() for _, s := range keystrs { - m.AddWanted(u.Key(s)) + m.AddEntry(u.Key(s), 1, false) } exported := m.Wantlist() @@ -65,12 +71,12 @@ func TestWantlist(t *testing.T) { present := false for _, s := range keystrs { - if s == string(k) { + if s == string(k.Key) { present = true } } if !present { - t.Logf("%v isn't in original list", string(k)) + t.Logf("%v isn't in original list", k.Key) t.Fail() } } @@ -80,19 +86,19 @@ func TestCopyProtoByValue(t *testing.T) { const str = "foo" m := New() protoBeforeAppend := m.ToProto() - m.AddWanted(u.Key(str)) - if contains(protoBeforeAppend.GetWantlist(), str) { + m.AddEntry(u.Key(str), 1, false) + if wantlistContains(protoBeforeAppend.GetWantlist(), str) { t.Fail() } } func TestToNetFromNetPreservesWantList(t *testing.T) { original := New() - original.AddWanted(u.Key("M")) - original.AddWanted(u.Key("B")) - original.AddWanted(u.Key("D")) - original.AddWanted(u.Key("T")) - original.AddWanted(u.Key("F")) + original.AddEntry(u.Key("M"), 1, false) + original.AddEntry(u.Key("B"), 1, false) + original.AddEntry(u.Key("D"), 1, false) + original.AddEntry(u.Key("T"), 1, false) + original.AddEntry(u.Key("F"), 1, false) var buf bytes.Buffer if err := original.ToNet(&buf); err != nil { @@ -106,11 +112,11 @@ func TestToNetFromNetPreservesWantList(t *testing.T) { keys := make(map[u.Key]bool) for _, k := range copied.Wantlist() { - keys[k] = true + keys[k.Key] = true } for _, k := range original.Wantlist() { - if _, ok := keys[k]; !ok { + if _, ok := keys[k.Key]; !ok { t.Fatalf("Key Missing: \"%v\"", k) } } @@ -146,9 +152,18 @@ func TestToAndFromNetMessage(t *testing.T) { } } -func contains(s []string, x string) bool { - for _, a := range s { - if a == x { +func wantlistContains(wantlist *pb.Message_Wantlist, x string) bool { + for _, e := range wantlist.GetEntries() { + if e.GetBlock() == x { + return true + } + } + return false +} + +func contains(strs []string, x string) bool { + for _, s := range strs { + if s == x { return true } } @@ -159,8 +174,8 @@ func TestDuplicates(t *testing.T) { b := blocks.NewBlock([]byte("foo")) msg := New() - msg.AddWanted(b.Key()) - msg.AddWanted(b.Key()) + msg.AddEntry(b.Key(), 1, false) + msg.AddEntry(b.Key(), 1, false) if len(msg.Wantlist()) != 1 { t.Fatal("Duplicate in BitSwapMessage") } diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go index 58385f5b7..c74b58c42 100644 --- a/exchange/bitswap/strategy/interface.go +++ b/exchange/bitswap/strategy/interface.go @@ -3,6 +3,7 @@ package strategy import ( "time" + bstore "github.com/jbenet/go-ipfs/blocks/blockstore" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" @@ -34,6 +35,8 @@ type Strategy interface { BlockSentToPeer(u.Key, peer.Peer) + GetAllocation(int, bstore.Blockstore) ([]*Task, error) + // Values determining bitswap behavioural patterns GetBatchSize() int GetRebroadcastDelay() time.Duration diff --git a/exchange/bitswap/strategy/ledger.go b/exchange/bitswap/strategy/ledger.go index 84e92d035..7ce7b73d9 100644 --- a/exchange/bitswap/strategy/ledger.go +++ b/exchange/bitswap/strategy/ledger.go @@ -3,6 +3,7 @@ package strategy import ( "time" + wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" ) @@ -13,7 +14,7 @@ type keySet map[u.Key]struct{} func newLedger(p peer.Peer, strategy strategyFunc) *ledger { return &ledger{ - wantList: keySet{}, + wantList: wl.NewWantlist(), Strategy: strategy, Partner: p, sentToPeer: make(map[u.Key]time.Time), @@ -39,7 +40,7 @@ type ledger struct { exchangeCount uint64 // wantList is a (bounded, small) set of keys that Partner desires. - wantList keySet + wantList *wl.Wantlist // sentToPeer is a set of keys to ensure we dont send duplicate blocks // to a given peer @@ -65,14 +66,17 @@ func (l *ledger) ReceivedBytes(n int) { } // TODO: this needs to be different. We need timeouts. -func (l *ledger) Wants(k u.Key) { +func (l *ledger) Wants(k u.Key, priority int) { log.Debugf("peer %s wants %s", l.Partner, k) - l.wantList[k] = struct{}{} + l.wantList.Add(k, priority) +} + +func (l *ledger) CancelWant(k u.Key) { + l.wantList.Remove(k) } func (l *ledger) WantListContains(k u.Key) bool { - _, ok := l.wantList[k] - return ok + return l.wantList.Contains(k) } func (l *ledger) ExchangeCount() uint64 { diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index fe7414caa..b21a3b2b1 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -5,7 +5,10 @@ import ( "sync" "time" + blocks "github.com/jbenet/go-ipfs/blocks" + bstore "github.com/jbenet/go-ipfs/blocks/blockstore" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" + wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" ) @@ -77,6 +80,60 @@ func (s *strategist) ShouldSendBlockToPeer(k u.Key, p peer.Peer) bool { return ledger.ShouldSend() } +type Task struct { + Peer peer.Peer + Blocks []*blocks.Block +} + +func (s *strategist) GetAllocation(bandwidth int, bs bstore.Blockstore) ([]*Task, error) { + var tasks []*Task + + s.lock.RLock() + defer s.lock.RUnlock() + var partners []peer.Peer + for _, ledger := range s.ledgerMap { + if ledger.ShouldSend() { + partners = append(partners, ledger.Partner) + } + } + if len(partners) == 0 { + return nil, nil + } + + bandwidthPerPeer := bandwidth / len(partners) + for _, p := range partners { + blksForPeer, err := s.getSendableBlocks(s.ledger(p).wantList, bs, bandwidthPerPeer) + if err != nil { + return nil, err + } + tasks = append(tasks, &Task{ + Peer: p, + Blocks: blksForPeer, + }) + } + + return tasks, nil +} + +func (s *strategist) getSendableBlocks(wantlist *wl.Wantlist, bs bstore.Blockstore, bw int) ([]*blocks.Block, error) { + var outblocks []*blocks.Block + for _, e := range wantlist.Entries() { + block, err := bs.Get(e.Value) + if err == u.ErrNotFound { + continue + } + if err != nil { + return nil, err + } + outblocks = append(outblocks, block) + bw -= len(block.Data) + if bw <= 0 { + break + } + } + return outblocks, nil +} + func (s *strategist) BlockSentToPeer(k u.Key, p peer.Peer) { s.lock.Lock() defer s.lock.Unlock() @@ -106,8 +163,15 @@ func (s *strategist) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error return errors.New("Strategy received nil message") } l := s.ledger(p) - for _, key := range m.Wantlist() { - l.Wants(key) + if m.Full() { + l.wantList = wl.NewWantlist() + } + for _, e := range m.Wantlist() { + if e.Cancel { + l.CancelWant(e.Key) + } else { + l.Wants(e.Key, e.Priority) + } } for _, block := range m.Blocks() { // FIXME extract blocks.NumBytes(block) or block.NumBytes() method @@ -165,5 +229,5 @@ func (s *strategist) GetBatchSize() int { } func (s *strategist) GetRebroadcastDelay() time.Duration { - return time.Second * 5 + return time.Second * 10 } diff --git a/exchange/bitswap/strategy/strategy_test.go b/exchange/bitswap/strategy/strategy_test.go index e063dff68..687ea4d34 100644 --- a/exchange/bitswap/strategy/strategy_test.go +++ b/exchange/bitswap/strategy/strategy_test.go @@ -61,7 +61,7 @@ func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { block := blocks.NewBlock([]byte("data wanted by beggar")) messageFromBeggarToChooser := message.New() - messageFromBeggarToChooser.AddWanted(block.Key()) + messageFromBeggarToChooser.AddEntry(block.Key(), 1, false) chooser.MessageReceived(beggar.Peer, messageFromBeggarToChooser) // for this test, doesn't matter if you record that beggar sent From 57e7dd7b8bc2a952ec34e5c3e576af6195b28847 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 07:57:39 +0000 Subject: [PATCH 04/64] extracted ledgerset from strategy, cleaned up a few comments from the PR --- exchange/bitswap/bitswap.go | 61 +++---- exchange/bitswap/bitswap_test.go | 80 ++++----- exchange/bitswap/message/message.go | 4 +- exchange/bitswap/strategy/interface.go | 33 +--- exchange/bitswap/strategy/ledger.go | 11 +- exchange/bitswap/strategy/ledgerset.go | 125 ++++++++++++++ .../{strategy_test.go => ledgerset_test.go} | 51 +++--- exchange/bitswap/strategy/strategy.go | 158 +----------------- exchange/bitswap/wantlist/wantlist.go | 2 +- 9 files changed, 216 insertions(+), 309 deletions(-) create mode 100644 exchange/bitswap/strategy/ledgerset.go rename exchange/bitswap/strategy/{strategy_test.go => ledgerset_test.go} (56%) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 1e0e86b61..d9da3380c 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -27,11 +27,14 @@ 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 +var providerRequestTimeout = time.Second * 10 +var hasBlockTimeout = time.Second * 15 +var rebroadcastDelay = time.Second * 10 const roundTime = time.Second / 2 +var bandwidthPerRound = 500000 + // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. @@ -53,13 +56,14 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout cancelFunc: cancelFunc, notifications: notif, strategy: strategy.New(nice), + ledgerset: strategy.NewLedgerSet(), routing: routing, sender: network, - wantlist: wl.NewWantlist(), + wantlist: wl.New(), batchRequests: make(chan []u.Key, 32), } network.SetDelegate(bs) - go bs.loop(ctx) + go bs.clientWorker(ctx) go bs.roundWorker(ctx) return bs @@ -85,11 +89,11 @@ type bitswap struct { // 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. - // TODO(brian): save the strategy's state to the datastore + // strategy makes decisions about how to interact with partners. strategy strategy.Strategy + ledgerset *strategy.LedgerSet + wantlist *wl.Wantlist // cancelFunc signals cancellation to the bitswap event loop @@ -159,10 +163,6 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { bs.wantlist.Remove(blk.Key()) bs.notifications.Publish(blk) child, _ := context.WithTimeout(ctx, hasBlockTimeout) - if err := bs.sendToPeersThatWant(child, blk); err != nil { - return err - } - child, _ = context.WithTimeout(ctx, hasBlockTimeout) return bs.routing.Provide(child, blk.Key()) } @@ -194,7 +194,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e // 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) + bs.ledgerset.MessageSent(p, message) }(peerToQuery) } return nil @@ -220,17 +220,16 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan func (bs *bitswap) roundWorker(ctx context.Context) { roundTicker := time.NewTicker(roundTime) - bandwidthPerRound := 500000 for { select { case <-ctx.Done(): return case <-roundTicker.C: - alloc, err := bs.strategy.GetAllocation(bandwidthPerRound, bs.blockstore) + alloc, err := bs.strategy.GetTasks(bandwidthPerRound, bs.ledgerset, bs.blockstore) if err != nil { log.Critical("%s", err) } - //log.Errorf("Allocation: %v", alloc) + log.Error(alloc) bs.processStrategyAllocation(ctx, alloc) } } @@ -241,9 +240,6 @@ func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strat for _, block := range t.Blocks { message := bsmsg.New() message.AddBlock(block) - for _, wanted := range bs.wantlist.Entries() { - message.AddEntry(wanted.Value, wanted.Priority, false) - } if err := bs.send(ctx, t.Peer, message); err != nil { log.Errorf("Message Send Failed: %s", err) } @@ -252,11 +248,11 @@ func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strat } // TODO ensure only one active request per key -func (bs *bitswap) loop(parent context.Context) { +func (bs *bitswap) clientWorker(parent context.Context) { ctx, cancel := context.WithCancel(parent) - broadcastSignal := time.NewTicker(bs.strategy.GetRebroadcastDelay()) + broadcastSignal := time.NewTicker(rebroadcastDelay) defer func() { cancel() // signal to derived async functions broadcastSignal.Stop() @@ -317,13 +313,14 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // This call records changes to wantlists, blocks received, // and number of bytes transfered. - bs.strategy.MessageReceived(p, incoming) + bs.ledgerset.MessageReceived(p, incoming) // TODO: this is bad, and could be easily abused. // Should only track *useful* messages in ledger var blkeys []u.Key for _, block := range incoming.Blocks() { blkeys = append(blkeys, block.Key()) + log.Errorf("Got block: %s", block) if err := bs.HasBlock(ctx, block); err != nil { log.Error(err) } @@ -342,7 +339,7 @@ func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { for _, k := range bkeys { message.AddEntry(k, 0, true) } - for _, p := range bs.strategy.Peers() { + for _, p := range bs.ledgerset.Peers() { err := bs.send(ctx, p, message) if err != nil { log.Errorf("Error sending message: %s", err) @@ -362,25 +359,7 @@ func (bs *bitswap) send(ctx context.Context, p peer.Peer, m bsmsg.BitSwapMessage if err := bs.sender.SendMessage(ctx, p, m); err != nil { return err } - return bs.strategy.MessageSent(p, m) -} - -func (bs *bitswap) sendToPeersThatWant(ctx context.Context, block *blocks.Block) error { - for _, p := range bs.strategy.Peers() { - if bs.strategy.BlockIsWantedByPeer(block.Key(), p) { - if bs.strategy.ShouldSendBlockToPeer(block.Key(), p) { - message := bsmsg.New() - message.AddBlock(block) - for _, wanted := range bs.wantlist.Entries() { - message.AddEntry(wanted.Value, wanted.Priority, false) - } - if err := bs.send(ctx, p, message); err != nil { - return err - } - } - } - } - return nil + return bs.ledgerset.MessageSent(p, m) } func (bs *bitswap) Close() error { diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 0e72883cc..9bf71dea6 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -206,60 +206,44 @@ func TestSendToWantingPeer(t *testing.T) { defer sg.Stop() bg := blocksutil.NewBlockGenerator() - me := sg.Next() - w := sg.Next() - o := sg.Next() + oldVal := rebroadcastDelay + rebroadcastDelay = time.Second / 2 + defer func() { rebroadcastDelay = oldVal }() - t.Logf("Session %v\n", me.Peer) - t.Logf("Session %v\n", w.Peer) - t.Logf("Session %v\n", o.Peer) + peerA := sg.Next() + peerB := sg.Next() + + t.Logf("Session %v\n", peerA.Peer) + t.Logf("Session %v\n", peerB.Peer) + + timeout := time.Second + waitTime := time.Second * 5 alpha := bg.Next() - - const timeout = 1000 * 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) - _, 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()) - ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := w.Blockstore().Put(beta); err != nil { - t.Fatal(err) - } - 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()) - ctx, _ = context.WithTimeout(context.Background(), timeout) - 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()) - ctx, _ = context.WithTimeout(context.Background(), timeout) - if err := o.Blockstore().Put(alpha); err != nil { - t.Fatal(err) - } - o.Exchange.HasBlock(ctx, alpha) - - 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 { - t.Fatal(err) - } - - t.Logf("%v should now have %v\n", w.Peer, alpha.Key()) - block, err := w.Blockstore().Get(alpha.Key()) + // peerA requests and waits for block alpha + ctx, _ := context.WithTimeout(context.TODO(), waitTime) + alphaPromise, err := peerA.Exchange.GetBlocks(ctx, []u.Key{alpha.Key()}) if err != nil { - t.Fatalf("Should not have received an error: %s", err) + t.Fatal(err) } - if block.Key() != alpha.Key() { - t.Fatal("Expected to receive alpha from me") + + // peerB announces to the network that he has block alpha + ctx, _ = context.WithTimeout(context.TODO(), timeout) + err = peerB.Exchange.HasBlock(ctx, alpha) + if err != nil { + t.Fatal(err) } + + // At some point, peerA should get alpha (or timeout) + blkrecvd, ok := <-alphaPromise + if !ok { + t.Fatal("context timed out and broke promise channel!") + } + + if blkrecvd.Key() != alpha.Key() { + t.Fatal("Wrong block!") + } + } func TestBasicBitswap(t *testing.T) { diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index 288fc9da7..b636e2024 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -24,13 +24,13 @@ type BitSwapMessage interface { Blocks() []*blocks.Block // AddEntry adds an entry to the Wantlist. - AddEntry(u.Key, int, bool) + AddEntry(key u.Key, priority int, cancel bool) // Sets whether or not the contained wantlist represents the entire wantlist // true = full wantlist // false = wantlist 'patch' // default: true - SetFull(bool) + SetFull(isFull bool) Full() bool diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go index c74b58c42..54af581f7 100644 --- a/exchange/bitswap/strategy/interface.go +++ b/exchange/bitswap/strategy/interface.go @@ -1,43 +1,12 @@ package strategy import ( - "time" - bstore "github.com/jbenet/go-ipfs/blocks/blockstore" - bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" - peer "github.com/jbenet/go-ipfs/peer" - u "github.com/jbenet/go-ipfs/util" ) type Strategy interface { - // Returns a slice of Peers with whom the local node has active sessions - Peers() []peer.Peer - - // BlockIsWantedByPeer returns true if peer wants the block given by this - // key - BlockIsWantedByPeer(u.Key, peer.Peer) bool - - // ShouldSendTo(Peer) decides whether to send data to this Peer - ShouldSendBlockToPeer(u.Key, peer.Peer) bool - // Seed initializes the decider to a deterministic state Seed(int64) - // MessageReceived records receipt of message for accounting purposes - MessageReceived(peer.Peer, bsmsg.BitSwapMessage) error - - // MessageSent records sending of message for accounting purposes - MessageSent(peer.Peer, bsmsg.BitSwapMessage) error - - NumBytesSentTo(peer.Peer) uint64 - - NumBytesReceivedFrom(peer.Peer) uint64 - - BlockSentToPeer(u.Key, peer.Peer) - - GetAllocation(int, bstore.Blockstore) ([]*Task, error) - - // Values determining bitswap behavioural patterns - GetBatchSize() int - GetRebroadcastDelay() time.Duration + GetTasks(bandwidth int, ledgers *LedgerSet, bs bstore.Blockstore) ([]*Task, error) } diff --git a/exchange/bitswap/strategy/ledger.go b/exchange/bitswap/strategy/ledger.go index 7ce7b73d9..684d383ef 100644 --- a/exchange/bitswap/strategy/ledger.go +++ b/exchange/bitswap/strategy/ledger.go @@ -12,10 +12,9 @@ import ( // access/lookups. type keySet map[u.Key]struct{} -func newLedger(p peer.Peer, strategy strategyFunc) *ledger { +func newLedger(p peer.Peer) *ledger { return &ledger{ - wantList: wl.NewWantlist(), - Strategy: strategy, + wantList: wl.New(), Partner: p, sentToPeer: make(map[u.Key]time.Time), } @@ -45,12 +44,6 @@ type ledger struct { // sentToPeer is a set of keys to ensure we dont send duplicate blocks // to a given peer sentToPeer map[u.Key]time.Time - - Strategy strategyFunc -} - -func (l *ledger) ShouldSend() bool { - return l.Strategy(l) } func (l *ledger) SentBytes(n int) { diff --git a/exchange/bitswap/strategy/ledgerset.go b/exchange/bitswap/strategy/ledgerset.go new file mode 100644 index 000000000..b5f03ae65 --- /dev/null +++ b/exchange/bitswap/strategy/ledgerset.go @@ -0,0 +1,125 @@ +package strategy + +import ( + "sync" + + bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" + wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" + peer "github.com/jbenet/go-ipfs/peer" + u "github.com/jbenet/go-ipfs/util" +) + +// LedgerMap lists Ledgers by their Partner key. +type ledgerMap map[peerKey]*ledger + +// FIXME share this externally +type peerKey u.Key + +type LedgerSet struct { + lock sync.RWMutex + ledgerMap ledgerMap +} + +func NewLedgerSet() *LedgerSet { + return &LedgerSet{ + ledgerMap: make(ledgerMap), + } +} + +// Returns a slice of Peers with whom the local node has active sessions +func (ls *LedgerSet) Peers() []peer.Peer { + ls.lock.RLock() + defer ls.lock.RUnlock() + + response := make([]peer.Peer, 0) + for _, ledger := range ls.ledgerMap { + response = append(response, ledger.Partner) + } + return response +} + +// BlockIsWantedByPeer returns true if peer wants the block given by this +// key +func (ls *LedgerSet) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { + ls.lock.RLock() + defer ls.lock.RUnlock() + + ledger := ls.ledger(p) + return ledger.WantListContains(k) +} + +// MessageReceived performs book-keeping. Returns error if passed invalid +// arguments. +func (ls *LedgerSet) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { + ls.lock.Lock() + defer ls.lock.Unlock() + + // TODO find a more elegant way to handle this check + /* + if p == nil { + return errors.New("Strategy received nil peer") + } + if m == nil { + return errors.New("Strategy received nil message") + } + */ + l := ls.ledger(p) + if m.Full() { + l.wantList = wl.New() + } + for _, e := range m.Wantlist() { + if e.Cancel { + l.CancelWant(e.Key) + } else { + l.Wants(e.Key, e.Priority) + } + } + for _, block := range m.Blocks() { + // FIXME extract blocks.NumBytes(block) or block.NumBytes() method + l.ReceivedBytes(len(block.Data)) + } + return nil +} + +// TODO add contents of m.WantList() to my local wantlist? NB: could introduce +// race conditions where I send a message, but MessageSent gets handled after +// MessageReceived. The information in the local wantlist could become +// inconsistent. Would need to ensure that Sends and acknowledgement of the +// send happen atomically + +func (ls *LedgerSet) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { + ls.lock.Lock() + defer ls.lock.Unlock() + + l := ls.ledger(p) + for _, block := range m.Blocks() { + l.SentBytes(len(block.Data)) + l.wantList.Remove(block.Key()) + } + + return nil +} + +func (ls *LedgerSet) NumBytesSentTo(p peer.Peer) uint64 { + ls.lock.RLock() + defer ls.lock.RUnlock() + + return ls.ledger(p).Accounting.BytesSent +} + +func (ls *LedgerSet) NumBytesReceivedFrom(p peer.Peer) uint64 { + ls.lock.RLock() + defer ls.lock.RUnlock() + + return ls.ledger(p).Accounting.BytesRecv +} + +// ledger lazily instantiates a ledger +func (ls *LedgerSet) ledger(p peer.Peer) *ledger { + l, ok := ls.ledgerMap[peerKey(p.Key())] + if !ok { + l = newLedger(p) + ls.ledgerMap[peerKey(p.Key())] = l + } + return l +} diff --git a/exchange/bitswap/strategy/strategy_test.go b/exchange/bitswap/strategy/ledgerset_test.go similarity index 56% rename from exchange/bitswap/strategy/strategy_test.go rename to exchange/bitswap/strategy/ledgerset_test.go index 687ea4d34..795752a12 100644 --- a/exchange/bitswap/strategy/strategy_test.go +++ b/exchange/bitswap/strategy/ledgerset_test.go @@ -10,21 +10,22 @@ import ( testutil "github.com/jbenet/go-ipfs/util/testutil" ) -type peerAndStrategist struct { +type peerAndLedgerset struct { peer.Peer - Strategy + ls *LedgerSet } -func newPeerAndStrategist(idStr string) peerAndStrategist { - return peerAndStrategist{ - Peer: testutil.NewPeerWithIDString(idStr), - Strategy: New(true), +func newPeerAndLedgerset(idStr string) peerAndLedgerset { + return peerAndLedgerset{ + Peer: testutil.NewPeerWithIDString(idStr), + //Strategy: New(true), + ls: NewLedgerSet(), } } func TestConsistentAccounting(t *testing.T) { - sender := newPeerAndStrategist("Ernie") - receiver := newPeerAndStrategist("Bert") + sender := newPeerAndLedgerset("Ernie") + receiver := newPeerAndLedgerset("Bert") // Send messages from Ernie to Bert for i := 0; i < 1000; i++ { @@ -33,69 +34,69 @@ func TestConsistentAccounting(t *testing.T) { content := []string{"this", "is", "message", "i"} m.AddBlock(blocks.NewBlock([]byte(strings.Join(content, " ")))) - sender.MessageSent(receiver.Peer, m) - receiver.MessageReceived(sender.Peer, m) + sender.ls.MessageSent(receiver.Peer, m) + receiver.ls.MessageReceived(sender.Peer, m) } // Ensure sender records the change - if sender.NumBytesSentTo(receiver.Peer) == 0 { + if sender.ls.NumBytesSentTo(receiver.Peer) == 0 { t.Fatal("Sent bytes were not recorded") } // Ensure sender and receiver have the same values - if sender.NumBytesSentTo(receiver.Peer) != receiver.NumBytesReceivedFrom(sender.Peer) { + if sender.ls.NumBytesSentTo(receiver.Peer) != receiver.ls.NumBytesReceivedFrom(sender.Peer) { t.Fatal("Inconsistent book-keeping. Strategies don't agree") } // Ensure sender didn't record receving anything. And that the receiver // didn't record sending anything - if receiver.NumBytesSentTo(sender.Peer) != 0 || sender.NumBytesReceivedFrom(receiver.Peer) != 0 { + if receiver.ls.NumBytesSentTo(sender.Peer) != 0 || sender.ls.NumBytesReceivedFrom(receiver.Peer) != 0 { t.Fatal("Bert didn't send bytes to Ernie") } } func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { - beggar := newPeerAndStrategist("can't be chooser") - chooser := newPeerAndStrategist("chooses JIF") + beggar := newPeerAndLedgerset("can't be chooser") + chooser := newPeerAndLedgerset("chooses JIF") block := blocks.NewBlock([]byte("data wanted by beggar")) messageFromBeggarToChooser := message.New() messageFromBeggarToChooser.AddEntry(block.Key(), 1, false) - chooser.MessageReceived(beggar.Peer, messageFromBeggarToChooser) + chooser.ls.MessageReceived(beggar.Peer, messageFromBeggarToChooser) // for this test, doesn't matter if you record that beggar sent - if !chooser.BlockIsWantedByPeer(block.Key(), beggar.Peer) { + if !chooser.ls.BlockIsWantedByPeer(block.Key(), beggar.Peer) { t.Fatal("chooser failed to record that beggar wants block") } } func TestPeerIsAddedToPeersWhenMessageReceivedOrSent(t *testing.T) { - sanfrancisco := newPeerAndStrategist("sf") - seattle := newPeerAndStrategist("sea") + sanfrancisco := newPeerAndLedgerset("sf") + seattle := newPeerAndLedgerset("sea") m := message.New() - sanfrancisco.MessageSent(seattle.Peer, m) - seattle.MessageReceived(sanfrancisco.Peer, m) + sanfrancisco.ls.MessageSent(seattle.Peer, m) + seattle.ls.MessageReceived(sanfrancisco.Peer, m) if seattle.Peer.Key() == sanfrancisco.Peer.Key() { t.Fatal("Sanity Check: Peers have same Key!") } - if !peerIsPartner(seattle.Peer, sanfrancisco.Strategy) { + if !peerIsPartner(seattle.Peer, sanfrancisco.ls) { t.Fatal("Peer wasn't added as a Partner") } - if !peerIsPartner(sanfrancisco.Peer, seattle.Strategy) { + if !peerIsPartner(sanfrancisco.Peer, seattle.ls) { t.Fatal("Peer wasn't added as a Partner") } } -func peerIsPartner(p peer.Peer, s Strategy) bool { - for _, partner := range s.Peers() { +func peerIsPartner(p peer.Peer, ls *LedgerSet) bool { + for _, partner := range ls.Peers() { if partner.Key() == p.Key() { return true } diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index b21a3b2b1..d425fcc77 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -1,20 +1,13 @@ package strategy import ( - "errors" - "sync" - "time" - blocks "github.com/jbenet/go-ipfs/blocks" bstore "github.com/jbenet/go-ipfs/blocks/blockstore" - bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" ) -const resendTimeoutPeriod = time.Minute - var log = u.Logger("strategy") // TODO niceness should be on a per-peer basis. Use-case: Certain peers are @@ -28,81 +21,37 @@ func New(nice bool) Strategy { stratFunc = standardStrategy } return &strategist{ - ledgerMap: ledgerMap{}, strategyFunc: stratFunc, } } type strategist struct { - lock sync.RWMutex - ledgerMap strategyFunc } -// LedgerMap lists Ledgers by their Partner key. -type ledgerMap map[peerKey]*ledger - -// FIXME share this externally -type peerKey u.Key - -// Peers returns a list of peers -func (s *strategist) Peers() []peer.Peer { - s.lock.RLock() - defer s.lock.RUnlock() - - response := make([]peer.Peer, 0) - for _, ledger := range s.ledgerMap { - response = append(response, ledger.Partner) - } - return response -} - -func (s *strategist) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { - s.lock.RLock() - defer s.lock.RUnlock() - - ledger := s.ledger(p) - return ledger.WantListContains(k) -} - -func (s *strategist) ShouldSendBlockToPeer(k u.Key, p peer.Peer) bool { - s.lock.RLock() - defer s.lock.RUnlock() - - ledger := s.ledger(p) - - // Dont resend blocks within a certain time period - t, ok := ledger.sentToPeer[k] - if ok && t.Add(resendTimeoutPeriod).After(time.Now()) { - return false - } - - return ledger.ShouldSend() -} - type Task struct { Peer peer.Peer Blocks []*blocks.Block } -func (s *strategist) GetAllocation(bandwidth int, bs bstore.Blockstore) ([]*Task, error) { +func (s *strategist) GetTasks(bandwidth int, ledgers *LedgerSet, bs bstore.Blockstore) ([]*Task, error) { var tasks []*Task - s.lock.RLock() - defer s.lock.RUnlock() + ledgers.lock.RLock() var partners []peer.Peer - for _, ledger := range s.ledgerMap { - if ledger.ShouldSend() { + for _, ledger := range ledgers.ledgerMap { + if s.strategyFunc(ledger) { partners = append(partners, ledger.Partner) } } + ledgers.lock.RUnlock() if len(partners) == 0 { return nil, nil } bandwidthPerPeer := bandwidth / len(partners) for _, p := range partners { - blksForPeer, err := s.getSendableBlocks(s.ledger(p).wantList, bs, bandwidthPerPeer) + blksForPeer, err := s.getSendableBlocks(ledgers.ledger(p).wantList, bs, bandwidthPerPeer) if err != nil { return nil, err } @@ -134,100 +83,7 @@ func (s *strategist) getSendableBlocks(wantlist *wl.Wantlist, bs bstore.Blocksto return outblocks, nil } -func (s *strategist) BlockSentToPeer(k u.Key, p peer.Peer) { - s.lock.Lock() - defer s.lock.Unlock() - - ledger := s.ledger(p) - ledger.sentToPeer[k] = time.Now() -} - +func test() {} func (s *strategist) Seed(int64) { - s.lock.Lock() - defer s.lock.Unlock() - // 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() - - // TODO find a more elegant way to handle this check - if p == nil { - return errors.New("Strategy received nil peer") - } - if m == nil { - return errors.New("Strategy received nil message") - } - l := s.ledger(p) - if m.Full() { - l.wantList = wl.NewWantlist() - } - for _, e := range m.Wantlist() { - if e.Cancel { - l.CancelWant(e.Key) - } else { - l.Wants(e.Key, e.Priority) - } - } - for _, block := range m.Blocks() { - // FIXME extract blocks.NumBytes(block) or block.NumBytes() method - l.ReceivedBytes(len(block.Data)) - } - return nil -} - -// TODO add contents of m.WantList() to my local wantlist? NB: could introduce -// race conditions where I send a message, but MessageSent gets handled after -// MessageReceived. The information in the local wantlist could become -// inconsistent. Would need to ensure that Sends and acknowledgement of the -// send happen atomically - -func (s *strategist) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { - s.lock.Lock() - defer s.lock.Unlock() - - l := s.ledger(p) - for _, block := range m.Blocks() { - l.SentBytes(len(block.Data)) - } - - // TODO remove these blocks from peer's want list - - return nil -} - -func (s *strategist) NumBytesSentTo(p peer.Peer) uint64 { - s.lock.RLock() - defer s.lock.RUnlock() - - return s.ledger(p).Accounting.BytesSent -} - -func (s *strategist) NumBytesReceivedFrom(p peer.Peer) uint64 { - s.lock.RLock() - defer s.lock.RUnlock() - - return s.ledger(p).Accounting.BytesRecv -} - -// ledger lazily instantiates a ledger -func (s *strategist) ledger(p peer.Peer) *ledger { - l, ok := s.ledgerMap[peerKey(p.Key())] - if !ok { - l = newLedger(p, s.strategyFunc) - s.ledgerMap[peerKey(p.Key())] = l - } - return l -} - -func (s *strategist) GetBatchSize() int { - return 10 -} - -func (s *strategist) GetRebroadcastDelay() time.Duration { - return time.Second * 10 -} diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 041064901..f9cf52eb2 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -9,7 +9,7 @@ type Wantlist struct { set map[u.Key]*Entry } -func NewWantlist() *Wantlist { +func New() *Wantlist { return &Wantlist{ set: make(map[u.Key]*Entry), } From 3778eedff0ec11a8edc7ac4a82f87a10e83668fb Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 18:47:11 +0000 Subject: [PATCH 05/64] dont spawn so many goroutines when rebroadcasting wantlist --- exchange/bitswap/bitswap.go | 69 +++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index d9da3380c..33f37b107 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -201,21 +201,57 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wantlist) { + provset := make(map[u.Key]peer.Peer) + provcollect := make(chan peer.Peer) + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + wg := sync.WaitGroup{} + // Get providers for all entries in wantlist (could take a while) for _, e := range wantlist.Entries() { wg.Add(1) go func(k u.Key) { child, _ := context.WithTimeout(ctx, providerRequestTimeout) providers := bs.routing.FindProvidersAsync(child, k, maxProvidersPerRequest) - err := bs.sendWantListTo(ctx, providers) - if err != nil { - log.Errorf("error sending wantlist: %s", err) + for prov := range providers { + provcollect <- prov } wg.Done() }(e.Value) } - wg.Wait() + + // When all workers finish, close the providers channel + go func() { + wg.Wait() + close(provcollect) + }() + + // Filter out duplicates, + // no need to send our wantlists out twice in a given time period + for { + select { + case p, ok := <-provcollect: + if !ok { + break + } + provset[p.Key()] = p + case <-ctx.Done(): + log.Error("Context cancelled before we got all the providers!") + return + } + } + + message := bsmsg.New() + message.SetFull(true) + for _, e := range bs.wantlist.Entries() { + message.AddEntry(e.Value, e.Priority, false) + } + + for _, prov := range provset { + bs.send(ctx, prov, message) + } } func (bs *bitswap) roundWorker(ctx context.Context) { @@ -229,22 +265,25 @@ func (bs *bitswap) roundWorker(ctx context.Context) { if err != nil { log.Critical("%s", err) } - log.Error(alloc) - bs.processStrategyAllocation(ctx, alloc) + err = bs.processStrategyAllocation(ctx, alloc) + if err != nil { + log.Critical("Error processing strategy allocation: %s", err) + } } } } -func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strategy.Task) { +func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strategy.Task) error { for _, t := range alloc { for _, block := range t.Blocks { message := bsmsg.New() message.AddBlock(block) if err := bs.send(ctx, t.Peer, message); err != nil { - log.Errorf("Message Send Failed: %s", err) + return err } } } + return nil } // TODO ensure only one active request per key @@ -252,22 +291,16 @@ func (bs *bitswap) clientWorker(parent context.Context) { ctx, cancel := context.WithCancel(parent) - broadcastSignal := time.NewTicker(rebroadcastDelay) - defer func() { - cancel() // signal to derived async functions - broadcastSignal.Stop() - }() + broadcastSignal := time.After(rebroadcastDelay) + defer cancel() for { select { - case <-broadcastSignal.C: + case <-broadcastSignal: // Resend unfulfilled wantlist keys bs.sendWantlistToProviders(ctx, bs.wantlist) + broadcastSignal = time.After(rebroadcastDelay) 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 From e7bba82dcbff2c02284a3ce0a89274ba2fbff0c5 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 21:12:07 +0000 Subject: [PATCH 06/64] add priorities to GetBlocks requests, and add waitgroup to sendWantListTo --- exchange/bitswap/bitswap.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 33f37b107..b3fc629b9 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -174,10 +174,12 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e for _, wanted := range bs.wantlist.Entries() { message.AddEntry(wanted.Value, wanted.Priority, false) } + wg := sync.WaitGroup{} for peerToQuery := range peers { - log.Debug("sending query to: %s", peerToQuery) log.Event(ctx, "PeerToQuery", peerToQuery) + wg.Add(1) go func(p peer.Peer) { + defer wg.Done() log.Event(ctx, "DialPeer", p) err := bs.sender.DialPeer(ctx, p) @@ -197,6 +199,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e bs.ledgerset.MessageSent(p, message) }(peerToQuery) } + wg.Wait() return nil } @@ -305,8 +308,8 @@ func (bs *bitswap) clientWorker(parent context.Context) { log.Warning("Received batch request for zero blocks") continue } - for _, k := range ks { - bs.wantlist.Add(k, 1) + for i, k := range ks { + bs.wantlist.Add(k, len(ks)-i) } // 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 From 50aa37fec42e1c6f28e7786343cf3fd0fbd8968d Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 10 Dec 2014 23:01:56 +0000 Subject: [PATCH 07/64] blockstore.ErrNotFound, and proper wantlist sorting --- blocks/blockstore/blockstore.go | 7 ++++++- blockservice/blockservice.go | 4 +--- exchange/bitswap/strategy/strategy.go | 2 +- exchange/bitswap/wantlist/wantlist.go | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/blocks/blockstore/blockstore.go b/blocks/blockstore/blockstore.go index d4849cb43..e203ffc50 100644 --- a/blocks/blockstore/blockstore.go +++ b/blocks/blockstore/blockstore.go @@ -6,14 +6,16 @@ import ( "errors" 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" u "github.com/jbenet/go-ipfs/util" ) var ValueTypeMismatch = errors.New("The retrieved value is not a Block") +var ErrNotFound = errors.New("blockstore: block not found") + // Blockstore wraps a ThreadSafeDatastore type Blockstore interface { DeleteBlock(u.Key) error @@ -34,6 +36,9 @@ type blockstore struct { func (bs *blockstore) Get(k u.Key) (*blocks.Block, error) { maybeData, err := bs.datastore.Get(k.DsKey()) + if err == ds.ErrNotFound { + return nil, ErrNotFound + } if err != nil { return nil, err } diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index f44eaa0f5..9014cab46 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -8,8 +8,6 @@ import ( "fmt" 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" "github.com/jbenet/go-ipfs/blocks/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" @@ -67,7 +65,7 @@ func (s *BlockService) GetBlock(ctx context.Context, k u.Key) (*blocks.Block, er return block, nil // TODO be careful checking ErrNotFound. If the underlying // implementation changes, this will break. - } else if err == ds.ErrNotFound && s.Exchange != nil { + } else if err == blockstore.ErrNotFound && s.Exchange != nil { log.Debug("Blockservice: Searching bitswap.") blk, err := s.Exchange.GetBlock(ctx, k) if err != nil { diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index d425fcc77..ff7f4d74d 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -68,7 +68,7 @@ func (s *strategist) getSendableBlocks(wantlist *wl.Wantlist, bs bstore.Blocksto var outblocks []*blocks.Block for _, e := range wantlist.Entries() { block, err := bs.Get(e.Value) - if err == u.ErrNotFound { + if err == bstore.ErrNotFound { continue } if err != nil { diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index f9cf52eb2..d57b9d523 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -43,7 +43,7 @@ type entrySlice []*Entry func (es entrySlice) Len() int { return len(es) } func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } -func (es entrySlice) Less(i, j int) bool { return es[i].Priority < es[j].Priority } +func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priority } func (w *Wantlist) Entries() []*Entry { var es entrySlice From da68475bcfdc5fbe4981be1fca8d78f76e51faa2 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 10 Dec 2014 17:02:47 -0800 Subject: [PATCH 08/64] fix go version License: MIT Signed-off-by: Brian Tiger Chow --- Godeps/Godeps.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 84f4b4c60..38b0ec272 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/jbenet/go-ipfs", - "GoVersion": "devel +ffe33f1f1f17 Tue Nov 25 15:41:33 2014 +1100", + "GoVersion": "go1.3", "Packages": [ "./..." ], From 59f0ffb8c6c040ac50428b70c486c467bf6c60b0 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Sat, 13 Dec 2014 06:34:00 -0800 Subject: [PATCH 09/64] remove noisy statement License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 1 - 1 file changed, 1 deletion(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index b3fc629b9..cae1baa33 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -356,7 +356,6 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm var blkeys []u.Key for _, block := range incoming.Blocks() { blkeys = append(blkeys, block.Key()) - log.Errorf("Got block: %s", block) if err := bs.HasBlock(ctx, block); err != nil { log.Error(err) } From 946d2a96f23a7ab2d96f1ec8d77f72dd7457f7cd Mon Sep 17 00:00:00 2001 From: Jeromy Date: Sun, 14 Dec 2014 03:16:10 +0000 Subject: [PATCH 10/64] add locks to wantlist to avoid race condition --- exchange/bitswap/wantlist/wantlist.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index d57b9d523..0de0ba803 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -3,9 +3,11 @@ package wantlist import ( u "github.com/jbenet/go-ipfs/util" "sort" + "sync" ) type Wantlist struct { + lk sync.RWMutex set map[u.Key]*Entry } @@ -21,6 +23,8 @@ type Entry struct { } func (w *Wantlist) Add(k u.Key, priority int) { + w.lk.Lock() + defer w.lk.Unlock() if _, ok := w.set[k]; ok { return } @@ -31,10 +35,14 @@ func (w *Wantlist) Add(k u.Key, priority int) { } func (w *Wantlist) Remove(k u.Key) { + w.lk.Lock() + defer w.lk.Unlock() delete(w.set, k) } func (w *Wantlist) Contains(k u.Key) bool { + w.lk.RLock() + defer w.lk.RUnlock() _, ok := w.set[k] return ok } @@ -46,6 +54,8 @@ func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priority } func (w *Wantlist) Entries() []*Entry { + w.lk.RLock() + defer w.lk.RUnlock() var es entrySlice for _, e := range w.set { From cfbe92bc8bcddef4f429438a42ce32abec5929a0 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 15 Dec 2014 01:33:04 +0000 Subject: [PATCH 11/64] rewrite sendWantlistToProviders --- exchange/bitswap/bitswap.go | 60 ++++++++++++------------------------- routing/dht/routing.go | 5 ++-- routing/dht/util.go | 44 --------------------------- util/peerset/peerset.go | 48 +++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 87 deletions(-) create mode 100644 util/peerset/peerset.go diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index cae1baa33..ee80df950 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -19,6 +19,7 @@ import ( peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" eventlog "github.com/jbenet/go-ipfs/util/eventlog" + pset "github.com/jbenet/go-ipfs/util/peerset" ) var log = eventlog.Logger("bitswap") @@ -204,57 +205,34 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wantlist) { - provset := make(map[u.Key]peer.Peer) - provcollect := make(chan peer.Peer) - ctx, cancel := context.WithCancel(ctx) defer cancel() - wg := sync.WaitGroup{} - // Get providers for all entries in wantlist (could take a while) - for _, e := range wantlist.Entries() { - wg.Add(1) - go func(k u.Key) { - child, _ := context.WithTimeout(ctx, providerRequestTimeout) - providers := bs.routing.FindProvidersAsync(child, k, maxProvidersPerRequest) - - for prov := range providers { - provcollect <- prov - } - wg.Done() - }(e.Value) - } - - // When all workers finish, close the providers channel - go func() { - wg.Wait() - close(provcollect) - }() - - // Filter out duplicates, - // no need to send our wantlists out twice in a given time period - for { - select { - case p, ok := <-provcollect: - if !ok { - break - } - provset[p.Key()] = p - case <-ctx.Done(): - log.Error("Context cancelled before we got all the providers!") - return - } - } - message := bsmsg.New() message.SetFull(true) for _, e := range bs.wantlist.Entries() { message.AddEntry(e.Value, e.Priority, false) } - for _, prov := range provset { - bs.send(ctx, prov, message) + ps := pset.NewPeerSet() + + // Get providers for all entries in wantlist (could take a while) + wg := sync.WaitGroup{} + for _, e := range wantlist.Entries() { + wg.Add(1) + go func(k u.Key) { + defer wg.Done() + child, _ := context.WithTimeout(ctx, providerRequestTimeout) + providers := bs.routing.FindProvidersAsync(child, k, maxProvidersPerRequest) + + for prov := range providers { + if ps.AddIfSmallerThan(prov, -1) { //Do once per peer + bs.send(ctx, prov, message) + } + } + }(e.Value) } + wg.Wait() } func (bs *bitswap) roundWorker(ctx context.Context) { diff --git a/routing/dht/routing.go b/routing/dht/routing.go index 76260e710..f8036ca38 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -11,6 +11,7 @@ import ( pb "github.com/jbenet/go-ipfs/routing/dht/pb" kb "github.com/jbenet/go-ipfs/routing/kbucket" u "github.com/jbenet/go-ipfs/util" + pset "github.com/jbenet/go-ipfs/util/peerset" ) // asyncQueryBuffer is the size of buffered channels in async queries. This @@ -140,7 +141,7 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int func (dht *IpfsDHT) findProvidersAsyncRoutine(ctx context.Context, key u.Key, count int, peerOut chan peer.Peer) { defer close(peerOut) - ps := newPeerSet() + ps := pset.NewPeerSet() provs := dht.providers.GetProviders(ctx, key) for _, p := range provs { // NOTE: assuming that this list of peers is unique @@ -207,7 +208,7 @@ func (dht *IpfsDHT) findProvidersAsyncRoutine(ctx context.Context, key u.Key, co } } -func (dht *IpfsDHT) addPeerListAsync(ctx context.Context, k u.Key, peers []*pb.Message_Peer, ps *peerSet, count int, out chan peer.Peer) { +func (dht *IpfsDHT) addPeerListAsync(ctx context.Context, k u.Key, peers []*pb.Message_Peer, ps *pset.PeerSet, count int, out chan peer.Peer) { var wg sync.WaitGroup for _, pbp := range peers { wg.Add(1) diff --git a/routing/dht/util.go b/routing/dht/util.go index 00ac38dbc..2b0c1e2a2 100644 --- a/routing/dht/util.go +++ b/routing/dht/util.go @@ -2,8 +2,6 @@ package dht import ( "sync" - - peer "github.com/jbenet/go-ipfs/peer" ) // Pool size is the number of nodes used for group find/set RPC calls @@ -39,45 +37,3 @@ func (c *counter) Size() (s int) { c.mut.Unlock() return } - -// peerSet is a threadsafe set of peers -type peerSet struct { - ps map[string]bool - lk sync.RWMutex -} - -func newPeerSet() *peerSet { - ps := new(peerSet) - ps.ps = make(map[string]bool) - return ps -} - -func (ps *peerSet) Add(p peer.Peer) { - ps.lk.Lock() - ps.ps[string(p.ID())] = true - ps.lk.Unlock() -} - -func (ps *peerSet) Contains(p peer.Peer) bool { - ps.lk.RLock() - _, ok := ps.ps[string(p.ID())] - ps.lk.RUnlock() - return ok -} - -func (ps *peerSet) Size() int { - ps.lk.RLock() - defer ps.lk.RUnlock() - return len(ps.ps) -} - -func (ps *peerSet) AddIfSmallerThan(p peer.Peer, maxsize int) bool { - var success bool - ps.lk.Lock() - if _, ok := ps.ps[string(p.ID())]; !ok && len(ps.ps) < maxsize { - success = true - ps.ps[string(p.ID())] = true - } - ps.lk.Unlock() - return success -} diff --git a/util/peerset/peerset.go b/util/peerset/peerset.go new file mode 100644 index 000000000..c2a488e43 --- /dev/null +++ b/util/peerset/peerset.go @@ -0,0 +1,48 @@ +package peerset + +import ( + peer "github.com/jbenet/go-ipfs/peer" + "sync" +) + +// PeerSet is a threadsafe set of peers +type PeerSet struct { + ps map[string]bool + lk sync.RWMutex +} + +func NewPeerSet() *PeerSet { + ps := new(PeerSet) + ps.ps = make(map[string]bool) + return ps +} + +func (ps *PeerSet) Add(p peer.Peer) { + ps.lk.Lock() + ps.ps[string(p.ID())] = true + ps.lk.Unlock() +} + +func (ps *PeerSet) Contains(p peer.Peer) bool { + ps.lk.RLock() + _, ok := ps.ps[string(p.ID())] + ps.lk.RUnlock() + return ok +} + +func (ps *PeerSet) Size() int { + ps.lk.RLock() + defer ps.lk.RUnlock() + return len(ps.ps) +} + +func (ps *PeerSet) AddIfSmallerThan(p peer.Peer, maxsize int) bool { + var success bool + ps.lk.Lock() + if _, ok := ps.ps[string(p.ID())]; !ok && len(ps.ps) < maxsize { + success = true + ps.ps[string(p.ID())] = true + } + ps.lk.Unlock() + return success +} From 029e305f19a7df97d80b951cfe17692c6f9ce46e Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 16 Dec 2014 02:01:21 +0000 Subject: [PATCH 12/64] tasklist queue for bitswap tasks --- Godeps/Godeps.json | 2 +- exchange/bitswap/bitswap.go | 38 ++---- exchange/bitswap/bitswap_test.go | 12 +- exchange/bitswap/strategy/interface.go | 2 +- exchange/bitswap/strategy/ledgerset.go | 138 ++++++++++++++------ exchange/bitswap/strategy/ledgerset_test.go | 26 ++-- exchange/bitswap/strategy/strategy.go | 18 ++- exchange/bitswap/strategy/tasklist.go | 72 ++++++++++ 8 files changed, 211 insertions(+), 97 deletions(-) create mode 100644 exchange/bitswap/strategy/tasklist.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 38b0ec272..84f4b4c60 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/jbenet/go-ipfs", - "GoVersion": "go1.3", + "GoVersion": "devel +ffe33f1f1f17 Tue Nov 25 15:41:33 2014 +1100", "Packages": [ "./..." ], diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index ee80df950..c0df58551 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -56,8 +56,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout blockstore: bstore, cancelFunc: cancelFunc, notifications: notif, - strategy: strategy.New(nice), - ledgerset: strategy.NewLedgerSet(), + ledgermanager: strategy.NewLedgerManager(bstore, ctx), routing: routing, sender: network, wantlist: wl.New(), @@ -93,7 +92,7 @@ type bitswap struct { // strategy makes decisions about how to interact with partners. strategy strategy.Strategy - ledgerset *strategy.LedgerSet + ledgermanager *strategy.LedgerManager wantlist *wl.Wantlist @@ -197,7 +196,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e // FIXME ensure accounting is handled correctly when // communication fails. May require slightly different API to // get better guarantees. May need shared sequence numbers. - bs.ledgerset.MessageSent(p, message) + bs.ledgermanager.MessageSent(p, message) }(peerToQuery) } wg.Wait() @@ -236,35 +235,24 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan } func (bs *bitswap) roundWorker(ctx context.Context) { - roundTicker := time.NewTicker(roundTime) for { select { case <-ctx.Done(): return - case <-roundTicker.C: - alloc, err := bs.strategy.GetTasks(bandwidthPerRound, bs.ledgerset, bs.blockstore) + case task := <-bs.ledgermanager.GetTaskChan(): + block, err := bs.blockstore.Get(task.Key) if err != nil { - log.Critical("%s", err) + log.Errorf("Expected to have block %s, but it was not found!", task.Key) + continue } - err = bs.processStrategyAllocation(ctx, alloc) - if err != nil { - log.Critical("Error processing strategy allocation: %s", err) - } - } - } -} -func (bs *bitswap) processStrategyAllocation(ctx context.Context, alloc []*strategy.Task) error { - for _, t := range alloc { - for _, block := range t.Blocks { message := bsmsg.New() message.AddBlock(block) - if err := bs.send(ctx, t.Peer, message); err != nil { - return err - } + // TODO: maybe add keys from our wantlist? + + bs.send(ctx, task.Target, message) } } - return nil } // TODO ensure only one active request per key @@ -327,7 +315,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // This call records changes to wantlists, blocks received, // and number of bytes transfered. - bs.ledgerset.MessageReceived(p, incoming) + bs.ledgermanager.MessageReceived(p, incoming) // TODO: this is bad, and could be easily abused. // Should only track *useful* messages in ledger @@ -352,7 +340,7 @@ func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { for _, k := range bkeys { message.AddEntry(k, 0, true) } - for _, p := range bs.ledgerset.Peers() { + for _, p := range bs.ledgermanager.Peers() { err := bs.send(ctx, p, message) if err != nil { log.Errorf("Error sending message: %s", err) @@ -372,7 +360,7 @@ func (bs *bitswap) send(ctx context.Context, p peer.Peer, m bsmsg.BitSwapMessage if err := bs.sender.SendMessage(ctx, p, m); err != nil { return err } - return bs.ledgerset.MessageSent(p, m) + return bs.ledgermanager.MessageSent(p, m) } func (bs *bitswap) Close() error { diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 9bf71dea6..2c04b0508 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -26,7 +26,7 @@ func TestClose(t *testing.T) { vnet := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rout := mockrouting.NewServer() sesgen := NewSessionGenerator(vnet, rout) - defer sesgen.Stop() + defer sesgen.Close() bgen := blocksutil.NewBlockGenerator() block := bgen.Next() @@ -41,7 +41,7 @@ func TestGetBlockTimeout(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() g := NewSessionGenerator(net, rs) - defer g.Stop() + defer g.Close() self := g.Next() @@ -59,7 +59,7 @@ func TestProviderForKeyButNetworkCannotFind(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() g := NewSessionGenerator(net, rs) - defer g.Stop() + defer g.Close() block := blocks.NewBlock([]byte("block")) rs.Client(testutil.NewPeerWithIDString("testing")).Provide(context.Background(), block.Key()) // but not on network @@ -83,7 +83,7 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { rs := mockrouting.NewServer() block := blocks.NewBlock([]byte("block")) g := NewSessionGenerator(net, rs) - defer g.Stop() + defer g.Close() hasBlock := g.Next() defer hasBlock.Exchange.Close() @@ -137,7 +137,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() sg := NewSessionGenerator(net, rs) - defer sg.Stop() + defer sg.Close() bg := blocksutil.NewBlockGenerator() t.Log("Test a few nodes trying to get one file with a lot of blocks") @@ -203,7 +203,7 @@ func TestSendToWantingPeer(t *testing.T) { net := tn.VirtualNetwork(delay.Fixed(kNetworkDelay)) rs := mockrouting.NewServer() sg := NewSessionGenerator(net, rs) - defer sg.Stop() + defer sg.Close() bg := blocksutil.NewBlockGenerator() oldVal := rebroadcastDelay diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go index 54af581f7..62cd77b8a 100644 --- a/exchange/bitswap/strategy/interface.go +++ b/exchange/bitswap/strategy/interface.go @@ -8,5 +8,5 @@ type Strategy interface { // Seed initializes the decider to a deterministic state Seed(int64) - GetTasks(bandwidth int, ledgers *LedgerSet, bs bstore.Blockstore) ([]*Task, error) + GetTasks(bandwidth int, ledgers *LedgerManager, bs bstore.Blockstore) ([]*Task, error) } diff --git a/exchange/bitswap/strategy/ledgerset.go b/exchange/bitswap/strategy/ledgerset.go index b5f03ae65..92808d2f0 100644 --- a/exchange/bitswap/strategy/ledgerset.go +++ b/exchange/bitswap/strategy/ledgerset.go @@ -3,6 +3,9 @@ package strategy import ( "sync" + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + + bstore "github.com/jbenet/go-ipfs/blocks/blockstore" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" @@ -15,24 +18,62 @@ type ledgerMap map[peerKey]*ledger // FIXME share this externally type peerKey u.Key -type LedgerSet struct { - lock sync.RWMutex - ledgerMap ledgerMap +type LedgerManager struct { + lock sync.RWMutex + ledgerMap ledgerMap + bs bstore.Blockstore + tasklist *TaskList + taskOut chan *Task + workSignal chan struct{} + ctx context.Context } -func NewLedgerSet() *LedgerSet { - return &LedgerSet{ - ledgerMap: make(ledgerMap), +func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager { + lm := &LedgerManager{ + ledgerMap: make(ledgerMap), + bs: bs, + tasklist: NewTaskList(), + taskOut: make(chan *Task, 4), + workSignal: make(chan struct{}), + ctx: ctx, + } + go lm.taskWorker() + return lm +} + +func (lm *LedgerManager) taskWorker() { + for { + nextTask := lm.tasklist.GetNext() + if nextTask == nil { + // No tasks in the list? + // Wait until there are! + select { + case <-lm.ctx.Done(): + return + case <-lm.workSignal: + } + continue + } + + select { + case <-lm.ctx.Done(): + return + case lm.taskOut <- nextTask: + } } } +func (lm *LedgerManager) GetTaskChan() <-chan *Task { + return lm.taskOut +} + // Returns a slice of Peers with whom the local node has active sessions -func (ls *LedgerSet) Peers() []peer.Peer { - ls.lock.RLock() - defer ls.lock.RUnlock() +func (lm *LedgerManager) Peers() []peer.Peer { + lm.lock.RLock() + defer lm.lock.RUnlock() response := make([]peer.Peer, 0) - for _, ledger := range ls.ledgerMap { + for _, ledger := range lm.ledgerMap { response = append(response, ledger.Partner) } return response @@ -40,43 +81,55 @@ func (ls *LedgerSet) Peers() []peer.Peer { // BlockIsWantedByPeer returns true if peer wants the block given by this // key -func (ls *LedgerSet) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { - ls.lock.RLock() - defer ls.lock.RUnlock() +func (lm *LedgerManager) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { + lm.lock.RLock() + defer lm.lock.RUnlock() - ledger := ls.ledger(p) + ledger := lm.ledger(p) return ledger.WantListContains(k) } // MessageReceived performs book-keeping. Returns error if passed invalid // arguments. -func (ls *LedgerSet) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { - ls.lock.Lock() - defer ls.lock.Unlock() +func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { + lm.lock.Lock() + defer lm.lock.Unlock() - // TODO find a more elegant way to handle this check - /* - if p == nil { - return errors.New("Strategy received nil peer") - } - if m == nil { - return errors.New("Strategy received nil message") - } - */ - l := ls.ledger(p) + l := lm.ledger(p) if m.Full() { l.wantList = wl.New() } for _, e := range m.Wantlist() { if e.Cancel { l.CancelWant(e.Key) + lm.tasklist.Cancel(e.Key, p) } else { l.Wants(e.Key, e.Priority) + lm.tasklist.Add(e.Key, e.Priority, p) + + // Signal task generation to restart (if stopped!) + select { + case lm.workSignal <- struct{}{}: + default: + } } } + for _, block := range m.Blocks() { // FIXME extract blocks.NumBytes(block) or block.NumBytes() method l.ReceivedBytes(len(block.Data)) + for _, l := range lm.ledgerMap { + if l.WantListContains(block.Key()) { + lm.tasklist.Add(block.Key(), 1, l.Partner) + + // Signal task generation to restart (if stopped!) + select { + case lm.workSignal <- struct{}{}: + default: + } + + } + } } return nil } @@ -87,39 +140,40 @@ func (ls *LedgerSet) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error // inconsistent. Would need to ensure that Sends and acknowledgement of the // send happen atomically -func (ls *LedgerSet) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { - ls.lock.Lock() - defer ls.lock.Unlock() +func (lm *LedgerManager) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { + lm.lock.Lock() + defer lm.lock.Unlock() - l := ls.ledger(p) + l := lm.ledger(p) for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) + lm.tasklist.Cancel(block.Key(), p) } return nil } -func (ls *LedgerSet) NumBytesSentTo(p peer.Peer) uint64 { - ls.lock.RLock() - defer ls.lock.RUnlock() +func (lm *LedgerManager) NumBytesSentTo(p peer.Peer) uint64 { + lm.lock.RLock() + defer lm.lock.RUnlock() - return ls.ledger(p).Accounting.BytesSent + return lm.ledger(p).Accounting.BytesSent } -func (ls *LedgerSet) NumBytesReceivedFrom(p peer.Peer) uint64 { - ls.lock.RLock() - defer ls.lock.RUnlock() +func (lm *LedgerManager) NumBytesReceivedFrom(p peer.Peer) uint64 { + lm.lock.RLock() + defer lm.lock.RUnlock() - return ls.ledger(p).Accounting.BytesRecv + return lm.ledger(p).Accounting.BytesRecv } // ledger lazily instantiates a ledger -func (ls *LedgerSet) ledger(p peer.Peer) *ledger { - l, ok := ls.ledgerMap[peerKey(p.Key())] +func (lm *LedgerManager) ledger(p peer.Peer) *ledger { + l, ok := lm.ledgerMap[peerKey(p.Key())] if !ok { l = newLedger(p) - ls.ledgerMap[peerKey(p.Key())] = l + lm.ledgerMap[peerKey(p.Key())] = l } return l } diff --git a/exchange/bitswap/strategy/ledgerset_test.go b/exchange/bitswap/strategy/ledgerset_test.go index 795752a12..819489799 100644 --- a/exchange/bitswap/strategy/ledgerset_test.go +++ b/exchange/bitswap/strategy/ledgerset_test.go @@ -4,28 +4,30 @@ import ( "strings" "testing" + "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + blocks "github.com/jbenet/go-ipfs/blocks" message "github.com/jbenet/go-ipfs/exchange/bitswap/message" peer "github.com/jbenet/go-ipfs/peer" testutil "github.com/jbenet/go-ipfs/util/testutil" ) -type peerAndLedgerset struct { +type peerAndLedgermanager struct { peer.Peer - ls *LedgerSet + ls *LedgerManager } -func newPeerAndLedgerset(idStr string) peerAndLedgerset { - return peerAndLedgerset{ +func newPeerAndLedgermanager(idStr string) peerAndLedgermanager { + return peerAndLedgermanager{ Peer: testutil.NewPeerWithIDString(idStr), //Strategy: New(true), - ls: NewLedgerSet(), + ls: NewLedgerManager(nil, context.TODO()), } } func TestConsistentAccounting(t *testing.T) { - sender := newPeerAndLedgerset("Ernie") - receiver := newPeerAndLedgerset("Bert") + sender := newPeerAndLedgermanager("Ernie") + receiver := newPeerAndLedgermanager("Bert") // Send messages from Ernie to Bert for i := 0; i < 1000; i++ { @@ -56,8 +58,8 @@ func TestConsistentAccounting(t *testing.T) { } func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { - beggar := newPeerAndLedgerset("can't be chooser") - chooser := newPeerAndLedgerset("chooses JIF") + beggar := newPeerAndLedgermanager("can't be chooser") + chooser := newPeerAndLedgermanager("chooses JIF") block := blocks.NewBlock([]byte("data wanted by beggar")) @@ -74,8 +76,8 @@ func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { func TestPeerIsAddedToPeersWhenMessageReceivedOrSent(t *testing.T) { - sanfrancisco := newPeerAndLedgerset("sf") - seattle := newPeerAndLedgerset("sea") + sanfrancisco := newPeerAndLedgermanager("sf") + seattle := newPeerAndLedgermanager("sea") m := message.New() @@ -95,7 +97,7 @@ func TestPeerIsAddedToPeersWhenMessageReceivedOrSent(t *testing.T) { } } -func peerIsPartner(p peer.Peer, ls *LedgerSet) bool { +func peerIsPartner(p peer.Peer, ls *LedgerManager) bool { for _, partner := range ls.Peers() { if partner.Key() == p.Key() { return true diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go index ff7f4d74d..5b0d9830d 100644 --- a/exchange/bitswap/strategy/strategy.go +++ b/exchange/bitswap/strategy/strategy.go @@ -1,15 +1,16 @@ package strategy import ( - blocks "github.com/jbenet/go-ipfs/blocks" - bstore "github.com/jbenet/go-ipfs/blocks/blockstore" - wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" - peer "github.com/jbenet/go-ipfs/peer" + //blocks "github.com/jbenet/go-ipfs/blocks" + //bstore "github.com/jbenet/go-ipfs/blocks/blockstore" + //wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" + //peer "github.com/jbenet/go-ipfs/peer" 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 @@ -29,12 +30,7 @@ type strategist struct { strategyFunc } -type Task struct { - Peer peer.Peer - Blocks []*blocks.Block -} - -func (s *strategist) GetTasks(bandwidth int, ledgers *LedgerSet, bs bstore.Blockstore) ([]*Task, error) { +func (s *strategist) GetTasks(bandwidth int, ledgers *LedgerManager, bs bstore.Blockstore) ([]*Task, error) { var tasks []*Task ledgers.lock.RLock() @@ -87,3 +83,5 @@ func test() {} func (s *strategist) Seed(int64) { // TODO } + +*/ diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/tasklist.go new file mode 100644 index 000000000..fb8c64109 --- /dev/null +++ b/exchange/bitswap/strategy/tasklist.go @@ -0,0 +1,72 @@ +package strategy + +import ( + peer "github.com/jbenet/go-ipfs/peer" + u "github.com/jbenet/go-ipfs/util" +) + +// TODO: at some point, the strategy needs to plug in here +// to help decide how to sort tasks (on add) and how to select +// tasks (on getnext). For now, we are assuming a dumb/nice strategy. +type TaskList struct { + tasks []*Task + taskmap map[u.Key]*Task +} + +func NewTaskList() *TaskList { + return &TaskList{ + taskmap: make(map[u.Key]*Task), + } +} + +type Task struct { + Key u.Key + Target peer.Peer + theirPriority int +} + +// Add currently adds a new task to the end of the list +// TODO: make this into a priority queue +func (tl *TaskList) Add(block u.Key, priority int, to peer.Peer) { + if task, ok := tl.taskmap[to.Key()+block]; ok { + // TODO: when priority queue is implemented, + // rearrange this Task + task.theirPriority = priority + return + } + task := &Task{ + Key: block, + Target: to, + theirPriority: priority, + } + tl.tasks = append(tl.tasks, task) + tl.taskmap[to.Key()+block] = task +} + +// GetNext returns the next task to be performed by bitswap +// the task is then removed from the list +func (tl *TaskList) GetNext() *Task { + var out *Task + for len(tl.tasks) > 0 { + // TODO: instead of zero, use exponential distribution + // it will help reduce the chance of receiving + // the same block from multiple peers + out = tl.tasks[0] + tl.tasks = tl.tasks[1:] + delete(tl.taskmap, out.Target.Key()+out.Key) + // Filter out blocks that have been cancelled + if out.theirPriority >= 0 { + break + } + } + + return out +} + +// Cancel lazily cancels the sending of a block to a given peer +func (tl *TaskList) Cancel(k u.Key, p peer.Peer) { + t, ok := tl.taskmap[p.Key()+k] + if ok { + t.theirPriority = -1 + } +} From 90a30961ded5283afdf08ca72b62dbc6b81059c2 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 16 Dec 2014 02:14:30 +0000 Subject: [PATCH 13/64] renaming and removing empty strategy file --- .../{ledgerset.go => ledgermanager.go} | 2 + ...edgerset_test.go => ledgermanager_test.go} | 0 exchange/bitswap/strategy/strategy.go | 87 ------------------- 3 files changed, 2 insertions(+), 87 deletions(-) rename exchange/bitswap/strategy/{ledgerset.go => ledgermanager.go} (99%) rename exchange/bitswap/strategy/{ledgerset_test.go => ledgermanager_test.go} (100%) delete mode 100644 exchange/bitswap/strategy/strategy.go diff --git a/exchange/bitswap/strategy/ledgerset.go b/exchange/bitswap/strategy/ledgermanager.go similarity index 99% rename from exchange/bitswap/strategy/ledgerset.go rename to exchange/bitswap/strategy/ledgermanager.go index 92808d2f0..4712b6a3e 100644 --- a/exchange/bitswap/strategy/ledgerset.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -12,6 +12,8 @@ import ( u "github.com/jbenet/go-ipfs/util" ) +var log = u.Logger("strategy") + // LedgerMap lists Ledgers by their Partner key. type ledgerMap map[peerKey]*ledger diff --git a/exchange/bitswap/strategy/ledgerset_test.go b/exchange/bitswap/strategy/ledgermanager_test.go similarity index 100% rename from exchange/bitswap/strategy/ledgerset_test.go rename to exchange/bitswap/strategy/ledgermanager_test.go diff --git a/exchange/bitswap/strategy/strategy.go b/exchange/bitswap/strategy/strategy.go deleted file mode 100644 index 5b0d9830d..000000000 --- a/exchange/bitswap/strategy/strategy.go +++ /dev/null @@ -1,87 +0,0 @@ -package strategy - -import ( - //blocks "github.com/jbenet/go-ipfs/blocks" - //bstore "github.com/jbenet/go-ipfs/blocks/blockstore" - //wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" - //peer "github.com/jbenet/go-ipfs/peer" - 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 -func New(nice bool) Strategy { - var stratFunc strategyFunc - if nice { - stratFunc = yesManStrategy - } else { - stratFunc = standardStrategy - } - return &strategist{ - strategyFunc: stratFunc, - } -} - -type strategist struct { - strategyFunc -} - -func (s *strategist) GetTasks(bandwidth int, ledgers *LedgerManager, bs bstore.Blockstore) ([]*Task, error) { - var tasks []*Task - - ledgers.lock.RLock() - var partners []peer.Peer - for _, ledger := range ledgers.ledgerMap { - if s.strategyFunc(ledger) { - partners = append(partners, ledger.Partner) - } - } - ledgers.lock.RUnlock() - if len(partners) == 0 { - return nil, nil - } - - bandwidthPerPeer := bandwidth / len(partners) - for _, p := range partners { - blksForPeer, err := s.getSendableBlocks(ledgers.ledger(p).wantList, bs, bandwidthPerPeer) - if err != nil { - return nil, err - } - tasks = append(tasks, &Task{ - Peer: p, - Blocks: blksForPeer, - }) - } - - return tasks, nil -} - -func (s *strategist) getSendableBlocks(wantlist *wl.Wantlist, bs bstore.Blockstore, bw int) ([]*blocks.Block, error) { - var outblocks []*blocks.Block - for _, e := range wantlist.Entries() { - block, err := bs.Get(e.Value) - if err == bstore.ErrNotFound { - continue - } - if err != nil { - return nil, err - } - outblocks = append(outblocks, block) - bw -= len(block.Data) - if bw <= 0 { - break - } - } - return outblocks, nil -} - -func test() {} -func (s *strategist) Seed(int64) { - // TODO -} - -*/ From 2240272d850a245c55431215c84107fd69be921e Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 16 Dec 2014 04:01:15 +0000 Subject: [PATCH 14/64] change Provide RPC to not wait for an ACK, improves performance of 'Add' operations --- merkledag/merkledag.go | 4 +++- routing/dht/dht.go | 5 +---- routing/dht/dht_net.go | 23 ++++++++++++++++++++++- routing/mock/mockrouting_test.go | 4 ++++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index d22e3a396..3d8916b03 100644 --- a/merkledag/merkledag.go +++ b/merkledag/merkledag.go @@ -2,6 +2,7 @@ package merkledag import ( + "bytes" "fmt" "sync" "time" @@ -294,8 +295,9 @@ func FetchGraph(ctx context.Context, root *Node, serv DAGService) chan struct{} // returns the indexes of any links pointing to it func FindLinks(n *Node, k u.Key) []int { var out []int + keybytes := []byte(k) for i, lnk := range n.Links { - if u.Key(lnk.Hash) == k { + if bytes.Equal([]byte(lnk.Hash), keybytes) { out = append(out, i) } } diff --git a/routing/dht/dht.go b/routing/dht/dht.go index 5a68bd759..6e49c84cf 100644 --- a/routing/dht/dht.go +++ b/routing/dht/dht.go @@ -120,15 +120,12 @@ func (dht *IpfsDHT) putProvider(ctx context.Context, p peer.Peer, key string) er // add self as the provider pmes.ProviderPeers = pb.PeersToPBPeers(dht.network, []peer.Peer{dht.self}) - rpmes, err := dht.sendRequest(ctx, p, pmes) + err := dht.sendMessage(ctx, p, pmes) if err != nil { return err } log.Debugf("%s putProvider: %s for %s", dht.self, p, u.Key(key)) - if rpmes.GetKey() != pmes.GetKey() { - return errors.New("provider not added correctly") - } return nil } diff --git a/routing/dht/dht_net.go b/routing/dht/dht_net.go index d1898f15c..6e46b4de6 100644 --- a/routing/dht/dht_net.go +++ b/routing/dht/dht_net.go @@ -8,8 +8,8 @@ import ( peer "github.com/jbenet/go-ipfs/peer" pb "github.com/jbenet/go-ipfs/routing/dht/pb" - ggio "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/io" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + ggio "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/gogoprotobuf/io" ) // handleNewStream implements the inet.StreamHandler @@ -102,3 +102,24 @@ func (dht *IpfsDHT) sendRequest(ctx context.Context, p peer.Peer, pmes *pb.Messa log.Event(ctx, "dhtReceivedMessage", dht.self, p, rpmes) return rpmes, nil } + +// sendMessage sends out a message +func (dht *IpfsDHT) sendMessage(ctx context.Context, p peer.Peer, pmes *pb.Message) error { + + log.Debugf("%s dht starting stream", dht.self) + s, err := dht.network.NewStream(inet.ProtocolDHT, p) + if err != nil { + return err + } + defer s.Close() + + w := ggio.NewDelimitedWriter(s) + + log.Debugf("%s writing", dht.self) + if err := w.WriteMsg(pmes); err != nil { + return err + } + log.Event(ctx, "dhtSentMessage", dht.self, p, pmes) + log.Debugf("%s done", dht.self) + return nil +} diff --git a/routing/mock/mockrouting_test.go b/routing/mock/mockrouting_test.go index 6700cd8ed..44b1b52bd 100644 --- a/routing/mock/mockrouting_test.go +++ b/routing/mock/mockrouting_test.go @@ -36,6 +36,9 @@ func TestClientFindProviders(t *testing.T) { if err != nil { t.Fatal(err) } + + // This is bad... but simulating networks is hard + time.Sleep(time.Millisecond * 300) max := 100 providersFromHashTable, err := rs.Client(peer).FindProviders(context.Background(), k) @@ -160,6 +163,7 @@ func TestValidAfter(t *testing.T) { if err != nil { t.Fatal(err) } + t.Log("providers", providers) if len(providers) != 1 { t.Fail() } From 6389bfda6b5f7695524210ac152ac967918b8034 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 16 Dec 2014 04:52:55 +0000 Subject: [PATCH 15/64] some cleanup before CR --- Godeps/Godeps.json | 2 +- exchange/bitswap/bitswap.go | 11 ++++------- exchange/bitswap/strategy/interface.go | 12 ------------ exchange/bitswap/wantlist/wantlist.go | 13 +++++++++++++ 4 files changed, 18 insertions(+), 20 deletions(-) delete mode 100644 exchange/bitswap/strategy/interface.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 84f4b4c60..38b0ec272 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/jbenet/go-ipfs", - "GoVersion": "devel +ffe33f1f1f17 Tue Nov 25 15:41:33 2014 +1100", + "GoVersion": "go1.3", "Packages": [ "./..." ], diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index c0df58551..eb59542e4 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -32,10 +32,6 @@ var providerRequestTimeout = time.Second * 10 var hasBlockTimeout = time.Second * 15 var rebroadcastDelay = time.Second * 10 -const roundTime = time.Second / 2 - -var bandwidthPerRound = 500000 - // New initializes a BitSwap instance that communicates over the // provided BitSwapNetwork. This function registers the returned instance as // the network delegate. @@ -64,7 +60,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout } network.SetDelegate(bs) go bs.clientWorker(ctx) - go bs.roundWorker(ctx) + go bs.taskWorker(ctx) return bs } @@ -90,7 +86,8 @@ type bitswap struct { batchRequests chan []u.Key // strategy makes decisions about how to interact with partners. - strategy strategy.Strategy + // TODO: strategy commented out until we have a use for it again + //strategy strategy.Strategy ledgermanager *strategy.LedgerManager @@ -234,7 +231,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan wg.Wait() } -func (bs *bitswap) roundWorker(ctx context.Context) { +func (bs *bitswap) taskWorker(ctx context.Context) { for { select { case <-ctx.Done(): diff --git a/exchange/bitswap/strategy/interface.go b/exchange/bitswap/strategy/interface.go deleted file mode 100644 index 62cd77b8a..000000000 --- a/exchange/bitswap/strategy/interface.go +++ /dev/null @@ -1,12 +0,0 @@ -package strategy - -import ( - bstore "github.com/jbenet/go-ipfs/blocks/blockstore" -) - -type Strategy interface { - // Seed initializes the decider to a deterministic state - Seed(int64) - - GetTasks(bandwidth int, ledgers *LedgerManager, bs bstore.Blockstore) ([]*Task, error) -} diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 0de0ba803..e20bb4457 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -56,6 +56,19 @@ func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priorit func (w *Wantlist) Entries() []*Entry { w.lk.RLock() defer w.lk.RUnlock() + + var es entrySlice + + for _, e := range w.set { + es = append(es, e) + } + sort.Sort(es) + return es +} + +func (w *Wantlist) SortedEntries() []*Entry { + w.lk.RLock() + defer w.lk.RUnlock() var es entrySlice for _, e := range w.set { From 86c438b6e1bafa60406409b4099545b4a9d44878 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 15 Dec 2014 22:38:57 -0800 Subject: [PATCH 16/64] refactor() message API performing CR in the form of a PR. Let me know what you think. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 6 ++--- exchange/bitswap/message/message.go | 22 +++++++++++++++---- exchange/bitswap/message/message_test.go | 20 ++++++++--------- .../bitswap/strategy/ledgermanager_test.go | 2 +- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index eb59542e4..9b92bb0aa 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -169,7 +169,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } message := bsmsg.New() for _, wanted := range bs.wantlist.Entries() { - message.AddEntry(wanted.Value, wanted.Priority, false) + message.AddEntry(wanted.Value, wanted.Priority) } wg := sync.WaitGroup{} for peerToQuery := range peers { @@ -207,7 +207,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan message := bsmsg.New() message.SetFull(true) for _, e := range bs.wantlist.Entries() { - message.AddEntry(e.Value, e.Priority, false) + message.AddEntry(e.Value, e.Priority) } ps := pset.NewPeerSet() @@ -335,7 +335,7 @@ func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { message := bsmsg.New() message.SetFull(false) for _, k := range bkeys { - message.AddEntry(k, 0, true) + message.Cancel(k) } for _, p := range bs.ledgermanager.Peers() { err := bs.send(ctx, p, message) diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index b636e2024..478d8e258 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -24,7 +24,9 @@ type BitSwapMessage interface { Blocks() []*blocks.Block // AddEntry adds an entry to the Wantlist. - AddEntry(key u.Key, priority int, cancel bool) + AddEntry(key u.Key, priority int) + + Cancel(key u.Key) // Sets whether or not the contained wantlist represents the entire wantlist // true = full wantlist @@ -50,6 +52,10 @@ type impl struct { } func New() BitSwapMessage { + return newMsg() +} + +func newMsg() *impl { return &impl{ blocks: make(map[u.Key]*blocks.Block), wantlist: make(map[u.Key]*Entry), @@ -64,10 +70,10 @@ type Entry struct { } func newMessageFromProto(pbm pb.Message) BitSwapMessage { - m := New() + m := newMsg() m.SetFull(pbm.GetWantlist().GetFull()) for _, e := range pbm.GetWantlist().GetEntries() { - m.AddEntry(u.Key(e.GetBlock()), int(e.GetPriority()), e.GetCancel()) + m.addEntry(u.Key(e.GetBlock()), int(e.GetPriority()), e.GetCancel()) } for _, d := range pbm.GetBlocks() { b := blocks.NewBlock(d) @@ -100,7 +106,15 @@ func (m *impl) Blocks() []*blocks.Block { return bs } -func (m *impl) AddEntry(k u.Key, priority int, cancel bool) { +func (m *impl) Cancel(k u.Key) { + m.addEntry(k, 0, true) +} + +func (m *impl) AddEntry(k u.Key, priority int) { + m.addEntry(k, priority, false) +} + +func (m *impl) addEntry(k u.Key, priority int, cancel bool) { e, exists := m.wantlist[k] if exists { e.Priority = priority diff --git a/exchange/bitswap/message/message_test.go b/exchange/bitswap/message/message_test.go index 29eb6eb4e..a0df38c0b 100644 --- a/exchange/bitswap/message/message_test.go +++ b/exchange/bitswap/message/message_test.go @@ -14,7 +14,7 @@ import ( func TestAppendWanted(t *testing.T) { const str = "foo" m := New() - m.AddEntry(u.Key(str), 1, false) + m.AddEntry(u.Key(str), 1) if !wantlistContains(m.ToProto().GetWantlist(), str) { t.Fail() @@ -63,7 +63,7 @@ func TestWantlist(t *testing.T) { keystrs := []string{"foo", "bar", "baz", "bat"} m := New() for _, s := range keystrs { - m.AddEntry(u.Key(s), 1, false) + m.AddEntry(u.Key(s), 1) } exported := m.Wantlist() @@ -86,7 +86,7 @@ func TestCopyProtoByValue(t *testing.T) { const str = "foo" m := New() protoBeforeAppend := m.ToProto() - m.AddEntry(u.Key(str), 1, false) + m.AddEntry(u.Key(str), 1) if wantlistContains(protoBeforeAppend.GetWantlist(), str) { t.Fail() } @@ -94,11 +94,11 @@ func TestCopyProtoByValue(t *testing.T) { func TestToNetFromNetPreservesWantList(t *testing.T) { original := New() - original.AddEntry(u.Key("M"), 1, false) - original.AddEntry(u.Key("B"), 1, false) - original.AddEntry(u.Key("D"), 1, false) - original.AddEntry(u.Key("T"), 1, false) - original.AddEntry(u.Key("F"), 1, false) + original.AddEntry(u.Key("M"), 1) + original.AddEntry(u.Key("B"), 1) + original.AddEntry(u.Key("D"), 1) + original.AddEntry(u.Key("T"), 1) + original.AddEntry(u.Key("F"), 1) var buf bytes.Buffer if err := original.ToNet(&buf); err != nil { @@ -174,8 +174,8 @@ func TestDuplicates(t *testing.T) { b := blocks.NewBlock([]byte("foo")) msg := New() - msg.AddEntry(b.Key(), 1, false) - msg.AddEntry(b.Key(), 1, false) + msg.AddEntry(b.Key(), 1) + msg.AddEntry(b.Key(), 1) if len(msg.Wantlist()) != 1 { t.Fatal("Duplicate in BitSwapMessage") } diff --git a/exchange/bitswap/strategy/ledgermanager_test.go b/exchange/bitswap/strategy/ledgermanager_test.go index 819489799..f2a98cb77 100644 --- a/exchange/bitswap/strategy/ledgermanager_test.go +++ b/exchange/bitswap/strategy/ledgermanager_test.go @@ -64,7 +64,7 @@ func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { block := blocks.NewBlock([]byte("data wanted by beggar")) messageFromBeggarToChooser := message.New() - messageFromBeggarToChooser.AddEntry(block.Key(), 1, false) + messageFromBeggarToChooser.AddEntry(block.Key(), 1) chooser.ls.MessageReceived(beggar.Peer, messageFromBeggarToChooser) // for this test, doesn't matter if you record that beggar sent From 29ef238fa36f1997da7e8c56ae348b0c0c19015e Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 15 Dec 2014 22:42:38 -0800 Subject: [PATCH 17/64] remove dead code License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledger.go | 9 +++++++ exchange/bitswap/strategy/math.go | 34 -------------------------- exchange/bitswap/strategy/math_test.go | 17 ------------- 3 files changed, 9 insertions(+), 51 deletions(-) delete mode 100644 exchange/bitswap/strategy/math.go delete mode 100644 exchange/bitswap/strategy/math_test.go diff --git a/exchange/bitswap/strategy/ledger.go b/exchange/bitswap/strategy/ledger.go index 684d383ef..649c1e73e 100644 --- a/exchange/bitswap/strategy/ledger.go +++ b/exchange/bitswap/strategy/ledger.go @@ -46,6 +46,15 @@ type ledger struct { sentToPeer map[u.Key]time.Time } +type debtRatio struct { + BytesSent uint64 + BytesRecv uint64 +} + +func (dr *debtRatio) Value() float64 { + return float64(dr.BytesSent) / float64(dr.BytesRecv+1) +} + func (l *ledger) SentBytes(n int) { l.exchangeCount++ l.lastExchange = time.Now() diff --git a/exchange/bitswap/strategy/math.go b/exchange/bitswap/strategy/math.go deleted file mode 100644 index c5339e5b3..000000000 --- a/exchange/bitswap/strategy/math.go +++ /dev/null @@ -1,34 +0,0 @@ -package strategy - -import ( - "math" - "math/rand" -) - -type strategyFunc func(*ledger) bool - -// TODO avoid using rand.Float64 method. it uses a singleton lock and may cause -// performance issues. Instead, instantiate a rand struct and use that to call -// Float64() -func standardStrategy(l *ledger) bool { - return rand.Float64() <= probabilitySend(l.Accounting.Value()) -} - -func yesManStrategy(l *ledger) bool { - return true -} - -func probabilitySend(ratio float64) float64 { - x := 1 + math.Exp(6-3*ratio) - y := 1 / x - return 1 - y -} - -type debtRatio struct { - BytesSent uint64 - BytesRecv uint64 -} - -func (dr *debtRatio) Value() float64 { - return float64(dr.BytesSent) / float64(dr.BytesRecv+1) -} diff --git a/exchange/bitswap/strategy/math_test.go b/exchange/bitswap/strategy/math_test.go deleted file mode 100644 index 58092bc09..000000000 --- a/exchange/bitswap/strategy/math_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package strategy - -import ( - "testing" -) - -func TestProbabilitySendDecreasesAsRatioIncreases(t *testing.T) { - grateful := debtRatio{BytesSent: 0, BytesRecv: 10000} - pWhenGrateful := probabilitySend(grateful.Value()) - - abused := debtRatio{BytesSent: 10000, BytesRecv: 0} - pWhenAbused := probabilitySend(abused.Value()) - - if pWhenGrateful < pWhenAbused { - t.Fail() - } -} From 1bced710b15636654c8fcc34975ac743d29f54be Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 15 Dec 2014 22:46:10 -0800 Subject: [PATCH 18/64] queue-like naming License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 6 +++--- exchange/bitswap/strategy/tasklist.go | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 4712b6a3e..73cd94711 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -45,7 +45,7 @@ func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager func (lm *LedgerManager) taskWorker() { for { - nextTask := lm.tasklist.GetNext() + nextTask := lm.tasklist.Pop() if nextTask == nil { // No tasks in the list? // Wait until there are! @@ -107,7 +107,7 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er lm.tasklist.Cancel(e.Key, p) } else { l.Wants(e.Key, e.Priority) - lm.tasklist.Add(e.Key, e.Priority, p) + lm.tasklist.Push(e.Key, e.Priority, p) // Signal task generation to restart (if stopped!) select { @@ -122,7 +122,7 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er l.ReceivedBytes(len(block.Data)) for _, l := range lm.ledgerMap { if l.WantListContains(block.Key()) { - lm.tasklist.Add(block.Key(), 1, l.Partner) + lm.tasklist.Push(block.Key(), 1, l.Partner) // Signal task generation to restart (if stopped!) select { diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/tasklist.go index fb8c64109..f0a1b7d00 100644 --- a/exchange/bitswap/strategy/tasklist.go +++ b/exchange/bitswap/strategy/tasklist.go @@ -25,9 +25,9 @@ type Task struct { theirPriority int } -// Add currently adds a new task to the end of the list +// Push currently adds a new task to the end of the list // TODO: make this into a priority queue -func (tl *TaskList) Add(block u.Key, priority int, to peer.Peer) { +func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[to.Key()+block]; ok { // TODO: when priority queue is implemented, // rearrange this Task @@ -43,9 +43,9 @@ func (tl *TaskList) Add(block u.Key, priority int, to peer.Peer) { tl.taskmap[to.Key()+block] = task } -// GetNext returns the next task to be performed by bitswap -// the task is then removed from the list -func (tl *TaskList) GetNext() *Task { +// Pop returns the next task to be performed by bitswap the task is then +// removed from the list +func (tl *TaskList) Pop() *Task { var out *Task for len(tl.tasks) > 0 { // TODO: instead of zero, use exponential distribution From f028c44fd205141c32a3ef3ac32d15c3365fefbb Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 15 Dec 2014 22:52:27 -0800 Subject: [PATCH 19/64] name findOrCreate License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 73cd94711..d6699e9f0 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -87,7 +87,7 @@ func (lm *LedgerManager) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { lm.lock.RLock() defer lm.lock.RUnlock() - ledger := lm.ledger(p) + ledger := lm.findOrCreate(p) return ledger.WantListContains(k) } @@ -97,7 +97,7 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er lm.lock.Lock() defer lm.lock.Unlock() - l := lm.ledger(p) + l := lm.findOrCreate(p) if m.Full() { l.wantList = wl.New() } @@ -146,7 +146,7 @@ func (lm *LedgerManager) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error lm.lock.Lock() defer lm.lock.Unlock() - l := lm.ledger(p) + l := lm.findOrCreate(p) for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) @@ -160,18 +160,18 @@ func (lm *LedgerManager) NumBytesSentTo(p peer.Peer) uint64 { lm.lock.RLock() defer lm.lock.RUnlock() - return lm.ledger(p).Accounting.BytesSent + return lm.findOrCreate(p).Accounting.BytesSent } func (lm *LedgerManager) NumBytesReceivedFrom(p peer.Peer) uint64 { lm.lock.RLock() defer lm.lock.RUnlock() - return lm.ledger(p).Accounting.BytesRecv + return lm.findOrCreate(p).Accounting.BytesRecv } // ledger lazily instantiates a ledger -func (lm *LedgerManager) ledger(p peer.Peer) *ledger { +func (lm *LedgerManager) findOrCreate(p peer.Peer) *ledger { l, ok := lm.ledgerMap[peerKey(p.Key())] if !ok { l = newLedger(p) From f533678724c655446cb1d5534283ad5923da9fab Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Mon, 15 Dec 2014 23:00:53 -0800 Subject: [PATCH 20/64] avoid attaching context to object when it's not necessary. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index d6699e9f0..df10072eb 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -27,7 +27,6 @@ type LedgerManager struct { tasklist *TaskList taskOut chan *Task workSignal chan struct{} - ctx context.Context } func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager { @@ -37,20 +36,19 @@ func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager tasklist: NewTaskList(), taskOut: make(chan *Task, 4), workSignal: make(chan struct{}), - ctx: ctx, } - go lm.taskWorker() + go lm.taskWorker(ctx) return lm } -func (lm *LedgerManager) taskWorker() { +func (lm *LedgerManager) taskWorker(ctx context.Context) { for { nextTask := lm.tasklist.Pop() if nextTask == nil { // No tasks in the list? // Wait until there are! select { - case <-lm.ctx.Done(): + case <-ctx.Done(): return case <-lm.workSignal: } @@ -58,7 +56,7 @@ func (lm *LedgerManager) taskWorker() { } select { - case <-lm.ctx.Done(): + case <-ctx.Done(): return case lm.taskOut <- nextTask: } From 69dd26023637965f0232a7d095378b33e74d74fe Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 16 Dec 2014 18:33:36 +0000 Subject: [PATCH 21/64] refactor peerSet --- exchange/bitswap/bitswap.go | 2 +- routing/dht/routing.go | 8 ++++---- util/peerset/peerset.go | 21 +++++++++++++++++---- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 9b92bb0aa..5cf28c96d 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -222,7 +222,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan providers := bs.routing.FindProvidersAsync(child, k, maxProvidersPerRequest) for prov := range providers { - if ps.AddIfSmallerThan(prov, -1) { //Do once per peer + if ps.TryAdd(prov) { //Do once per peer bs.send(ctx, prov, message) } } diff --git a/routing/dht/routing.go b/routing/dht/routing.go index f8036ca38..bfe38e200 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -141,11 +141,11 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int func (dht *IpfsDHT) findProvidersAsyncRoutine(ctx context.Context, key u.Key, count int, peerOut chan peer.Peer) { defer close(peerOut) - ps := pset.NewPeerSet() + ps := pset.NewLimitedPeerSet(count) provs := dht.providers.GetProviders(ctx, key) for _, p := range provs { // NOTE: assuming that this list of peers is unique - if ps.AddIfSmallerThan(p, count) { + if ps.TryAdd(p) { select { case peerOut <- p: case <-ctx.Done(): @@ -176,7 +176,7 @@ func (dht *IpfsDHT) findProvidersAsyncRoutine(ctx context.Context, key u.Key, co // Add unique providers from request, up to 'count' for _, prov := range provs { - if ps.AddIfSmallerThan(prov, count) { + if ps.TryAdd(prov) { select { case peerOut <- prov: case <-ctx.Done(): @@ -226,7 +226,7 @@ func (dht *IpfsDHT) addPeerListAsync(ctx context.Context, k u.Key, peers []*pb.M } dht.providers.AddProvider(k, p) - if ps.AddIfSmallerThan(p, count) { + if ps.TryAdd(p) { select { case out <- p: case <-ctx.Done(): diff --git a/util/peerset/peerset.go b/util/peerset/peerset.go index c2a488e43..e969b4a4b 100644 --- a/util/peerset/peerset.go +++ b/util/peerset/peerset.go @@ -7,13 +7,22 @@ import ( // PeerSet is a threadsafe set of peers type PeerSet struct { - ps map[string]bool - lk sync.RWMutex + ps map[string]bool + lk sync.RWMutex + size int } func NewPeerSet() *PeerSet { ps := new(PeerSet) ps.ps = make(map[string]bool) + ps.size = -1 + return ps +} + +func NewLimitedPeerSet(size int) *PeerSet { + ps := new(PeerSet) + ps.ps = make(map[string]bool) + ps.size = -1 return ps } @@ -36,10 +45,14 @@ func (ps *PeerSet) Size() int { return len(ps.ps) } -func (ps *PeerSet) AddIfSmallerThan(p peer.Peer, maxsize int) bool { +// TryAdd Attempts to add the given peer into the set. +// This operation can fail for one of two reasons: +// 1) The given peer is already in the set +// 2) The number of peers in the set is equal to size +func (ps *PeerSet) TryAdd(p peer.Peer) bool { var success bool ps.lk.Lock() - if _, ok := ps.ps[string(p.ID())]; !ok && len(ps.ps) < maxsize { + if _, ok := ps.ps[string(p.ID())]; !ok && (len(ps.ps) < ps.size || ps.size == -1) { success = true ps.ps[string(p.ID())] = true } From c0a18d9c08b54a17bfbfc6ec9214f9267df5746e Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:26:41 -0800 Subject: [PATCH 22/64] fix(test): nil Blockstore License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager_test.go b/exchange/bitswap/strategy/ledgermanager_test.go index f2a98cb77..eb89c9959 100644 --- a/exchange/bitswap/strategy/ledgermanager_test.go +++ b/exchange/bitswap/strategy/ledgermanager_test.go @@ -4,9 +4,11 @@ import ( "strings" "testing" - "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - + 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" + 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" message "github.com/jbenet/go-ipfs/exchange/bitswap/message" peer "github.com/jbenet/go-ipfs/peer" testutil "github.com/jbenet/go-ipfs/util/testutil" @@ -21,7 +23,7 @@ func newPeerAndLedgermanager(idStr string) peerAndLedgermanager { return peerAndLedgermanager{ Peer: testutil.NewPeerWithIDString(idStr), //Strategy: New(true), - ls: NewLedgerManager(nil, context.TODO()), + ls: NewLedgerManager(blockstore.NewBlockstore(sync.MutexWrap(ds.NewMapDatastore())), context.TODO()), } } From 5603b2e0a4e3eb8ae1cc8a49c88425e5d9ec498b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:10:20 -0800 Subject: [PATCH 23/64] style: line wrapping 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 5cf28c96d..bccd04418 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -32,10 +32,10 @@ var providerRequestTimeout = time.Second * 10 var hasBlockTimeout = time.Second * 15 var rebroadcastDelay = time.Second * 10 -// New initializes a BitSwap instance that communicates over the -// provided BitSwapNetwork. This function registers the returned instance as -// the network delegate. -// Runs until context is cancelled +// New initializes a BitSwap instance that communicates over the provided +// BitSwapNetwork. This function registers the returned instance as the network +// delegate. +// Runs until context is cancelled. func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, routing bsnet.Routing, bstore blockstore.Blockstore, nice bool) exchange.Interface { From 8bef1dce804db407a48031de8ba6bec114769105 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:09:13 -0800 Subject: [PATCH 24/64] fix: move to callsite so public callers don't experience the internal timeout rule 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 bccd04418..57ae6a6ac 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -159,8 +159,7 @@ func (bs *bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { } bs.wantlist.Remove(blk.Key()) bs.notifications.Publish(blk) - child, _ := context.WithTimeout(ctx, hasBlockTimeout) - return bs.routing.Provide(child, blk.Key()) + return bs.routing.Provide(ctx, blk.Key()) } func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) error { @@ -319,7 +318,8 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm var blkeys []u.Key for _, block := range incoming.Blocks() { blkeys = append(blkeys, block.Key()) - if err := bs.HasBlock(ctx, block); err != nil { + hasBlockCtx, _ := context.WithTimeout(ctx, hasBlockTimeout) + if err := bs.HasBlock(hasBlockCtx, block); err != nil { log.Error(err) } } From a495a014afd5037b3f4fe6ccfdbabd4cec640c1a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:09:57 -0800 Subject: [PATCH 25/64] style constify variables good to const until it's required for them to be variable. TODO pass them in as configuration options --- exchange/bitswap/bitswap.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 57ae6a6ac..e95ffbc4f 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -24,13 +24,17 @@ import ( var log = eventlog.Logger("bitswap") -// Number of providers to request for sending a wantlist to -// TODO: if a 'non-nice' strategy is implemented, consider increasing this value -const maxProvidersPerRequest = 3 +const ( + // Number of providers to request for sending a wantlist to + // TODO: if a 'non-nice' strategy is implemented, consider increasing this value + maxProvidersPerRequest = 3 + providerRequestTimeout = time.Second * 10 + hasBlockTimeout = time.Second * 15 +) -var providerRequestTimeout = time.Second * 10 -var hasBlockTimeout = time.Second * 15 -var rebroadcastDelay = time.Second * 10 +var ( + rebroadcastDelay = time.Second * 10 +) // New initializes a BitSwap instance that communicates over the provided // BitSwapNetwork. This function registers the returned instance as the network From 2ea8ed81ac5d4cd0f99a6cbf3a6f33e43fbf4cf4 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:27:35 -0800 Subject: [PATCH 26/64] refactor: change Tasks to Outbox notice that moving the blockstore fetch into the manager removes the weird error handling case. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 14 ++---------- exchange/bitswap/strategy/ledgermanager.go | 25 ++++++++++++++++------ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index e95ffbc4f..4458db946 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -239,18 +239,8 @@ func (bs *bitswap) taskWorker(ctx context.Context) { select { case <-ctx.Done(): return - case task := <-bs.ledgermanager.GetTaskChan(): - block, err := bs.blockstore.Get(task.Key) - if err != nil { - log.Errorf("Expected to have block %s, but it was not found!", task.Key) - continue - } - - message := bsmsg.New() - message.AddBlock(block) - // TODO: maybe add keys from our wantlist? - - bs.send(ctx, task.Target, message) + case envelope := <-bs.ledgermanager.Outbox(): + bs.send(ctx, envelope.Peer, envelope.Message) } } } diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index df10072eb..3c79c855c 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -20,12 +20,17 @@ type ledgerMap map[peerKey]*ledger // FIXME share this externally type peerKey u.Key +type Envelope struct { + Peer peer.Peer + Message bsmsg.BitSwapMessage +} + type LedgerManager struct { lock sync.RWMutex ledgerMap ledgerMap bs bstore.Blockstore tasklist *TaskList - taskOut chan *Task + outbox chan Envelope workSignal chan struct{} } @@ -34,7 +39,7 @@ func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager ledgerMap: make(ledgerMap), bs: bs, tasklist: NewTaskList(), - taskOut: make(chan *Task, 4), + outbox: make(chan Envelope, 4), // TODO extract constant workSignal: make(chan struct{}), } go lm.taskWorker(ctx) @@ -54,17 +59,25 @@ func (lm *LedgerManager) taskWorker(ctx context.Context) { } continue } - + block, err := lm.bs.Get(nextTask.Key) + if err != nil { + continue // TODO maybe return an error + } + // construct message here so we can make decisions about any additional + // information we may want to include at this time. + m := bsmsg.New() + m.AddBlock(block) + // TODO: maybe add keys from our wantlist? select { case <-ctx.Done(): return - case lm.taskOut <- nextTask: + case lm.outbox <- Envelope{Peer: nextTask.Target, Message: m}: } } } -func (lm *LedgerManager) GetTaskChan() <-chan *Task { - return lm.taskOut +func (lm *LedgerManager) Outbox() <-chan Envelope { + return lm.outbox } // Returns a slice of Peers with whom the local node has active sessions From 8c05c444a5ad758b3a6e1bfec1767221f17b9340 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:31:49 -0800 Subject: [PATCH 27/64] refactor: avoid loop reuse License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 4458db946..998114192 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -309,14 +309,16 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // TODO: this is bad, and could be easily abused. // Should only track *useful* messages in ledger - var blkeys []u.Key for _, block := range incoming.Blocks() { - blkeys = append(blkeys, block.Key()) hasBlockCtx, _ := context.WithTimeout(ctx, hasBlockTimeout) if err := bs.HasBlock(hasBlockCtx, block); err != nil { log.Error(err) } } + var blkeys []u.Key + for _, block := range incoming.Blocks() { + blkeys = append(blkeys, block.Key()) + } if len(blkeys) > 0 { bs.cancelBlocks(ctx, blkeys) } From f66d94aaf14106efc2b7855828f285f7f7b0db5a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:35:26 -0800 Subject: [PATCH 28/64] fix: move the check into the function. function should be a no-op when passed an empty slice License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 998114192..f1ae4b556 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -315,19 +315,20 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm log.Error(err) } } - var blkeys []u.Key + var keys []u.Key for _, block := range incoming.Blocks() { - blkeys = append(blkeys, block.Key()) - } - if len(blkeys) > 0 { - bs.cancelBlocks(ctx, blkeys) + keys = append(keys, block.Key()) } + bs.cancelBlocks(ctx, keys) // TODO: consider changing this function to not return anything return nil, nil } func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { + if len(bkeys) < 1 { + return + } message := bsmsg.New() message.SetFull(false) for _, k := range bkeys { From 10e970c0cf0a4d90d9164456d9d8dc5da115a193 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:52:30 -0800 Subject: [PATCH 29/64] refactor: context first in argument list (merely by convention) License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 2 +- exchange/bitswap/strategy/ledgermanager.go | 2 +- exchange/bitswap/strategy/ledgermanager_test.go | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index f1ae4b556..cae7ab1e8 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -56,7 +56,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout blockstore: bstore, cancelFunc: cancelFunc, notifications: notif, - ledgermanager: strategy.NewLedgerManager(bstore, ctx), + ledgermanager: strategy.NewLedgerManager(ctx, bstore), routing: routing, sender: network, wantlist: wl.New(), diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 3c79c855c..1ea61bb7d 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -34,7 +34,7 @@ type LedgerManager struct { workSignal chan struct{} } -func NewLedgerManager(bs bstore.Blockstore, ctx context.Context) *LedgerManager { +func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager { lm := &LedgerManager{ ledgerMap: make(ledgerMap), bs: bs, diff --git a/exchange/bitswap/strategy/ledgermanager_test.go b/exchange/bitswap/strategy/ledgermanager_test.go index eb89c9959..5c78f2f81 100644 --- a/exchange/bitswap/strategy/ledgermanager_test.go +++ b/exchange/bitswap/strategy/ledgermanager_test.go @@ -23,7 +23,8 @@ func newPeerAndLedgermanager(idStr string) peerAndLedgermanager { return peerAndLedgermanager{ Peer: testutil.NewPeerWithIDString(idStr), //Strategy: New(true), - ls: NewLedgerManager(blockstore.NewBlockstore(sync.MutexWrap(ds.NewMapDatastore())), context.TODO()), + ls: NewLedgerManager(context.TODO(), + blockstore.NewBlockstore(sync.MutexWrap(ds.NewMapDatastore()))), } } From 460967839eeea637411b8a97fbd9bf27f975689a Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 20:56:18 -0800 Subject: [PATCH 30/64] doc: comment License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/tasklist.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/tasklist.go index f0a1b7d00..19bb9748e 100644 --- a/exchange/bitswap/strategy/tasklist.go +++ b/exchange/bitswap/strategy/tasklist.go @@ -43,8 +43,7 @@ func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { tl.taskmap[to.Key()+block] = task } -// Pop returns the next task to be performed by bitswap the task is then -// removed from the list +// Pop 'pops' the next task to be performed. Returns nil no task exists. func (tl *TaskList) Pop() *Task { var out *Task for len(tl.tasks) > 0 { From 93fde86d18071549d3b5251eb16991fe0a2b66d7 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:01:01 -0800 Subject: [PATCH 31/64] refactor: taskKey := p.Key() + block.Key() for clarity and to avoid errors, define a function License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 1 - exchange/bitswap/strategy/tasklist.go | 17 +++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 1ea61bb7d..6c6f7ee75 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -17,7 +17,6 @@ var log = u.Logger("strategy") // LedgerMap lists Ledgers by their Partner key. type ledgerMap map[peerKey]*ledger -// FIXME share this externally type peerKey u.Key type Envelope struct { diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/tasklist.go index 19bb9748e..0e8948cbb 100644 --- a/exchange/bitswap/strategy/tasklist.go +++ b/exchange/bitswap/strategy/tasklist.go @@ -10,12 +10,12 @@ import ( // tasks (on getnext). For now, we are assuming a dumb/nice strategy. type TaskList struct { tasks []*Task - taskmap map[u.Key]*Task + taskmap map[string]*Task } func NewTaskList() *TaskList { return &TaskList{ - taskmap: make(map[u.Key]*Task), + taskmap: make(map[string]*Task), } } @@ -28,7 +28,7 @@ type Task struct { // Push currently adds a new task to the end of the list // TODO: make this into a priority queue func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { - if task, ok := tl.taskmap[to.Key()+block]; ok { + if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, // rearrange this Task task.theirPriority = priority @@ -40,7 +40,7 @@ func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { theirPriority: priority, } tl.tasks = append(tl.tasks, task) - tl.taskmap[to.Key()+block] = task + tl.taskmap[taskKey(to, block)] = task } // Pop 'pops' the next task to be performed. Returns nil no task exists. @@ -52,7 +52,7 @@ func (tl *TaskList) Pop() *Task { // the same block from multiple peers out = tl.tasks[0] tl.tasks = tl.tasks[1:] - delete(tl.taskmap, out.Target.Key()+out.Key) + delete(tl.taskmap, taskKey(out.Target, out.Key)) // Filter out blocks that have been cancelled if out.theirPriority >= 0 { break @@ -64,8 +64,13 @@ func (tl *TaskList) Pop() *Task { // Cancel lazily cancels the sending of a block to a given peer func (tl *TaskList) Cancel(k u.Key, p peer.Peer) { - t, ok := tl.taskmap[p.Key()+k] + t, ok := tl.taskmap[taskKey(p, k)] if ok { t.theirPriority = -1 } } + +// taskKey returns a key that uniquely identifies a task. +func taskKey(p peer.Peer, k u.Key) string { + return string(p.Key() + k) +} From 286723d8882142dea2fc63bc391038ed4d0d176b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:06:10 -0800 Subject: [PATCH 32/64] unexport task and taskList the less bitswap has to know about, the easier it'll be for readers. (This now returns Messages.) License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 4 ++-- exchange/bitswap/strategy/tasklist.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 6c6f7ee75..77b5d66b1 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -28,7 +28,7 @@ type LedgerManager struct { lock sync.RWMutex ledgerMap ledgerMap bs bstore.Blockstore - tasklist *TaskList + tasklist *taskList outbox chan Envelope workSignal chan struct{} } @@ -37,7 +37,7 @@ func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager lm := &LedgerManager{ ledgerMap: make(ledgerMap), bs: bs, - tasklist: NewTaskList(), + tasklist: newTaskList(), outbox: make(chan Envelope, 4), // TODO extract constant workSignal: make(chan struct{}), } diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/tasklist.go index 0e8948cbb..8e89c238b 100644 --- a/exchange/bitswap/strategy/tasklist.go +++ b/exchange/bitswap/strategy/tasklist.go @@ -8,13 +8,13 @@ import ( // TODO: at some point, the strategy needs to plug in here // to help decide how to sort tasks (on add) and how to select // tasks (on getnext). For now, we are assuming a dumb/nice strategy. -type TaskList struct { +type taskList struct { tasks []*Task taskmap map[string]*Task } -func NewTaskList() *TaskList { - return &TaskList{ +func newTaskList() *taskList { + return &taskList{ taskmap: make(map[string]*Task), } } @@ -27,7 +27,7 @@ type Task struct { // Push currently adds a new task to the end of the list // TODO: make this into a priority queue -func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { +func (tl *taskList) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, // rearrange this Task @@ -44,7 +44,7 @@ func (tl *TaskList) Push(block u.Key, priority int, to peer.Peer) { } // Pop 'pops' the next task to be performed. Returns nil no task exists. -func (tl *TaskList) Pop() *Task { +func (tl *taskList) Pop() *Task { var out *Task for len(tl.tasks) > 0 { // TODO: instead of zero, use exponential distribution @@ -63,7 +63,7 @@ func (tl *TaskList) Pop() *Task { } // Cancel lazily cancels the sending of a block to a given peer -func (tl *TaskList) Cancel(k u.Key, p peer.Peer) { +func (tl *taskList) Cancel(k u.Key, p peer.Peer) { t, ok := tl.taskmap[taskKey(p, k)] if ok { t.theirPriority = -1 From b41fef2fffaddb94b6a32972862d60bd73ac65ac Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:08:28 -0800 Subject: [PATCH 33/64] refactor: remove peerKey type we've been using maps with peers long enough now that this probably is no longer necessary License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 77b5d66b1..4bc8f2efc 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -15,9 +15,7 @@ import ( var log = u.Logger("strategy") // LedgerMap lists Ledgers by their Partner key. -type ledgerMap map[peerKey]*ledger - -type peerKey u.Key +type ledgerMap map[u.Key]*ledger type Envelope struct { Peer peer.Peer @@ -182,10 +180,10 @@ func (lm *LedgerManager) NumBytesReceivedFrom(p peer.Peer) uint64 { // ledger lazily instantiates a ledger func (lm *LedgerManager) findOrCreate(p peer.Peer) *ledger { - l, ok := lm.ledgerMap[peerKey(p.Key())] + l, ok := lm.ledgerMap[p.Key()] if !ok { l = newLedger(p) - lm.ledgerMap[peerKey(p.Key())] = l + lm.ledgerMap[p.Key()] = l } return l } From cc2a7312a28e7fd12eed75f85890cb528feb6cb5 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:25:37 -0800 Subject: [PATCH 34/64] add comment to fix race License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 4bc8f2efc..d328510a1 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -23,9 +23,11 @@ type Envelope struct { } type LedgerManager struct { - lock sync.RWMutex - ledgerMap ledgerMap - bs bstore.Blockstore + lock sync.RWMutex + ledgerMap ledgerMap + bs bstore.Blockstore + // FIXME tasklist isn't threadsafe nor is it protected by a mutex. consider + // a way to avoid sharing the tasklist between the worker and the receiver tasklist *taskList outbox chan Envelope workSignal chan struct{} From 7280aac83ff52af54225cde33188d0b7b9ee9b4b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:28:55 -0800 Subject: [PATCH 35/64] perf: avoid lots of communication by signaling once at end of method License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 25 +++++++++++----------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index d328510a1..a84a5b7c8 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -104,6 +104,16 @@ func (lm *LedgerManager) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { // MessageReceived performs book-keeping. Returns error if passed invalid // arguments. func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { + newWorkExists := false + defer func() { + if newWorkExists { + // Signal task generation to restart (if stopped!) + select { + case lm.workSignal <- struct{}{}: + default: + } + } + }() lm.lock.Lock() defer lm.lock.Unlock() @@ -117,13 +127,8 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er lm.tasklist.Cancel(e.Key, p) } else { l.Wants(e.Key, e.Priority) + newWorkExists = true lm.tasklist.Push(e.Key, e.Priority, p) - - // Signal task generation to restart (if stopped!) - select { - case lm.workSignal <- struct{}{}: - default: - } } } @@ -132,14 +137,8 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er l.ReceivedBytes(len(block.Data)) for _, l := range lm.ledgerMap { if l.WantListContains(block.Key()) { + newWorkExists = true lm.tasklist.Push(block.Key(), 1, l.Partner) - - // Signal task generation to restart (if stopped!) - select { - case lm.workSignal <- struct{}{}: - default: - } - } } } From 198aa1959ae721ec646e0f0ad317525703a69068 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:36:55 -0800 Subject: [PATCH 36/64] it's not a queue yet but it's okay to name it as such License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 18 +++++++++--------- .../strategy/{tasklist.go => taskqueue.go} | 12 ++++++------ 2 files changed, 15 insertions(+), 15 deletions(-) rename exchange/bitswap/strategy/{tasklist.go => taskqueue.go} (87%) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index a84a5b7c8..47117553c 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -26,9 +26,9 @@ type LedgerManager struct { lock sync.RWMutex ledgerMap ledgerMap bs bstore.Blockstore - // FIXME tasklist isn't threadsafe nor is it protected by a mutex. consider - // a way to avoid sharing the tasklist between the worker and the receiver - tasklist *taskList + // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider + // a way to avoid sharing the taskqueue between the worker and the receiver + taskqueue *taskQueue outbox chan Envelope workSignal chan struct{} } @@ -37,7 +37,7 @@ func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager lm := &LedgerManager{ ledgerMap: make(ledgerMap), bs: bs, - tasklist: newTaskList(), + taskqueue: newTaskQueue(), outbox: make(chan Envelope, 4), // TODO extract constant workSignal: make(chan struct{}), } @@ -47,7 +47,7 @@ func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager func (lm *LedgerManager) taskWorker(ctx context.Context) { for { - nextTask := lm.tasklist.Pop() + nextTask := lm.taskqueue.Pop() if nextTask == nil { // No tasks in the list? // Wait until there are! @@ -124,11 +124,11 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er for _, e := range m.Wantlist() { if e.Cancel { l.CancelWant(e.Key) - lm.tasklist.Cancel(e.Key, p) + lm.taskqueue.Cancel(e.Key, p) } else { l.Wants(e.Key, e.Priority) newWorkExists = true - lm.tasklist.Push(e.Key, e.Priority, p) + lm.taskqueue.Push(e.Key, e.Priority, p) } } @@ -138,7 +138,7 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er for _, l := range lm.ledgerMap { if l.WantListContains(block.Key()) { newWorkExists = true - lm.tasklist.Push(block.Key(), 1, l.Partner) + lm.taskqueue.Push(block.Key(), 1, l.Partner) } } } @@ -159,7 +159,7 @@ func (lm *LedgerManager) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) - lm.tasklist.Cancel(block.Key(), p) + lm.taskqueue.Cancel(block.Key(), p) } return nil diff --git a/exchange/bitswap/strategy/tasklist.go b/exchange/bitswap/strategy/taskqueue.go similarity index 87% rename from exchange/bitswap/strategy/tasklist.go rename to exchange/bitswap/strategy/taskqueue.go index 8e89c238b..fbb21926e 100644 --- a/exchange/bitswap/strategy/tasklist.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -8,13 +8,13 @@ import ( // TODO: at some point, the strategy needs to plug in here // to help decide how to sort tasks (on add) and how to select // tasks (on getnext). For now, we are assuming a dumb/nice strategy. -type taskList struct { +type taskQueue struct { tasks []*Task taskmap map[string]*Task } -func newTaskList() *taskList { - return &taskList{ +func newTaskQueue() *taskQueue { + return &taskQueue{ taskmap: make(map[string]*Task), } } @@ -27,7 +27,7 @@ type Task struct { // Push currently adds a new task to the end of the list // TODO: make this into a priority queue -func (tl *taskList) Push(block u.Key, priority int, to peer.Peer) { +func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, // rearrange this Task @@ -44,7 +44,7 @@ func (tl *taskList) Push(block u.Key, priority int, to peer.Peer) { } // Pop 'pops' the next task to be performed. Returns nil no task exists. -func (tl *taskList) Pop() *Task { +func (tl *taskQueue) Pop() *Task { var out *Task for len(tl.tasks) > 0 { // TODO: instead of zero, use exponential distribution @@ -63,7 +63,7 @@ func (tl *taskList) Pop() *Task { } // Cancel lazily cancels the sending of a block to a given peer -func (tl *taskList) Cancel(k u.Key, p peer.Peer) { +func (tl *taskQueue) Cancel(k u.Key, p peer.Peer) { t, ok := tl.taskmap[taskKey(p, k)] if ok { t.theirPriority = -1 From 181ff4eab1063475bd833c11e6343c409b658785 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:42:30 -0800 Subject: [PATCH 37/64] tq.Cancel -> tq.Remove License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 4 ++-- exchange/bitswap/strategy/taskqueue.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 47117553c..23c5e2df0 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -124,7 +124,7 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er for _, e := range m.Wantlist() { if e.Cancel { l.CancelWant(e.Key) - lm.taskqueue.Cancel(e.Key, p) + lm.taskqueue.Remove(e.Key, p) } else { l.Wants(e.Key, e.Priority) newWorkExists = true @@ -159,7 +159,7 @@ func (lm *LedgerManager) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) - lm.taskqueue.Cancel(block.Key(), p) + lm.taskqueue.Remove(block.Key(), p) } return nil diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/strategy/taskqueue.go index fbb21926e..b721431ba 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -62,8 +62,8 @@ func (tl *taskQueue) Pop() *Task { return out } -// Cancel lazily cancels the sending of a block to a given peer -func (tl *taskQueue) Cancel(k u.Key, p peer.Peer) { +// Remove lazily removes a task from the queue +func (tl *taskQueue) Remove(k u.Key, p peer.Peer) { t, ok := tl.taskmap[taskKey(p, k)] if ok { t.theirPriority = -1 From e11f099c4230e165471a1369b0d132e7bc56f94c Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:43:38 -0800 Subject: [PATCH 38/64] privatize Task License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/taskqueue.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/strategy/taskqueue.go index b721431ba..0b92b256a 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -9,17 +9,17 @@ import ( // to help decide how to sort tasks (on add) and how to select // tasks (on getnext). For now, we are assuming a dumb/nice strategy. type taskQueue struct { - tasks []*Task - taskmap map[string]*Task + tasks []*task + taskmap map[string]*task } func newTaskQueue() *taskQueue { return &taskQueue{ - taskmap: make(map[string]*Task), + taskmap: make(map[string]*task), } } -type Task struct { +type task struct { Key u.Key Target peer.Peer theirPriority int @@ -30,11 +30,11 @@ type Task struct { func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, - // rearrange this Task + // rearrange this task task.theirPriority = priority return } - task := &Task{ + task := &task{ Key: block, Target: to, theirPriority: priority, @@ -44,8 +44,8 @@ func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { } // Pop 'pops' the next task to be performed. Returns nil no task exists. -func (tl *taskQueue) Pop() *Task { - var out *Task +func (tl *taskQueue) Pop() *task { + var out *task for len(tl.tasks) > 0 { // TODO: instead of zero, use exponential distribution // it will help reduce the chance of receiving From 1d715956f50310278841f9eebd59b2c216e8fe02 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 21:51:00 -0800 Subject: [PATCH 39/64] doc: add comment to Envelope License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 23c5e2df0..a2701c208 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -17,8 +17,11 @@ var log = u.Logger("strategy") // LedgerMap lists Ledgers by their Partner key. type ledgerMap map[u.Key]*ledger +// Envelope contains a message for a Peer type Envelope struct { - Peer peer.Peer + // Peer is the intended recipient + Peer peer.Peer + // Message is the payload Message bsmsg.BitSwapMessage } From fedcebf67a6b01339bcd57397dcee2484a45cf97 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:08:53 -0800 Subject: [PATCH 40/64] refactor: re-use wantlist.Entry type wherever it makes sense it seems to make sense since, in each place, the Key and Priority represent the same information b/c you know the saying... "It is better to have 100 functions operate on one data structure than 10 functions on 10 data structures." License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 6 +++--- exchange/bitswap/message/message.go | 14 ++++++++------ exchange/bitswap/strategy/ledgermanager.go | 2 +- exchange/bitswap/strategy/taskqueue.go | 22 ++++++++++++---------- exchange/bitswap/wantlist/wantlist.go | 4 ++-- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index cae7ab1e8..d9b3c52ef 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -172,7 +172,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e } message := bsmsg.New() for _, wanted := range bs.wantlist.Entries() { - message.AddEntry(wanted.Value, wanted.Priority) + message.AddEntry(wanted.Key, wanted.Priority) } wg := sync.WaitGroup{} for peerToQuery := range peers { @@ -210,7 +210,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan message := bsmsg.New() message.SetFull(true) for _, e := range bs.wantlist.Entries() { - message.AddEntry(e.Value, e.Priority) + message.AddEntry(e.Key, e.Priority) } ps := pset.NewPeerSet() @@ -229,7 +229,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wan bs.send(ctx, prov, message) } } - }(e.Value) + }(e.Key) } wg.Wait() } diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index 478d8e258..245fc35fb 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -5,6 +5,7 @@ import ( blocks "github.com/jbenet/go-ipfs/blocks" pb "github.com/jbenet/go-ipfs/exchange/bitswap/message/internal/pb" + wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" inet "github.com/jbenet/go-ipfs/net" u "github.com/jbenet/go-ipfs/util" @@ -64,9 +65,8 @@ func newMsg() *impl { } type Entry struct { - Key u.Key - Priority int - Cancel bool + wantlist.Entry + Cancel bool } func newMessageFromProto(pbm pb.Message) BitSwapMessage { @@ -121,9 +121,11 @@ func (m *impl) addEntry(k u.Key, priority int, cancel bool) { e.Cancel = cancel } else { m.wantlist[k] = &Entry{ - Key: k, - Priority: priority, - Cancel: cancel, + Entry: wantlist.Entry{ + Key: k, + Priority: priority, + }, + Cancel: cancel, } } } diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index a2701c208..26e47e14e 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -61,7 +61,7 @@ func (lm *LedgerManager) taskWorker(ctx context.Context) { } continue } - block, err := lm.bs.Get(nextTask.Key) + block, err := lm.bs.Get(nextTask.Entry.Key) if err != nil { continue // TODO maybe return an error } diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/strategy/taskqueue.go index 0b92b256a..d5a4eb886 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -1,6 +1,7 @@ package strategy import ( + wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" ) @@ -20,9 +21,8 @@ func newTaskQueue() *taskQueue { } type task struct { - Key u.Key - Target peer.Peer - theirPriority int + Entry wantlist.Entry + Target peer.Peer } // Push currently adds a new task to the end of the list @@ -31,13 +31,15 @@ func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, // rearrange this task - task.theirPriority = priority + task.Entry.Priority = priority return } task := &task{ - Key: block, - Target: to, - theirPriority: priority, + Entry: wantlist.Entry{ + Key: block, + Priority: priority, + }, + Target: to, } tl.tasks = append(tl.tasks, task) tl.taskmap[taskKey(to, block)] = task @@ -52,9 +54,9 @@ func (tl *taskQueue) Pop() *task { // the same block from multiple peers out = tl.tasks[0] tl.tasks = tl.tasks[1:] - delete(tl.taskmap, taskKey(out.Target, out.Key)) + delete(tl.taskmap, taskKey(out.Target, out.Entry.Key)) // Filter out blocks that have been cancelled - if out.theirPriority >= 0 { + if out.Entry.Priority >= 0 { // FIXME separate the "cancel" signal from priority break } } @@ -66,7 +68,7 @@ func (tl *taskQueue) Pop() *task { func (tl *taskQueue) Remove(k u.Key, p peer.Peer) { t, ok := tl.taskmap[taskKey(p, k)] if ok { - t.theirPriority = -1 + t.Entry.Priority = -1 } } diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index e20bb4457..2c50daa49 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -18,7 +18,7 @@ func New() *Wantlist { } type Entry struct { - Value u.Key + Key u.Key Priority int } @@ -29,7 +29,7 @@ func (w *Wantlist) Add(k u.Key, priority int) { return } w.set[k] = &Entry{ - Value: k, + Key: k, Priority: priority, } } From 7fdbae1306652b375a16cc58ba5fd1be4a8112f6 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:12:27 -0800 Subject: [PATCH 41/64] refactor: separate responsibilties Before, priority carried two pieces of information. One: priority as defined by remote peer Two: whether task is trashed This assumes the protocol is defined for natural numbers instead of integers. That may not always be the case. Better to leave that assumption outside so this package isn't coupled to the whims of the protocol. The protocol may be changed to allow any integer value to be used. Hopefully by that time, new responsibilties weren't added to the Priority variable. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/taskqueue.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/strategy/taskqueue.go index d5a4eb886..4dbfdd92b 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -23,6 +23,7 @@ func newTaskQueue() *taskQueue { type task struct { Entry wantlist.Entry Target peer.Peer + Trash bool } // Push currently adds a new task to the end of the list @@ -55,12 +56,11 @@ func (tl *taskQueue) Pop() *task { out = tl.tasks[0] tl.tasks = tl.tasks[1:] delete(tl.taskmap, taskKey(out.Target, out.Entry.Key)) - // Filter out blocks that have been cancelled - if out.Entry.Priority >= 0 { // FIXME separate the "cancel" signal from priority - break + if out.Trash { + continue // discarding tasks that have been removed } + break // and return |out| } - return out } @@ -68,7 +68,7 @@ func (tl *taskQueue) Pop() *task { func (tl *taskQueue) Remove(k u.Key, p peer.Peer) { t, ok := tl.taskmap[taskKey(p, k)] if ok { - t.Entry.Priority = -1 + t.Trash = true } } From 39d71931a895165443b6e66d3762c34988f7797d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:20:21 -0800 Subject: [PATCH 42/64] mv comment License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/taskqueue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/strategy/taskqueue.go index 4dbfdd92b..69bb95cd4 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/strategy/taskqueue.go @@ -10,6 +10,7 @@ import ( // to help decide how to sort tasks (on add) and how to select // tasks (on getnext). For now, we are assuming a dumb/nice strategy. type taskQueue struct { + // TODO: make this into a priority queue tasks []*task taskmap map[string]*task } @@ -27,7 +28,6 @@ type task struct { } // Push currently adds a new task to the end of the list -// TODO: make this into a priority queue func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { if task, ok := tl.taskmap[taskKey(to, block)]; ok { // TODO: when priority queue is implemented, From 962a9477cc798af7b368e4b1bb1b510871b8c3e2 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:21:36 -0800 Subject: [PATCH 43/64] refactor: remove ledgerMap type it's only used in two places, but i think we've been using maps on IPFS types so much now that the specificity is no longer necessary License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 26e47e14e..258f92fd1 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -3,8 +3,7 @@ package strategy import ( "sync" - "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" - + context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" bstore "github.com/jbenet/go-ipfs/blocks/blockstore" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" @@ -14,9 +13,6 @@ import ( var log = u.Logger("strategy") -// LedgerMap lists Ledgers by their Partner key. -type ledgerMap map[u.Key]*ledger - // Envelope contains a message for a Peer type Envelope struct { // Peer is the intended recipient @@ -26,8 +22,9 @@ type Envelope struct { } type LedgerManager struct { - lock sync.RWMutex - ledgerMap ledgerMap + lock sync.RWMutex + // ledgerMap lists Ledgers by their Partner key. + ledgerMap map[u.Key]*ledger bs bstore.Blockstore // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider // a way to avoid sharing the taskqueue between the worker and the receiver @@ -38,7 +35,7 @@ type LedgerManager struct { func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager { lm := &LedgerManager{ - ledgerMap: make(ledgerMap), + ledgerMap: make(map[u.Key]*ledger), bs: bs, taskqueue: newTaskQueue(), outbox: make(chan Envelope, 4), // TODO extract constant From d069ae11f433077b6ac6b16a7322e13907a0feb8 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:24:54 -0800 Subject: [PATCH 44/64] refactor: put mutex next to the things it protects If we put the lock next to the fields it protects, it can sometimes make it easier to reason about threadsafety. In this case, it reveals that the task queue (not threadsafe) isn't protected by the mutex, yet shared between the worker and callers. @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/strategy/ledgermanager.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/strategy/ledgermanager.go index 258f92fd1..92e6ea9c2 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/strategy/ledgermanager.go @@ -22,15 +22,19 @@ type Envelope struct { } type LedgerManager struct { + // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider + // a way to avoid sharing the taskqueue between the worker and the receiver + taskqueue *taskQueue + + workSignal chan struct{} + + outbox chan Envelope + + bs bstore.Blockstore + lock sync.RWMutex // ledgerMap lists Ledgers by their Partner key. ledgerMap map[u.Key]*ledger - bs bstore.Blockstore - // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider - // a way to avoid sharing the taskqueue between the worker and the receiver - taskqueue *taskQueue - outbox chan Envelope - workSignal chan struct{} } func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager { From bef622222da715561fb3ac963719a08ca2caf696 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:46:53 -0800 Subject: [PATCH 45/64] refactor: wantlist splits into WL and ThreadSafe WL bitswap keeps the threadsafe version. observing the ledger shows that it doesn't need it anymore (ledgermanager is protected and safe). License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 8 +-- exchange/bitswap/wantlist/wantlist.go | 88 ++++++++++++++++++++------- 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index d9b3c52ef..473bf117e 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -15,7 +15,7 @@ import ( bsnet "github.com/jbenet/go-ipfs/exchange/bitswap/network" notifications "github.com/jbenet/go-ipfs/exchange/bitswap/notifications" strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" - wl "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" + wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" eventlog "github.com/jbenet/go-ipfs/util/eventlog" @@ -59,7 +59,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout ledgermanager: strategy.NewLedgerManager(ctx, bstore), routing: routing, sender: network, - wantlist: wl.New(), + wantlist: wantlist.NewThreadSafe(), batchRequests: make(chan []u.Key, 32), } network.SetDelegate(bs) @@ -95,7 +95,7 @@ type bitswap struct { ledgermanager *strategy.LedgerManager - wantlist *wl.Wantlist + wantlist *wantlist.ThreadSafe // cancelFunc signals cancellation to the bitswap event loop cancelFunc func() @@ -203,7 +203,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e return nil } -func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wl.Wantlist) { +func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wantlist.ThreadSafe) { ctx, cancel := context.WithCancel(ctx) defer cancel() diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 2c50daa49..6ef018668 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -6,25 +6,86 @@ import ( "sync" ) +type ThreadSafe struct { + lk sync.RWMutex + Wantlist +} + +// not threadsafe type Wantlist struct { - lk sync.RWMutex set map[u.Key]*Entry } +type Entry struct { + Key u.Key + Priority int +} + +type entrySlice []*Entry + +func (es entrySlice) Len() int { return len(es) } +func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } +func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priority } + +func NewThreadSafe() *ThreadSafe { + return &ThreadSafe{ + Wantlist: *New(), + } +} + func New() *Wantlist { return &Wantlist{ set: make(map[u.Key]*Entry), } } -type Entry struct { - Key u.Key - Priority int +func (w *ThreadSafe) Add(k u.Key, priority int) { + // TODO rm defer for perf + w.lk.Lock() + defer w.lk.Unlock() + w.Wantlist.Add(k, priority) +} + +func (w *ThreadSafe) Remove(k u.Key) { + // TODO rm defer for perf + w.lk.Lock() + defer w.lk.Unlock() + w.Wantlist.Remove(k) +} + +func (w *ThreadSafe) Contains(k u.Key) bool { + // TODO rm defer for perf + w.lk.RLock() + defer w.lk.RUnlock() + return w.Wantlist.Contains(k) +} + +func (w *ThreadSafe) Entries() []*Entry { + w.lk.RLock() + defer w.lk.RUnlock() + var es entrySlice + for _, e := range w.set { + es = append(es, e) + } + // TODO rename SortedEntries (state that they're sorted so callers know + // they're paying an expense) + sort.Sort(es) + return es +} + +func (w *ThreadSafe) SortedEntries() []*Entry { + w.lk.RLock() + defer w.lk.RUnlock() + var es entrySlice + + for _, e := range w.set { + es = append(es, e) + } + sort.Sort(es) + return es } func (w *Wantlist) Add(k u.Key, priority int) { - w.lk.Lock() - defer w.lk.Unlock() if _, ok := w.set[k]; ok { return } @@ -35,28 +96,15 @@ func (w *Wantlist) Add(k u.Key, priority int) { } func (w *Wantlist) Remove(k u.Key) { - w.lk.Lock() - defer w.lk.Unlock() delete(w.set, k) } func (w *Wantlist) Contains(k u.Key) bool { - w.lk.RLock() - defer w.lk.RUnlock() _, ok := w.set[k] return ok } -type entrySlice []*Entry - -func (es entrySlice) Len() int { return len(es) } -func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } -func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priority } - func (w *Wantlist) Entries() []*Entry { - w.lk.RLock() - defer w.lk.RUnlock() - var es entrySlice for _, e := range w.set { @@ -67,8 +115,6 @@ func (w *Wantlist) Entries() []*Entry { } func (w *Wantlist) SortedEntries() []*Entry { - w.lk.RLock() - defer w.lk.RUnlock() var es entrySlice for _, e := range w.set { From 5bd0b9546288947d2d400827d11310e71af73375 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 22:52:29 -0800 Subject: [PATCH 46/64] rename to strategy.LedgerManager to decision.Engine License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 21 ++-- .../ledgermanager.go => decision/engine.go} | 102 +++++++++--------- .../engine_test.go} | 38 +++---- .../bitswap/{strategy => decision}/ledger.go | 2 +- exchange/bitswap/decision/ledger_test.go | 1 + .../{strategy => decision}/taskqueue.go | 2 +- exchange/bitswap/strategy/ledger_test.go | 1 - 7 files changed, 81 insertions(+), 86 deletions(-) rename exchange/bitswap/{strategy/ledgermanager.go => decision/engine.go} (61%) rename exchange/bitswap/{strategy/ledgermanager_test.go => decision/engine_test.go} (70%) rename exchange/bitswap/{strategy => decision}/ledger.go (99%) create mode 100644 exchange/bitswap/decision/ledger_test.go rename exchange/bitswap/{strategy => decision}/taskqueue.go (99%) delete mode 100644 exchange/bitswap/strategy/ledger_test.go diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 473bf117e..d0e49d182 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -7,14 +7,13 @@ 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" blockstore "github.com/jbenet/go-ipfs/blocks/blockstore" exchange "github.com/jbenet/go-ipfs/exchange" + decision "github.com/jbenet/go-ipfs/exchange/bitswap/decision" bsmsg "github.com/jbenet/go-ipfs/exchange/bitswap/message" bsnet "github.com/jbenet/go-ipfs/exchange/bitswap/network" notifications "github.com/jbenet/go-ipfs/exchange/bitswap/notifications" - strategy "github.com/jbenet/go-ipfs/exchange/bitswap/strategy" wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" @@ -56,7 +55,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout blockstore: bstore, cancelFunc: cancelFunc, notifications: notif, - ledgermanager: strategy.NewLedgerManager(ctx, bstore), + engine: decision.NewEngine(ctx, bstore), routing: routing, sender: network, wantlist: wantlist.NewThreadSafe(), @@ -89,11 +88,7 @@ type bitswap struct { // have more than a single block in the set batchRequests chan []u.Key - // strategy makes decisions about how to interact with partners. - // TODO: strategy commented out until we have a use for it again - //strategy strategy.Strategy - - ledgermanager *strategy.LedgerManager + engine *decision.Engine wantlist *wantlist.ThreadSafe @@ -196,7 +191,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.Peer) e // FIXME ensure accounting is handled correctly when // communication fails. May require slightly different API to // get better guarantees. May need shared sequence numbers. - bs.ledgermanager.MessageSent(p, message) + bs.engine.MessageSent(p, message) }(peerToQuery) } wg.Wait() @@ -239,7 +234,7 @@ func (bs *bitswap) taskWorker(ctx context.Context) { select { case <-ctx.Done(): return - case envelope := <-bs.ledgermanager.Outbox(): + case envelope := <-bs.engine.Outbox(): bs.send(ctx, envelope.Peer, envelope.Message) } } @@ -305,7 +300,7 @@ func (bs *bitswap) ReceiveMessage(ctx context.Context, p peer.Peer, incoming bsm // This call records changes to wantlists, blocks received, // and number of bytes transfered. - bs.ledgermanager.MessageReceived(p, incoming) + bs.engine.MessageReceived(p, incoming) // TODO: this is bad, and could be easily abused. // Should only track *useful* messages in ledger @@ -334,7 +329,7 @@ func (bs *bitswap) cancelBlocks(ctx context.Context, bkeys []u.Key) { for _, k := range bkeys { message.Cancel(k) } - for _, p := range bs.ledgermanager.Peers() { + for _, p := range bs.engine.Peers() { err := bs.send(ctx, p, message) if err != nil { log.Errorf("Error sending message: %s", err) @@ -354,7 +349,7 @@ func (bs *bitswap) send(ctx context.Context, p peer.Peer, m bsmsg.BitSwapMessage if err := bs.sender.SendMessage(ctx, p, m); err != nil { return err } - return bs.ledgermanager.MessageSent(p, m) + return bs.engine.MessageSent(p, m) } func (bs *bitswap) Close() error { diff --git a/exchange/bitswap/strategy/ledgermanager.go b/exchange/bitswap/decision/engine.go similarity index 61% rename from exchange/bitswap/strategy/ledgermanager.go rename to exchange/bitswap/decision/engine.go index 92e6ea9c2..3b81d2582 100644 --- a/exchange/bitswap/strategy/ledgermanager.go +++ b/exchange/bitswap/decision/engine.go @@ -1,4 +1,4 @@ -package strategy +package decision import ( "sync" @@ -11,7 +11,7 @@ import ( u "github.com/jbenet/go-ipfs/util" ) -var log = u.Logger("strategy") +var log = u.Logger("engine") // Envelope contains a message for a Peer type Envelope struct { @@ -21,7 +21,7 @@ type Envelope struct { Message bsmsg.BitSwapMessage } -type LedgerManager struct { +type Engine struct { // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider // a way to avoid sharing the taskqueue between the worker and the receiver taskqueue *taskQueue @@ -37,32 +37,32 @@ type LedgerManager struct { ledgerMap map[u.Key]*ledger } -func NewLedgerManager(ctx context.Context, bs bstore.Blockstore) *LedgerManager { - lm := &LedgerManager{ +func NewEngine(ctx context.Context, bs bstore.Blockstore) *Engine { + e := &Engine{ ledgerMap: make(map[u.Key]*ledger), bs: bs, taskqueue: newTaskQueue(), outbox: make(chan Envelope, 4), // TODO extract constant workSignal: make(chan struct{}), } - go lm.taskWorker(ctx) - return lm + go e.taskWorker(ctx) + return e } -func (lm *LedgerManager) taskWorker(ctx context.Context) { +func (e *Engine) taskWorker(ctx context.Context) { for { - nextTask := lm.taskqueue.Pop() + nextTask := e.taskqueue.Pop() if nextTask == nil { // No tasks in the list? // Wait until there are! select { case <-ctx.Done(): return - case <-lm.workSignal: + case <-e.workSignal: } continue } - block, err := lm.bs.Get(nextTask.Entry.Key) + block, err := e.bs.Get(nextTask.Entry.Key) if err != nil { continue // TODO maybe return an error } @@ -74,22 +74,22 @@ func (lm *LedgerManager) taskWorker(ctx context.Context) { select { case <-ctx.Done(): return - case lm.outbox <- Envelope{Peer: nextTask.Target, Message: m}: + case e.outbox <- Envelope{Peer: nextTask.Target, Message: m}: } } } -func (lm *LedgerManager) Outbox() <-chan Envelope { - return lm.outbox +func (e *Engine) Outbox() <-chan Envelope { + return e.outbox } // Returns a slice of Peers with whom the local node has active sessions -func (lm *LedgerManager) Peers() []peer.Peer { - lm.lock.RLock() - defer lm.lock.RUnlock() +func (e *Engine) Peers() []peer.Peer { + e.lock.RLock() + defer e.lock.RUnlock() response := make([]peer.Peer, 0) - for _, ledger := range lm.ledgerMap { + for _, ledger := range e.ledgerMap { response = append(response, ledger.Partner) } return response @@ -97,52 +97,52 @@ func (lm *LedgerManager) Peers() []peer.Peer { // BlockIsWantedByPeer returns true if peer wants the block given by this // key -func (lm *LedgerManager) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { - lm.lock.RLock() - defer lm.lock.RUnlock() +func (e *Engine) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { + e.lock.RLock() + defer e.lock.RUnlock() - ledger := lm.findOrCreate(p) + ledger := e.findOrCreate(p) return ledger.WantListContains(k) } // MessageReceived performs book-keeping. Returns error if passed invalid // arguments. -func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { +func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { newWorkExists := false defer func() { if newWorkExists { // Signal task generation to restart (if stopped!) select { - case lm.workSignal <- struct{}{}: + case e.workSignal <- struct{}{}: default: } } }() - lm.lock.Lock() - defer lm.lock.Unlock() + e.lock.Lock() + defer e.lock.Unlock() - l := lm.findOrCreate(p) + l := e.findOrCreate(p) if m.Full() { l.wantList = wl.New() } - for _, e := range m.Wantlist() { - if e.Cancel { - l.CancelWant(e.Key) - lm.taskqueue.Remove(e.Key, p) + for _, entry := range m.Wantlist() { + if entry.Cancel { + l.CancelWant(entry.Key) + e.taskqueue.Remove(entry.Key, p) } else { - l.Wants(e.Key, e.Priority) + l.Wants(entry.Key, entry.Priority) newWorkExists = true - lm.taskqueue.Push(e.Key, e.Priority, p) + e.taskqueue.Push(entry.Key, entry.Priority, p) } } for _, block := range m.Blocks() { // FIXME extract blocks.NumBytes(block) or block.NumBytes() method l.ReceivedBytes(len(block.Data)) - for _, l := range lm.ledgerMap { + for _, l := range e.ledgerMap { if l.WantListContains(block.Key()) { newWorkExists = true - lm.taskqueue.Push(block.Key(), 1, l.Partner) + e.taskqueue.Push(block.Key(), 1, l.Partner) } } } @@ -155,40 +155,40 @@ func (lm *LedgerManager) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) er // inconsistent. Would need to ensure that Sends and acknowledgement of the // send happen atomically -func (lm *LedgerManager) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { - lm.lock.Lock() - defer lm.lock.Unlock() +func (e *Engine) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { + e.lock.Lock() + defer e.lock.Unlock() - l := lm.findOrCreate(p) + l := e.findOrCreate(p) for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) - lm.taskqueue.Remove(block.Key(), p) + e.taskqueue.Remove(block.Key(), p) } return nil } -func (lm *LedgerManager) NumBytesSentTo(p peer.Peer) uint64 { - lm.lock.RLock() - defer lm.lock.RUnlock() +func (e *Engine) NumBytesSentTo(p peer.Peer) uint64 { + e.lock.RLock() + defer e.lock.RUnlock() - return lm.findOrCreate(p).Accounting.BytesSent + return e.findOrCreate(p).Accounting.BytesSent } -func (lm *LedgerManager) NumBytesReceivedFrom(p peer.Peer) uint64 { - lm.lock.RLock() - defer lm.lock.RUnlock() +func (e *Engine) NumBytesReceivedFrom(p peer.Peer) uint64 { + e.lock.RLock() + defer e.lock.RUnlock() - return lm.findOrCreate(p).Accounting.BytesRecv + return e.findOrCreate(p).Accounting.BytesRecv } // ledger lazily instantiates a ledger -func (lm *LedgerManager) findOrCreate(p peer.Peer) *ledger { - l, ok := lm.ledgerMap[p.Key()] +func (e *Engine) findOrCreate(p peer.Peer) *ledger { + l, ok := e.ledgerMap[p.Key()] if !ok { l = newLedger(p) - lm.ledgerMap[p.Key()] = l + e.ledgerMap[p.Key()] = l } return l } diff --git a/exchange/bitswap/strategy/ledgermanager_test.go b/exchange/bitswap/decision/engine_test.go similarity index 70% rename from exchange/bitswap/strategy/ledgermanager_test.go rename to exchange/bitswap/decision/engine_test.go index 5c78f2f81..592236c3e 100644 --- a/exchange/bitswap/strategy/ledgermanager_test.go +++ b/exchange/bitswap/decision/engine_test.go @@ -1,4 +1,4 @@ -package strategy +package decision import ( "strings" @@ -14,16 +14,16 @@ import ( testutil "github.com/jbenet/go-ipfs/util/testutil" ) -type peerAndLedgermanager struct { +type peerAndEngine struct { peer.Peer - ls *LedgerManager + Engine *Engine } -func newPeerAndLedgermanager(idStr string) peerAndLedgermanager { - return peerAndLedgermanager{ +func newPeerAndLedgermanager(idStr string) peerAndEngine { + return peerAndEngine{ Peer: testutil.NewPeerWithIDString(idStr), //Strategy: New(true), - ls: NewLedgerManager(context.TODO(), + Engine: NewEngine(context.TODO(), blockstore.NewBlockstore(sync.MutexWrap(ds.NewMapDatastore()))), } } @@ -39,23 +39,23 @@ func TestConsistentAccounting(t *testing.T) { content := []string{"this", "is", "message", "i"} m.AddBlock(blocks.NewBlock([]byte(strings.Join(content, " ")))) - sender.ls.MessageSent(receiver.Peer, m) - receiver.ls.MessageReceived(sender.Peer, m) + sender.Engine.MessageSent(receiver.Peer, m) + receiver.Engine.MessageReceived(sender.Peer, m) } // Ensure sender records the change - if sender.ls.NumBytesSentTo(receiver.Peer) == 0 { + if sender.Engine.NumBytesSentTo(receiver.Peer) == 0 { t.Fatal("Sent bytes were not recorded") } // Ensure sender and receiver have the same values - if sender.ls.NumBytesSentTo(receiver.Peer) != receiver.ls.NumBytesReceivedFrom(sender.Peer) { + if sender.Engine.NumBytesSentTo(receiver.Peer) != receiver.Engine.NumBytesReceivedFrom(sender.Peer) { t.Fatal("Inconsistent book-keeping. Strategies don't agree") } // Ensure sender didn't record receving anything. And that the receiver // didn't record sending anything - if receiver.ls.NumBytesSentTo(sender.Peer) != 0 || sender.ls.NumBytesReceivedFrom(receiver.Peer) != 0 { + if receiver.Engine.NumBytesSentTo(sender.Peer) != 0 || sender.Engine.NumBytesReceivedFrom(receiver.Peer) != 0 { t.Fatal("Bert didn't send bytes to Ernie") } } @@ -69,10 +69,10 @@ func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { messageFromBeggarToChooser := message.New() messageFromBeggarToChooser.AddEntry(block.Key(), 1) - chooser.ls.MessageReceived(beggar.Peer, messageFromBeggarToChooser) + chooser.Engine.MessageReceived(beggar.Peer, messageFromBeggarToChooser) // for this test, doesn't matter if you record that beggar sent - if !chooser.ls.BlockIsWantedByPeer(block.Key(), beggar.Peer) { + if !chooser.Engine.BlockIsWantedByPeer(block.Key(), beggar.Peer) { t.Fatal("chooser failed to record that beggar wants block") } } @@ -84,24 +84,24 @@ func TestPeerIsAddedToPeersWhenMessageReceivedOrSent(t *testing.T) { m := message.New() - sanfrancisco.ls.MessageSent(seattle.Peer, m) - seattle.ls.MessageReceived(sanfrancisco.Peer, m) + sanfrancisco.Engine.MessageSent(seattle.Peer, m) + seattle.Engine.MessageReceived(sanfrancisco.Peer, m) if seattle.Peer.Key() == sanfrancisco.Peer.Key() { t.Fatal("Sanity Check: Peers have same Key!") } - if !peerIsPartner(seattle.Peer, sanfrancisco.ls) { + if !peerIsPartner(seattle.Peer, sanfrancisco.Engine) { t.Fatal("Peer wasn't added as a Partner") } - if !peerIsPartner(sanfrancisco.Peer, seattle.ls) { + if !peerIsPartner(sanfrancisco.Peer, seattle.Engine) { t.Fatal("Peer wasn't added as a Partner") } } -func peerIsPartner(p peer.Peer, ls *LedgerManager) bool { - for _, partner := range ls.Peers() { +func peerIsPartner(p peer.Peer, e *Engine) bool { + for _, partner := range e.Peers() { if partner.Key() == p.Key() { return true } diff --git a/exchange/bitswap/strategy/ledger.go b/exchange/bitswap/decision/ledger.go similarity index 99% rename from exchange/bitswap/strategy/ledger.go rename to exchange/bitswap/decision/ledger.go index 649c1e73e..eea87af1f 100644 --- a/exchange/bitswap/strategy/ledger.go +++ b/exchange/bitswap/decision/ledger.go @@ -1,4 +1,4 @@ -package strategy +package decision import ( "time" diff --git a/exchange/bitswap/decision/ledger_test.go b/exchange/bitswap/decision/ledger_test.go new file mode 100644 index 000000000..a6dd04e35 --- /dev/null +++ b/exchange/bitswap/decision/ledger_test.go @@ -0,0 +1 @@ +package decision diff --git a/exchange/bitswap/strategy/taskqueue.go b/exchange/bitswap/decision/taskqueue.go similarity index 99% rename from exchange/bitswap/strategy/taskqueue.go rename to exchange/bitswap/decision/taskqueue.go index 69bb95cd4..1cf279ef7 100644 --- a/exchange/bitswap/strategy/taskqueue.go +++ b/exchange/bitswap/decision/taskqueue.go @@ -1,4 +1,4 @@ -package strategy +package decision import ( wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" diff --git a/exchange/bitswap/strategy/ledger_test.go b/exchange/bitswap/strategy/ledger_test.go deleted file mode 100644 index 4271d525c..000000000 --- a/exchange/bitswap/strategy/ledger_test.go +++ /dev/null @@ -1 +0,0 @@ -package strategy From 1d23e94f1670d88be70776eb0ef0ed12b7575fae Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:03:37 -0800 Subject: [PATCH 47/64] rm empty file License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/ledger_test.go | 1 - 1 file changed, 1 deletion(-) delete mode 100644 exchange/bitswap/decision/ledger_test.go diff --git a/exchange/bitswap/decision/ledger_test.go b/exchange/bitswap/decision/ledger_test.go deleted file mode 100644 index a6dd04e35..000000000 --- a/exchange/bitswap/decision/ledger_test.go +++ /dev/null @@ -1 +0,0 @@ -package decision From acc714823b300cfc3e08fcbcb2e69bbc8fb06714 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:07:58 -0800 Subject: [PATCH 48/64] rename to peerRequestQueue this opens up the possibility of having multiple queues. And for all outgoing messages to be managed by the decision engine License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index 3b81d2582..b8018eef0 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -22,9 +22,10 @@ type Envelope struct { } type Engine struct { - // FIXME taskqueue isn't threadsafe nor is it protected by a mutex. consider - // a way to avoid sharing the taskqueue between the worker and the receiver - taskqueue *taskQueue + // FIXME peerRequestQueue isn't threadsafe nor is it protected by a mutex. + // consider a way to avoid sharing the peerRequestQueue between the worker + // and the receiver + peerRequestQueue *taskQueue workSignal chan struct{} @@ -39,11 +40,11 @@ type Engine struct { func NewEngine(ctx context.Context, bs bstore.Blockstore) *Engine { e := &Engine{ - ledgerMap: make(map[u.Key]*ledger), - bs: bs, - taskqueue: newTaskQueue(), - outbox: make(chan Envelope, 4), // TODO extract constant - workSignal: make(chan struct{}), + ledgerMap: make(map[u.Key]*ledger), + bs: bs, + peerRequestQueue: newTaskQueue(), + outbox: make(chan Envelope, 4), // TODO extract constant + workSignal: make(chan struct{}), } go e.taskWorker(ctx) return e @@ -51,7 +52,7 @@ func NewEngine(ctx context.Context, bs bstore.Blockstore) *Engine { func (e *Engine) taskWorker(ctx context.Context) { for { - nextTask := e.taskqueue.Pop() + nextTask := e.peerRequestQueue.Pop() if nextTask == nil { // No tasks in the list? // Wait until there are! @@ -128,11 +129,11 @@ func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { for _, entry := range m.Wantlist() { if entry.Cancel { l.CancelWant(entry.Key) - e.taskqueue.Remove(entry.Key, p) + e.peerRequestQueue.Remove(entry.Key, p) } else { l.Wants(entry.Key, entry.Priority) newWorkExists = true - e.taskqueue.Push(entry.Key, entry.Priority, p) + e.peerRequestQueue.Push(entry.Key, entry.Priority, p) } } @@ -142,7 +143,7 @@ func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { for _, l := range e.ledgerMap { if l.WantListContains(block.Key()) { newWorkExists = true - e.taskqueue.Push(block.Key(), 1, l.Partner) + e.peerRequestQueue.Push(block.Key(), 1, l.Partner) } } } @@ -163,7 +164,7 @@ func (e *Engine) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { for _, block := range m.Blocks() { l.SentBytes(len(block.Data)) l.wantList.Remove(block.Key()) - e.taskqueue.Remove(block.Key(), p) + e.peerRequestQueue.Remove(block.Key(), p) } return nil From 8d4d5b86ef590c8ef461002a1895c3da61ecb8bf Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:32:04 -0800 Subject: [PATCH 49/64] fix: don't sort the output of Entries() only sort SortedEntries() License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/wantlist/wantlist.go | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 6ef018668..22b2c1c2c 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -63,26 +63,13 @@ func (w *ThreadSafe) Contains(k u.Key) bool { func (w *ThreadSafe) Entries() []*Entry { w.lk.RLock() defer w.lk.RUnlock() - var es entrySlice - for _, e := range w.set { - es = append(es, e) - } - // TODO rename SortedEntries (state that they're sorted so callers know - // they're paying an expense) - sort.Sort(es) - return es + return w.Wantlist.Entries() } func (w *ThreadSafe) SortedEntries() []*Entry { w.lk.RLock() defer w.lk.RUnlock() - var es entrySlice - - for _, e := range w.set { - es = append(es, e) - } - sort.Sort(es) - return es + return w.Wantlist.SortedEntries() } func (w *Wantlist) Add(k u.Key, priority int) { @@ -106,17 +93,14 @@ func (w *Wantlist) Contains(k u.Key) bool { func (w *Wantlist) Entries() []*Entry { var es entrySlice - for _, e := range w.set { es = append(es, e) } - sort.Sort(es) return es } func (w *Wantlist) SortedEntries() []*Entry { var es entrySlice - for _, e := range w.set { es = append(es, e) } From 1b1260b658a969baece8de9380bfff473b2938ec Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:39:39 -0800 Subject: [PATCH 50/64] rm unused method License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 10 ---------- exchange/bitswap/decision/engine_test.go | 17 ----------------- 2 files changed, 27 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index b8018eef0..e34b6a225 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -96,16 +96,6 @@ func (e *Engine) Peers() []peer.Peer { return response } -// BlockIsWantedByPeer returns true if peer wants the block given by this -// key -func (e *Engine) BlockIsWantedByPeer(k u.Key, p peer.Peer) bool { - e.lock.RLock() - defer e.lock.RUnlock() - - ledger := e.findOrCreate(p) - return ledger.WantListContains(k) -} - // MessageReceived performs book-keeping. Returns error if passed invalid // arguments. func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { diff --git a/exchange/bitswap/decision/engine_test.go b/exchange/bitswap/decision/engine_test.go index 592236c3e..5b1740754 100644 --- a/exchange/bitswap/decision/engine_test.go +++ b/exchange/bitswap/decision/engine_test.go @@ -60,23 +60,6 @@ func TestConsistentAccounting(t *testing.T) { } } -func TestBlockRecordedAsWantedAfterMessageReceived(t *testing.T) { - beggar := newPeerAndLedgermanager("can't be chooser") - chooser := newPeerAndLedgermanager("chooses JIF") - - block := blocks.NewBlock([]byte("data wanted by beggar")) - - messageFromBeggarToChooser := message.New() - messageFromBeggarToChooser.AddEntry(block.Key(), 1) - - chooser.Engine.MessageReceived(beggar.Peer, messageFromBeggarToChooser) - // for this test, doesn't matter if you record that beggar sent - - if !chooser.Engine.BlockIsWantedByPeer(block.Key(), beggar.Peer) { - t.Fatal("chooser failed to record that beggar wants block") - } -} - func TestPeerIsAddedToPeersWhenMessageReceivedOrSent(t *testing.T) { sanfrancisco := newPeerAndLedgermanager("sf") From 9c301a2d770dc3182914b4820ff4a293823243cf Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:41:02 -0800 Subject: [PATCH 51/64] add comment License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/wantlist/wantlist.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 22b2c1c2c..1bf662102 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -17,6 +17,8 @@ type Wantlist struct { } type Entry struct { + // TODO consider making entries immutable so they can be shared safely and + // slices can be copied efficiently. Key u.Key Priority int } From 6e7c46a6e29122df2f5837adcbd875eaf97b07f8 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:43:23 -0800 Subject: [PATCH 52/64] unexport functions License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 12 ++++-------- exchange/bitswap/decision/engine_test.go | 6 +++--- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index e34b6a225..1a46d4535 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -160,17 +160,13 @@ func (e *Engine) MessageSent(p peer.Peer, m bsmsg.BitSwapMessage) error { return nil } -func (e *Engine) NumBytesSentTo(p peer.Peer) uint64 { - e.lock.RLock() - defer e.lock.RUnlock() - +func (e *Engine) numBytesSentTo(p peer.Peer) uint64 { + // NB not threadsafe return e.findOrCreate(p).Accounting.BytesSent } -func (e *Engine) NumBytesReceivedFrom(p peer.Peer) uint64 { - e.lock.RLock() - defer e.lock.RUnlock() - +func (e *Engine) numBytesReceivedFrom(p peer.Peer) uint64 { + // NB not threadsafe return e.findOrCreate(p).Accounting.BytesRecv } diff --git a/exchange/bitswap/decision/engine_test.go b/exchange/bitswap/decision/engine_test.go index 5b1740754..148937573 100644 --- a/exchange/bitswap/decision/engine_test.go +++ b/exchange/bitswap/decision/engine_test.go @@ -44,18 +44,18 @@ func TestConsistentAccounting(t *testing.T) { } // Ensure sender records the change - if sender.Engine.NumBytesSentTo(receiver.Peer) == 0 { + if sender.Engine.numBytesSentTo(receiver.Peer) == 0 { t.Fatal("Sent bytes were not recorded") } // Ensure sender and receiver have the same values - if sender.Engine.NumBytesSentTo(receiver.Peer) != receiver.Engine.NumBytesReceivedFrom(sender.Peer) { + if sender.Engine.numBytesSentTo(receiver.Peer) != receiver.Engine.numBytesReceivedFrom(sender.Peer) { t.Fatal("Inconsistent book-keeping. Strategies don't agree") } // Ensure sender didn't record receving anything. And that the receiver // didn't record sending anything - if receiver.Engine.NumBytesSentTo(sender.Peer) != 0 || sender.Engine.NumBytesReceivedFrom(receiver.Peer) != 0 { + if receiver.Engine.numBytesSentTo(sender.Peer) != 0 || sender.Engine.numBytesReceivedFrom(receiver.Peer) != 0 { t.Fatal("Bert didn't send bytes to Ernie") } } From 2b60b641c4dc031f246293d63f03ed52d15fe65d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:54:24 -0800 Subject: [PATCH 53/64] fix: check blockstore before adding task addresses https://github.com/jbenet/go-ipfs/pull/438#discussion_r21953742 License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index 1a46d4535..29ee9dce2 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -122,8 +122,10 @@ func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { e.peerRequestQueue.Remove(entry.Key, p) } else { l.Wants(entry.Key, entry.Priority) - newWorkExists = true - e.peerRequestQueue.Push(entry.Key, entry.Priority, p) + if exists, err := e.bs.Has(entry.Key); err == nil && exists { + newWorkExists = true + e.peerRequestQueue.Push(entry.Key, entry.Priority, p) + } } } From e36d656632db4550b6e31bc8b270c11faef95e0d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Tue, 16 Dec 2014 23:57:28 -0800 Subject: [PATCH 54/64] log unusual event License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index 29ee9dce2..d50c5c0c6 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -65,7 +65,8 @@ func (e *Engine) taskWorker(ctx context.Context) { } block, err := e.bs.Get(nextTask.Entry.Key) if err != nil { - continue // TODO maybe return an error + log.Warning("engine: task exists to send block, but block is not in blockstore") + continue } // construct message here so we can make decisions about any additional // information we may want to include at this time. From 45faa4d7a08c80726fa1da30ede2f296e033d99d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 00:09:47 -0800 Subject: [PATCH 55/64] fix: set peerset size addresses https://github.com/jbenet/go-ipfs/pull/438#discussion_r21952271 License: MIT Signed-off-by: Brian Tiger Chow --- util/peerset/peerset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/peerset/peerset.go b/util/peerset/peerset.go index e969b4a4b..37c1a97b0 100644 --- a/util/peerset/peerset.go +++ b/util/peerset/peerset.go @@ -22,7 +22,7 @@ func NewPeerSet() *PeerSet { func NewLimitedPeerSet(size int) *PeerSet { ps := new(PeerSet) ps.ps = make(map[string]bool) - ps.size = -1 + ps.size = size return ps } From 19764880d88b7fe9c19071242025eccf6b9ddde7 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 00:10:42 -0800 Subject: [PATCH 56/64] doc: peerset fixme not changing this because i don't want to write a test for it now License: MIT Signed-off-by: Brian Tiger Chow --- util/peerset/peerset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/peerset/peerset.go b/util/peerset/peerset.go index 37c1a97b0..9b132b235 100644 --- a/util/peerset/peerset.go +++ b/util/peerset/peerset.go @@ -7,7 +7,7 @@ import ( // PeerSet is a threadsafe set of peers type PeerSet struct { - ps map[string]bool + ps map[string]bool // FIXME can be map[string]struct{} lk sync.RWMutex size int } From 0545c4d15d4c680a01e6207c45a08480740fade9 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 00:24:59 -0800 Subject: [PATCH 57/64] refactor: *Entry -> Entry in many places, entries are assigned from one slice to another and in different goroutines. In one place, entries were modified (in the queue). To avoid shared mutable state, probably best to handle entries by value. License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/message/message.go | 12 ++++++------ exchange/bitswap/wantlist/wantlist.go | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/exchange/bitswap/message/message.go b/exchange/bitswap/message/message.go index 245fc35fb..7f7f1d08e 100644 --- a/exchange/bitswap/message/message.go +++ b/exchange/bitswap/message/message.go @@ -19,7 +19,7 @@ import ( type BitSwapMessage interface { // Wantlist returns a slice of unique keys that represent data wanted by // the sender. - Wantlist() []*Entry + Wantlist() []Entry // Blocks returns a slice of unique blocks Blocks() []*blocks.Block @@ -48,7 +48,7 @@ type Exportable interface { type impl struct { full bool - wantlist map[u.Key]*Entry + wantlist map[u.Key]Entry blocks map[u.Key]*blocks.Block // map to detect duplicates } @@ -59,7 +59,7 @@ func New() BitSwapMessage { func newMsg() *impl { return &impl{ blocks: make(map[u.Key]*blocks.Block), - wantlist: make(map[u.Key]*Entry), + wantlist: make(map[u.Key]Entry), full: true, } } @@ -90,8 +90,8 @@ func (m *impl) Full() bool { return m.full } -func (m *impl) Wantlist() []*Entry { - var out []*Entry +func (m *impl) Wantlist() []Entry { + var out []Entry for _, e := range m.wantlist { out = append(out, e) } @@ -120,7 +120,7 @@ func (m *impl) addEntry(k u.Key, priority int, cancel bool) { e.Priority = priority e.Cancel = cancel } else { - m.wantlist[k] = &Entry{ + m.wantlist[k] = Entry{ Entry: wantlist.Entry{ Key: k, Priority: priority, diff --git a/exchange/bitswap/wantlist/wantlist.go b/exchange/bitswap/wantlist/wantlist.go index 1bf662102..aa58ee155 100644 --- a/exchange/bitswap/wantlist/wantlist.go +++ b/exchange/bitswap/wantlist/wantlist.go @@ -13,7 +13,7 @@ type ThreadSafe struct { // not threadsafe type Wantlist struct { - set map[u.Key]*Entry + set map[u.Key]Entry } type Entry struct { @@ -23,7 +23,7 @@ type Entry struct { Priority int } -type entrySlice []*Entry +type entrySlice []Entry func (es entrySlice) Len() int { return len(es) } func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] } @@ -37,7 +37,7 @@ func NewThreadSafe() *ThreadSafe { func New() *Wantlist { return &Wantlist{ - set: make(map[u.Key]*Entry), + set: make(map[u.Key]Entry), } } @@ -62,13 +62,13 @@ func (w *ThreadSafe) Contains(k u.Key) bool { return w.Wantlist.Contains(k) } -func (w *ThreadSafe) Entries() []*Entry { +func (w *ThreadSafe) Entries() []Entry { w.lk.RLock() defer w.lk.RUnlock() return w.Wantlist.Entries() } -func (w *ThreadSafe) SortedEntries() []*Entry { +func (w *ThreadSafe) SortedEntries() []Entry { w.lk.RLock() defer w.lk.RUnlock() return w.Wantlist.SortedEntries() @@ -78,7 +78,7 @@ func (w *Wantlist) Add(k u.Key, priority int) { if _, ok := w.set[k]; ok { return } - w.set[k] = &Entry{ + w.set[k] = Entry{ Key: k, Priority: priority, } @@ -93,7 +93,7 @@ func (w *Wantlist) Contains(k u.Key) bool { return ok } -func (w *Wantlist) Entries() []*Entry { +func (w *Wantlist) Entries() []Entry { var es entrySlice for _, e := range w.set { es = append(es, e) @@ -101,7 +101,7 @@ func (w *Wantlist) Entries() []*Entry { return es } -func (w *Wantlist) SortedEntries() []*Entry { +func (w *Wantlist) SortedEntries() []Entry { var es entrySlice for _, e := range w.set { es = append(es, e) From 4bcfe094a13d8af0c4b2b0f39731eb12714c5610 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 01:52:37 -0800 Subject: [PATCH 58/64] extract constants License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/bitswap.go | 3 ++- exchange/bitswap/decision/engine.go | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index d0e49d182..11c6affa8 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -29,6 +29,7 @@ const ( maxProvidersPerRequest = 3 providerRequestTimeout = time.Second * 10 hasBlockTimeout = time.Second * 15 + sizeBatchRequestChan = 32 ) var ( @@ -59,7 +60,7 @@ func New(parent context.Context, p peer.Peer, network bsnet.BitSwapNetwork, rout routing: routing, sender: network, wantlist: wantlist.NewThreadSafe(), - batchRequests: make(chan []u.Key, 32), + batchRequests: make(chan []u.Key, sizeBatchRequestChan), } network.SetDelegate(bs) go bs.clientWorker(ctx) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index d50c5c0c6..aade14955 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -13,6 +13,10 @@ import ( var log = u.Logger("engine") +const ( + sizeOutboxChan = 4 +) + // Envelope contains a message for a Peer type Envelope struct { // Peer is the intended recipient @@ -43,7 +47,7 @@ func NewEngine(ctx context.Context, bs bstore.Blockstore) *Engine { ledgerMap: make(map[u.Key]*ledger), bs: bs, peerRequestQueue: newTaskQueue(), - outbox: make(chan Envelope, 4), // TODO extract constant + outbox: make(chan Envelope, sizeOutboxChan), workSignal: make(chan struct{}), } go e.taskWorker(ctx) From 175513e2811950c44d45c22aebe1868f2dbfe60b Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 02:24:16 -0800 Subject: [PATCH 59/64] refactor(bs/decision.Engine): pass in Entry License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 4 ++-- exchange/bitswap/decision/taskqueue.go | 13 +++++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index aade14955..813268f5b 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -129,7 +129,7 @@ func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { l.Wants(entry.Key, entry.Priority) if exists, err := e.bs.Has(entry.Key); err == nil && exists { newWorkExists = true - e.peerRequestQueue.Push(entry.Key, entry.Priority, p) + e.peerRequestQueue.Push(entry.Entry, p) } } } @@ -140,7 +140,7 @@ func (e *Engine) MessageReceived(p peer.Peer, m bsmsg.BitSwapMessage) error { for _, l := range e.ledgerMap { if l.WantListContains(block.Key()) { newWorkExists = true - e.peerRequestQueue.Push(block.Key(), 1, l.Partner) + e.peerRequestQueue.Push(wl.Entry{block.Key(), 1}, l.Partner) } } } diff --git a/exchange/bitswap/decision/taskqueue.go b/exchange/bitswap/decision/taskqueue.go index 1cf279ef7..b6341c9b2 100644 --- a/exchange/bitswap/decision/taskqueue.go +++ b/exchange/bitswap/decision/taskqueue.go @@ -28,22 +28,19 @@ type task struct { } // Push currently adds a new task to the end of the list -func (tl *taskQueue) Push(block u.Key, priority int, to peer.Peer) { - if task, ok := tl.taskmap[taskKey(to, block)]; ok { +func (tl *taskQueue) Push(entry wantlist.Entry, to peer.Peer) { + if task, ok := tl.taskmap[taskKey(to, entry.Key)]; ok { // TODO: when priority queue is implemented, // rearrange this task - task.Entry.Priority = priority + task.Entry.Priority = entry.Priority return } task := &task{ - Entry: wantlist.Entry{ - Key: block, - Priority: priority, - }, + Entry: entry, Target: to, } tl.tasks = append(tl.tasks, task) - tl.taskmap[taskKey(to, block)] = task + tl.taskmap[taskKey(to, entry.Key)] = task } // Pop 'pops' the next task to be performed. Returns nil no task exists. From ce2d0a2258bb714cf48e6fc445c696434db2f0d3 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 02:24:54 -0800 Subject: [PATCH 60/64] fix: add lock to taskQueue @whyrusleeping may wanna have a look and make sure i didn't screw anything up here BenchmarkInstantaneousAddCat1MB-4 200 10763761 ns/op 97.42 MB/s BenchmarkInstantaneousAddCat2MB-4 panic: runtime error: invalid memory address or nil pointer dereference [signal 0xb code=0x1 addr=0x0 pc=0xbedd] goroutine 14297 [running]: github.com/jbenet/go-ipfs/exchange/bitswap/decision.(*taskQueue).Remove(0xc2087553a0, 0xc2085ef200, 0x22, 0x56f570, 0xc208367a40) /Users/btc/go/src/github.com/jbenet/go-ipfs/exchange/bitswap/decision/taskqueue.go:66 +0x82 github.com/jbenet/go-ipfs/exchange/bitswap/decision.(*Engine).MessageSent(0xc20871b5c0, 0x56f570, 0xc208367a40, 0x570040, 0xc208753d40, 0x0, 0x0) /Users/btc/go/src/github.com/jbenet/go-ipfs/exchange/bitswap/decision/engine.go:177 +0x29e github.com/jbenet/go-ipfs/exchange/bitswap.(*bitswap).send(0xc20871b7a0, 0x56f4d8, 0xc208379800, 0x56f570, 0xc208367a40, 0x570040, 0xc208753d40, 0x0, 0x0) /Users/btc/go/src/github.com/jbenet/go-ipfs/exchange/bitswap/bitswap.go:352 +0x11c github.com/jbenet/go-ipfs/exchange/bitswap.(*bitswap).taskWorker(0xc20871b7a0, 0x56f4d8, 0xc208379800) /Users/btc/go/src/github.com/jbenet/go-ipfs/exchange/bitswap/bitswap.go:238 +0x165 created by github.com/jbenet/go-ipfs/exchange/bitswap.New /Users/btc/go/src/github.com/jbenet/go-ipfs/exchange/bitswap/bitswap.go:66 +0x49e --- exchange/bitswap/decision/taskqueue.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/exchange/bitswap/decision/taskqueue.go b/exchange/bitswap/decision/taskqueue.go index b6341c9b2..a76c56e9b 100644 --- a/exchange/bitswap/decision/taskqueue.go +++ b/exchange/bitswap/decision/taskqueue.go @@ -1,6 +1,8 @@ package decision import ( + "sync" + wantlist "github.com/jbenet/go-ipfs/exchange/bitswap/wantlist" peer "github.com/jbenet/go-ipfs/peer" u "github.com/jbenet/go-ipfs/util" @@ -11,6 +13,7 @@ import ( // tasks (on getnext). For now, we are assuming a dumb/nice strategy. type taskQueue struct { // TODO: make this into a priority queue + lock sync.Mutex tasks []*task taskmap map[string]*task } @@ -29,6 +32,8 @@ type task struct { // Push currently adds a new task to the end of the list func (tl *taskQueue) Push(entry wantlist.Entry, to peer.Peer) { + tl.lock.Lock() + defer tl.lock.Unlock() if task, ok := tl.taskmap[taskKey(to, entry.Key)]; ok { // TODO: when priority queue is implemented, // rearrange this task @@ -45,6 +50,8 @@ func (tl *taskQueue) Push(entry wantlist.Entry, to peer.Peer) { // Pop 'pops' the next task to be performed. Returns nil no task exists. func (tl *taskQueue) Pop() *task { + tl.lock.Lock() + defer tl.lock.Unlock() var out *task for len(tl.tasks) > 0 { // TODO: instead of zero, use exponential distribution @@ -63,10 +70,12 @@ func (tl *taskQueue) Pop() *task { // Remove lazily removes a task from the queue func (tl *taskQueue) Remove(k u.Key, p peer.Peer) { + tl.lock.Lock() t, ok := tl.taskmap[taskKey(p, k)] if ok { t.Trash = true } + tl.lock.Unlock() } // taskKey returns a key that uniquely identifies a task. From bd3ee739b92d9a3740ee81e2a4eaad9eb22fafa8 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 03:11:57 -0800 Subject: [PATCH 61/64] doc: some comments about the future of the decision engine License: MIT Signed-off-by: Brian Tiger Chow --- exchange/bitswap/decision/engine.go | 44 ++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/decision/engine.go b/exchange/bitswap/decision/engine.go index 813268f5b..ea4539437 100644 --- a/exchange/bitswap/decision/engine.go +++ b/exchange/bitswap/decision/engine.go @@ -11,6 +11,36 @@ import ( u "github.com/jbenet/go-ipfs/util" ) +// TODO consider taking responsibility for other types of requests. For +// example, there could be a |cancelQueue| for all of the cancellation +// messages that need to go out. There could also be a |wantlistQueue| for +// the local peer's wantlists. Alternatively, these could all be bundled +// into a single, intelligent global queue that efficiently +// batches/combines and takes all of these into consideration. +// +// Right now, messages go onto the network for four reasons: +// 1. an initial `sendwantlist` message to a provider of the first key in a request +// 2. a periodic full sweep of `sendwantlist` messages to all providers +// 3. upon receipt of blocks, a `cancel` message to all peers +// 4. draining the priority queue of `blockrequests` from peers +// +// Presently, only `blockrequests` are handled by the decision engine. +// However, there is an opportunity to give it more responsibility! If the +// decision engine is given responsibility for all of the others, it can +// intelligently decide how to combine requests efficiently. +// +// Some examples of what would be possible: +// +// * when sending out the wantlists, include `cancel` requests +// * when handling `blockrequests`, include `sendwantlist` and `cancel` as appropriate +// * when handling `cancel`, if we recently received a wanted block from a +// peer, include a partial wantlist that contains a few other high priority +// blocks +// +// In a sense, if we treat the decision engine as a black box, it could do +// whatever it sees fit to produce desired outcomes (get wanted keys +// quickly, maintain good relationships with peers, etc). + var log = u.Logger("engine") const ( @@ -26,18 +56,24 @@ type Envelope struct { } type Engine struct { - // FIXME peerRequestQueue isn't threadsafe nor is it protected by a mutex. - // consider a way to avoid sharing the peerRequestQueue between the worker - // and the receiver + // peerRequestQueue is a priority queue of requests received from peers. + // Requests are popped from the queue, packaged up, and placed in the + // outbox. peerRequestQueue *taskQueue + // FIXME it's a bit odd for the client and the worker to both share memory + // (both modify the peerRequestQueue) and also to communicate over the + // workSignal channel. consider sending requests over the channel and + // allowing the worker to have exclusive access to the peerRequestQueue. In + // that case, no lock would be required. workSignal chan struct{} + // outbox contains outgoing messages to peers outbox chan Envelope bs bstore.Blockstore - lock sync.RWMutex + lock sync.RWMutex // protects the fields immediatly below // ledgerMap lists Ledgers by their Partner key. ledgerMap map[u.Key]*ledger } From 8100582e387d8a0fd1cdbeabcc41ea15dba7f05d Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 03:37:49 -0800 Subject: [PATCH 62/64] fix: batches of blocks have equal priority addresses... https://github.com/jbenet/go-ipfs/pull/438/files#r21878994 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 11c6affa8..149996b3a 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -3,6 +3,7 @@ package bitswap import ( + "math" "sync" "time" @@ -30,6 +31,8 @@ const ( providerRequestTimeout = time.Second * 10 hasBlockTimeout = time.Second * 15 sizeBatchRequestChan = 32 + // kMaxPriority is the max priority as defined by the bitswap protocol + kMaxPriority = math.MaxInt32 ) var ( @@ -261,7 +264,7 @@ func (bs *bitswap) clientWorker(parent context.Context) { continue } for i, k := range ks { - bs.wantlist.Add(k, len(ks)-i) + bs.wantlist.Add(k, kMaxPriority-i) } // 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 From 9328fbaf8141f2ff31eda6feb38518ddeb79fe19 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 17 Dec 2014 19:27:41 +0000 Subject: [PATCH 63/64] clean peerset constructor names --- exchange/bitswap/bitswap.go | 2 +- routing/dht/routing.go | 2 +- util/peerset/peerset.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 149996b3a..912ed1210 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -212,7 +212,7 @@ func (bs *bitswap) sendWantlistToProviders(ctx context.Context, wantlist *wantli message.AddEntry(e.Key, e.Priority) } - ps := pset.NewPeerSet() + ps := pset.New() // Get providers for all entries in wantlist (could take a while) wg := sync.WaitGroup{} diff --git a/routing/dht/routing.go b/routing/dht/routing.go index bfe38e200..51f15ff21 100644 --- a/routing/dht/routing.go +++ b/routing/dht/routing.go @@ -141,7 +141,7 @@ func (dht *IpfsDHT) FindProvidersAsync(ctx context.Context, key u.Key, count int func (dht *IpfsDHT) findProvidersAsyncRoutine(ctx context.Context, key u.Key, count int, peerOut chan peer.Peer) { defer close(peerOut) - ps := pset.NewLimitedPeerSet(count) + ps := pset.NewLimited(count) provs := dht.providers.GetProviders(ctx, key) for _, p := range provs { // NOTE: assuming that this list of peers is unique diff --git a/util/peerset/peerset.go b/util/peerset/peerset.go index 9b132b235..5dcf26682 100644 --- a/util/peerset/peerset.go +++ b/util/peerset/peerset.go @@ -12,14 +12,14 @@ type PeerSet struct { size int } -func NewPeerSet() *PeerSet { +func New() *PeerSet { ps := new(PeerSet) ps.ps = make(map[string]bool) ps.size = -1 return ps } -func NewLimitedPeerSet(size int) *PeerSet { +func NewLimited(size int) *PeerSet { ps := new(PeerSet) ps.ps = make(map[string]bool) ps.size = size From 9fafec1256a6eea7f42d8886e0b463a94778b1a6 Mon Sep 17 00:00:00 2001 From: Brian Tiger Chow Date: Wed, 17 Dec 2014 16:53:00 -0800 Subject: [PATCH 64/64] do not run epic tests in parallel @whyrusleeping License: MIT Signed-off-by: Brian Tiger Chow --- epictest/addcat_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/epictest/addcat_test.go b/epictest/addcat_test.go index a69660cf4..09ba58f79 100644 --- a/epictest/addcat_test.go +++ b/epictest/addcat_test.go @@ -26,10 +26,6 @@ import ( const kSeed = 1 func Test100MBInstantaneous(t *testing.T) { - t.Log("a sanity check") - - t.Parallel() - conf := Config{ NetworkLatency: 0, RoutingLatency: 0, @@ -41,10 +37,7 @@ func Test100MBInstantaneous(t *testing.T) { func TestDegenerateSlowBlockstore(t *testing.T) { SkipUnlessEpic(t) - t.Parallel() - conf := Config{BlockstoreLatency: 50 * time.Millisecond} - if err := AddCatPowers(conf, 128); err != nil { t.Fatal(err) } @@ -52,10 +45,7 @@ func TestDegenerateSlowBlockstore(t *testing.T) { func TestDegenerateSlowNetwork(t *testing.T) { SkipUnlessEpic(t) - t.Parallel() - conf := Config{NetworkLatency: 400 * time.Millisecond} - if err := AddCatPowers(conf, 128); err != nil { t.Fatal(err) } @@ -63,10 +53,7 @@ func TestDegenerateSlowNetwork(t *testing.T) { func TestDegenerateSlowRouting(t *testing.T) { SkipUnlessEpic(t) - t.Parallel() - conf := Config{RoutingLatency: 400 * time.Millisecond} - if err := AddCatPowers(conf, 128); err != nil { t.Fatal(err) } @@ -74,10 +61,7 @@ func TestDegenerateSlowRouting(t *testing.T) { func Test100MBMacbookCoastToCoast(t *testing.T) { SkipUnlessEpic(t) - t.Parallel() - conf := Config{}.Network_NYtoSF().Blockstore_SlowSSD2014().Routing_Slow() - if err := AddCatBytes(RandomBytes(100*1024*1024), conf); err != nil { t.Fatal(err) }