From 86fb07aed7d32ccd1a33f8bab0ea47aa3adbb359 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 29 Apr 2015 01:36:47 -0700 Subject: [PATCH 01/14] try harder to not send duplicate blocks --- .../bitswap/decision/peer_request_queue.go | 28 +++++++++++++++---- exchange/bitswap/workers.go | 2 +- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/exchange/bitswap/decision/peer_request_queue.go b/exchange/bitswap/decision/peer_request_queue.go index e771ece0bb6..42928487dcc 100644 --- a/exchange/bitswap/decision/peer_request_queue.go +++ b/exchange/bitswap/decision/peer_request_queue.go @@ -46,7 +46,7 @@ func (tl *prq) Push(entry wantlist.Entry, to peer.ID) { defer tl.lock.Unlock() partner, ok := tl.partners[to] if !ok { - partner = &activePartner{taskQueue: pq.New(wrapCmp(V1))} + partner = newActivePartner() tl.pQueue.Push(partner) tl.partners[to] = partner } @@ -57,12 +57,19 @@ func (tl *prq) Push(entry wantlist.Entry, to peer.ID) { return } + partner.activelk.Lock() + defer partner.activelk.Unlock() + _, ok = partner.activeBlocks[entry.Key] + if ok { + return + } + task := &peerRequestTask{ Entry: entry, Target: to, created: time.Now(), Done: func() { - partner.TaskDone() + partner.TaskDone(entry.Key) tl.lock.Lock() tl.pQueue.Update(partner.Index()) tl.lock.Unlock() @@ -93,7 +100,7 @@ func (tl *prq) Pop() *peerRequestTask { continue // discarding tasks that have been removed } - partner.StartTask() + partner.StartTask(out.Entry.Key) partner.requests-- break // and return |out| } @@ -179,6 +186,8 @@ type activePartner struct { activelk sync.Mutex active int + activeBlocks map[u.Key]struct{} + // requests is the number of blocks this peer is currently requesting // request need not be locked around as it will only be modified under // the peerRequestQueue's locks @@ -191,6 +200,13 @@ type activePartner struct { taskQueue pq.PQ } +func newActivePartner() *activePartner { + return &activePartner{ + taskQueue: pq.New(wrapCmp(V1)), + activeBlocks: make(map[u.Key]struct{}), + } +} + // partnerCompare implements pq.ElemComparator func partnerCompare(a, b pq.Elem) bool { pa := a.(*activePartner) @@ -208,15 +224,17 @@ func partnerCompare(a, b pq.Elem) bool { } // StartTask signals that a task was started for this partner -func (p *activePartner) StartTask() { +func (p *activePartner) StartTask(k u.Key) { p.activelk.Lock() + p.activeBlocks[k] = struct{}{} p.active++ p.activelk.Unlock() } // TaskDone signals that a task was completed for this partner -func (p *activePartner) TaskDone() { +func (p *activePartner) TaskDone(k u.Key) { p.activelk.Lock() + delete(p.activeBlocks, k) p.active-- if p.active < 0 { panic("more tasks finished than started!") diff --git a/exchange/bitswap/workers.go b/exchange/bitswap/workers.go index 4e2bf43b87e..1fc59a21462 100644 --- a/exchange/bitswap/workers.go +++ b/exchange/bitswap/workers.go @@ -11,7 +11,7 @@ import ( u "github.com/ipfs/go-ipfs/util" ) -var TaskWorkerCount = 16 +var TaskWorkerCount = 8 func init() { twc := os.Getenv("IPFS_BITSWAP_TASK_WORKERS") From f998339acb8a5a74eac708576d66fcd062804a0d Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 29 Apr 2015 19:59:18 -0700 Subject: [PATCH 02/14] remove some redundant blockputs to avoid false duplicate block receives --- exchange/bitswap/bitswap.go | 9 +++++++++ exchange/bitswap/bitswap_test.go | 6 +----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 37826c4928a..937ee131e5f 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -219,6 +219,15 @@ func (bs *Bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { return errors.New("bitswap is closed") default: } + has, err := bs.blockstore.Has(blk.Key()) + if err != nil { + return err + } + + if has { + log.Error(bs.self, "Dup Block! ", blk.Key()) + } + if err := bs.blockstore.Put(blk); err != nil { return err } diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 85b3c0ec8ee..85a8e9d5d22 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/exchange/bitswap/bitswap_test.go @@ -69,9 +69,6 @@ func TestGetBlockFromPeerAfterPeerAnnounces(t *testing.T) { hasBlock := g.Next() defer hasBlock.Exchange.Close() - if err := hasBlock.Blockstore().Put(block); err != nil { - t.Fatal(err) - } if err := hasBlock.Exchange.HasBlock(context.Background(), block); err != nil { t.Fatal(err) } @@ -136,7 +133,6 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { var blkeys []u.Key first := instances[0] for _, b := range blocks { - first.Blockstore().Put(b) // TODO remove. don't need to do this. bitswap owns block blkeys = append(blkeys, b.Key()) first.Exchange.HasBlock(context.Background(), b) } @@ -144,7 +140,7 @@ func PerformDistributionTest(t *testing.T, numInstances, numBlocks int) { t.Log("Distribute!") wg := sync.WaitGroup{} - for _, inst := range instances { + for _, inst := range instances[1:] { wg.Add(1) go func(inst Instance) { defer wg.Done() From 90a3252d3962a338bba332c1b4ddf909f4c07f9b Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 28 Apr 2015 19:40:06 -0700 Subject: [PATCH 03/14] fix append bug and overwrite/truncate bug an alternative fix to the truncate/append problem Update ipns_unix.go --- fuse/ipns/ipns_unix.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/fuse/ipns/ipns_unix.go b/fuse/ipns/ipns_unix.go index 9a7234888a5..ebb10dac839 100644 --- a/fuse/ipns/ipns_unix.go +++ b/fuse/ipns/ipns_unix.go @@ -337,6 +337,20 @@ func (fi *File) Flush(ctx context.Context, req *fuse.FlushRequest) error { } } +func (fi *File) Setattr(ctx context.Context, req *fuse.SetattrRequest, resp *fuse.SetattrResponse) error { + cursize, err := fi.fi.Size() + if err != nil { + return err + } + if cursize != int64(req.Size) { + err := fi.fi.Truncate(int64(req.Size)) + if err != nil { + return err + } + } + return nil +} + // Fsync flushes the content in the file to disk, but does not // update the dag tree internally func (fi *File) Fsync(ctx context.Context, req *fuse.FsyncRequest) error { @@ -370,13 +384,21 @@ func (dir *Directory) Mkdir(ctx context.Context, req *fuse.MkdirRequest) (fs.Nod func (fi *File) Open(ctx context.Context, req *fuse.OpenRequest, resp *fuse.OpenResponse) (fs.Handle, error) { if req.Flags&fuse.OpenTruncate != 0 { - log.Warning("Need to truncate file!") + log.Info("Need to truncate file!") err := fi.fi.Truncate(0) if err != nil { return nil, err } } else if req.Flags&fuse.OpenAppend != 0 { - log.Warning("Need to append to file!") + log.Info("Need to append to file!") + + // seek(0) essentially resets the file object, this is required for appends to work + // properly + _, err := fi.fi.Seek(0, os.SEEK_SET) + if err != nil { + log.Error("seek reset failed: ", err) + return nil, err + } } return fi, nil } From 517e2d121f6ba93ccbef09134c02ceebfe6850cc Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 1 May 2015 23:24:47 -0700 Subject: [PATCH 04/14] quick fix for OOM panic that has been plaguing us --- p2p/crypto/secio/rw.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/p2p/crypto/secio/rw.go b/p2p/crypto/secio/rw.go index 5172bf0e8e7..959fd634a39 100644 --- a/p2p/crypto/secio/rw.go +++ b/p2p/crypto/secio/rw.go @@ -15,6 +15,10 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" ) +const MaxMsgSize = 8 * 1024 * 1024 + +var ErrMaxMessageSize = errors.New("attempted to read message larger than max size") + // ErrMACInvalid signals that a MAC verification failed var ErrMACInvalid = errors.New("MAC verification failed") @@ -130,6 +134,10 @@ func (r *etmReader) Read(buf []byte) (int, error) { return 0, err } + if fullLen > MaxMsgSize { + return 0, ErrMaxMessageSize + } + buf2 := buf changed := false // if not enough space, allocate a new buffer. From 6892eba511d9389501d92c683f67242e2ab86bed Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 2 May 2015 08:24:55 -0700 Subject: [PATCH 05/14] core/commands/internal/slice_util: Remove this unused package The last references to CastToReaders were commented out in 6faeee83 (cmds2/add: temp fix for -r. horrible hack, 2014-11-11) and then removed completely in 032e9c29 (core/commands2: Updated 'add' command for new file API, 2014-11-16). The last references to CastToStrings was removed in a0bd29d5 (core/commands2: Fixed swarm command for new arguments API, 2014-11-18). --- core/commands/internal/slice_util.go | 31 ---------------------------- 1 file changed, 31 deletions(-) delete mode 100644 core/commands/internal/slice_util.go diff --git a/core/commands/internal/slice_util.go b/core/commands/internal/slice_util.go deleted file mode 100644 index cfbdf718113..00000000000 --- a/core/commands/internal/slice_util.go +++ /dev/null @@ -1,31 +0,0 @@ -package internal - -import ( - "io" - - u "github.com/ipfs/go-ipfs/util" -) - -func CastToReaders(slice []interface{}) ([]io.Reader, error) { - readers := make([]io.Reader, 0) - for _, arg := range slice { - reader, ok := arg.(io.Reader) - if !ok { - return nil, u.ErrCast() - } - readers = append(readers, reader) - } - return readers, nil -} - -func CastToStrings(slice []interface{}) ([]string, error) { - strs := make([]string, 0) - for _, maybe := range slice { - str, ok := maybe.(string) - if !ok { - return nil, u.ErrCast() - } - strs = append(strs, str) - } - return strs, nil -} From 2a5b2f2f4a21c77d9b7f518d0607875d8af220c8 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 2 May 2015 22:35:10 +0200 Subject: [PATCH 06/14] parse_test: move helper functions License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 48 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index edbca7f7698..a1fbe13a34a 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -7,6 +7,30 @@ import ( "github.com/ipfs/go-ipfs/commands" ) +type kvs map[string]interface{} +type words []string + +func sameWords(a words, b words) bool { + for i, w := range a { + if w != b[i] { + return false + } + } + return true +} + +func sameKVs(a kvs, b kvs) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if v != b[k] { + return false + } + } + return true +} + func TestOptionParsing(t *testing.T) { subCmd := &commands.Command{} cmd := &commands.Command{ @@ -19,30 +43,6 @@ func TestOptionParsing(t *testing.T) { }, } - type kvs map[string]interface{} - type words []string - - sameWords := func(a words, b words) bool { - for i, w := range a { - if w != b[i] { - return false - } - } - return true - } - - sameKVs := func(a kvs, b kvs) bool { - if len(a) != len(b) { - return false - } - for k, v := range a { - if v != b[k] { - return false - } - } - return true - } - testHelper := func(args string, expectedOpts kvs, expectedWords words, expectErr bool) { _, opts, input, _, err := parseOpts(strings.Split(args, " "), cmd) if expectErr { From 3e4a06945f0af95f1ae99fe1a8478699be5ccc6d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 2 May 2015 22:47:03 +0200 Subject: [PATCH 07/14] parse_test: fix and test sameWords() License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index a1fbe13a34a..286a39170dd 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -11,6 +11,9 @@ type kvs map[string]interface{} type words []string func sameWords(a words, b words) bool { + if len(a) != len(b) { + return false + } for i, w := range a { if w != b[i] { return false @@ -31,6 +34,33 @@ func sameKVs(a kvs, b kvs) bool { return true } +func TestSameWords(t *testing.T) { + a := []string{"v1", "v2"} + b := []string{"v1", "v2", "v3"} + c := []string{"v2", "v3"} + d := []string{"v2"} + e := []string{"v2", "v3"} + f := []string{"v2", "v1"} + + test := func(a words, b words, v bool) { + if sameWords(a, b) != v { + t.Errorf("sameWords('%v', '%v') != %v", a, b, v) + } + } + + test(a, b, false) + test(a, a, true) + test(a, c, false) + test(b, c, false) + test(c, d, false) + test(c, e, true) + test(b, e, false) + test(a, b, false) + test(a, f, false) + test(e, f, false) + test(f, f, true) +} + func TestOptionParsing(t *testing.T) { subCmd := &commands.Command{} cmd := &commands.Command{ From d0752a714dcfc6887e6f37934aa6156965df64e7 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 2 May 2015 04:05:57 +0200 Subject: [PATCH 08/14] parse_test: add tests for stdin enabled arg Let's document how stdin enabled arguments currently work by adding some tests. License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 286a39170dd..92dca144bbc 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -3,6 +3,9 @@ package cli import ( "strings" "testing" + "io" + "io/ioutil" + "os" "github.com/ipfs/go-ipfs/commands" ) @@ -147,6 +150,11 @@ func TestArgumentParsing(t *testing.T) { commands.StringArg("b", true, false, "another arg"), }, }, + "stdinenabled": &commands.Command{ + Arguments: []commands.Argument{ + commands.StringArg("a", true, true, "some arg").EnableStdin(), + }, + }, }, } @@ -219,4 +227,31 @@ func TestArgumentParsing(t *testing.T) { if err == nil { t.Error("Should have failed (provided too many args, only takes 1)") } + + // Use a temp file to simulate stdin + fstdin, err := ioutil.TempFile("", "") + if err != nil { + t.Fatal(err) + } + defer os.Remove(fstdin.Name()) + + if _, err := io.WriteString(fstdin, "stdin1"); err != nil { + t.Fatal(err) + } + + _, _, _, err = Parse([]string{"stdinenabled", "value1", "value2"}, nil, rootCmd) + if err != nil { + t.Error("Should have passed") + t.Fatal(err) + } + _, _, _, err = Parse([]string{"stdinenabled"}, fstdin, rootCmd) + if err != nil { + t.Error("Should have passed") + t.Fatal(err) + } + _, _, _, err = Parse([]string{"stdinenabled", "value1"}, fstdin, rootCmd) + if err != nil { + t.Error("Should have passed") + t.Fatal(err) + } } From 58126c1c6ccd733c15170a1c9ce8536eff1e46f6 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 2 May 2015 04:50:42 +0200 Subject: [PATCH 09/14] parse_test: improve tests with stdin enabled arg Now also check that we get the right arguments from the parsing. License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 92dca144bbc..15635be419e 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -239,19 +239,23 @@ func TestArgumentParsing(t *testing.T) { t.Fatal(err) } - _, _, _, err = Parse([]string{"stdinenabled", "value1", "value2"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - t.Fatal(err) - } - _, _, _, err = Parse([]string{"stdinenabled"}, fstdin, rootCmd) - if err != nil { - t.Error("Should have passed") - t.Fatal(err) - } - _, _, _, err = Parse([]string{"stdinenabled", "value1"}, fstdin, rootCmd) - if err != nil { - t.Error("Should have passed") - t.Fatal(err) + test := func(cmd words, f *os.File, res words) { + if f != nil { + if _, err := f.Seek(0, os.SEEK_SET); err != nil { + t.Fatal(err) + } + } + req, _, _, err := Parse(cmd, f, rootCmd) + if err != nil { + t.Error("Command '%v' should have passed parsing", cmd) + } + if !sameWords(req.Arguments(), res) { + t.Errorf("Arguments parsed from '%v' are not '%v'", cmd, res) + } } + + test([]string{"stdinenabled", "value1", "value2"}, nil, []string{"value1", "value2"}) + test([]string{"stdinenabled"}, fstdin, []string{"stdin1"}) + test([]string{"stdinenabled", "value1"}, fstdin, []string{"stdin1", "value1"}) + test([]string{"stdinenabled", "value1", "value2"}, fstdin, []string{"stdin1", "value1", "value2"}) } From c6dcfaaf5d706b20b20320d0c9872a8e751c63b0 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 3 May 2015 10:14:56 +0200 Subject: [PATCH 10/14] parse_test: use fileToSimulateStdin() License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 15635be419e..27b1dfc5da0 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -229,14 +229,17 @@ func TestArgumentParsing(t *testing.T) { } // Use a temp file to simulate stdin - fstdin, err := ioutil.TempFile("", "") - if err != nil { - t.Fatal(err) - } - defer os.Remove(fstdin.Name()) + fileToSimulateStdin := func(t *testing.T, content string) (*os.File) { + fstdin, err := ioutil.TempFile("", "") + if err != nil { + t.Fatal(err) + } + defer os.Remove(fstdin.Name()) - if _, err := io.WriteString(fstdin, "stdin1"); err != nil { - t.Fatal(err) + if _, err := io.WriteString(fstdin, content); err != nil { + t.Fatal(err) + } + return fstdin } test := func(cmd words, f *os.File, res words) { @@ -255,6 +258,9 @@ func TestArgumentParsing(t *testing.T) { } test([]string{"stdinenabled", "value1", "value2"}, nil, []string{"value1", "value2"}) + + fstdin := fileToSimulateStdin(t, "stdin1") + test([]string{"stdinenabled"}, fstdin, []string{"stdin1"}) test([]string{"stdinenabled", "value1"}, fstdin, []string{"stdin1", "value1"}) test([]string{"stdinenabled", "value1", "value2"}, fstdin, []string{"stdin1", "value1", "value2"}) From 47a88f84294f66faa9550114766df9433ce518ed Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 3 May 2015 20:10:28 +0200 Subject: [PATCH 11/14] parse_test: improve existing tests License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 80 ++++++++++++++------------------------ 1 file changed, 29 insertions(+), 51 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 27b1dfc5da0..c9b4a11bdf1 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -158,28 +158,37 @@ func TestArgumentParsing(t *testing.T) { }, } - _, _, _, err := Parse([]string{"noarg"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") + test := func(cmd words, f *os.File, res words) { + if f != nil { + if _, err := f.Seek(0, os.SEEK_SET); err != nil { + t.Fatal(err) + } + } + req, _, _, err := Parse(cmd, f, rootCmd) + if err != nil { + t.Errorf("Command '%v' should have passed parsing", cmd) + } + if !sameWords(req.Arguments(), res) { + t.Errorf("Arguments parsed from '%v' are not '%v'", cmd, res) + } } - _, _, _, err = Parse([]string{"noarg", "value!"}, nil, rootCmd) + + test([]string{"noarg"}, nil, []string{}) + + _, _, _, err := Parse([]string{"noarg", "value!"}, nil, rootCmd) if err == nil { t.Error("Should have failed (provided an arg, but command didn't define any)") } - _, _, _, err = Parse([]string{"onearg", "value!"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } + test([]string{"onearg", "value!"}, nil, []string{"value!"}) + _, _, _, err = Parse([]string{"onearg"}, nil, rootCmd) if err == nil { t.Error("Should have failed (didn't provide any args, arg is required)") } - _, _, _, err = Parse([]string{"twoargs", "value1", "value2"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } + test([]string{"twoargs", "value1", "value2"}, nil, []string{"value1", "value2"}) + _, _, _, err = Parse([]string{"twoargs", "value!"}, nil, rootCmd) if err == nil { t.Error("Should have failed (only provided 1 arg, needs 2)") @@ -189,36 +198,20 @@ func TestArgumentParsing(t *testing.T) { t.Error("Should have failed (didn't provide any args, 2 required)") } - _, _, _, err = Parse([]string{"variadic", "value!"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } - _, _, _, err = Parse([]string{"variadic", "value1", "value2", "value3"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } + test([]string{"variadic", "value!"}, nil, []string{"value!"}) + test([]string{"variadic", "value1", "value2", "value3"}, nil, []string{"value1", "value2", "value3"}) + _, _, _, err = Parse([]string{"variadic"}, nil, rootCmd) if err == nil { t.Error("Should have failed (didn't provide any args, 1 required)") } - _, _, _, err = Parse([]string{"optional", "value!"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } - _, _, _, err = Parse([]string{"optional"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } + test([]string{"optional", "value!"}, nil, []string{"value!"}) + test([]string{"optional"}, nil, []string{}) + + test([]string{"reversedoptional", "value1", "value2"}, nil, []string{"value1", "value2"}) + test([]string{"reversedoptional", "value!"}, nil, []string{"value!"}) - _, _, _, err = Parse([]string{"reversedoptional", "value1", "value2"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } - _, _, _, err = Parse([]string{"reversedoptional", "value!"}, nil, rootCmd) - if err != nil { - t.Error("Should have passed") - } _, _, _, err = Parse([]string{"reversedoptional"}, nil, rootCmd) if err == nil { t.Error("Should have failed (didn't provide any args, 1 required)") @@ -242,21 +235,6 @@ func TestArgumentParsing(t *testing.T) { return fstdin } - test := func(cmd words, f *os.File, res words) { - if f != nil { - if _, err := f.Seek(0, os.SEEK_SET); err != nil { - t.Fatal(err) - } - } - req, _, _, err := Parse(cmd, f, rootCmd) - if err != nil { - t.Error("Command '%v' should have passed parsing", cmd) - } - if !sameWords(req.Arguments(), res) { - t.Errorf("Arguments parsed from '%v' are not '%v'", cmd, res) - } - } - test([]string{"stdinenabled", "value1", "value2"}, nil, []string{"value1", "value2"}) fstdin := fileToSimulateStdin(t, "stdin1") From 93f253e00b11921f593f21b001acd5f4c597d005 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 3 May 2015 20:18:40 +0200 Subject: [PATCH 12/14] parse_test: add testFail() to simplify tests License: MIT Signed-off-by: Christian Couder --- commands/cli/parse_test.go | 46 ++++++++++++-------------------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index c9b4a11bdf1..5248b4a2a79 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -173,38 +173,26 @@ func TestArgumentParsing(t *testing.T) { } } - test([]string{"noarg"}, nil, []string{}) - - _, _, _, err := Parse([]string{"noarg", "value!"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (provided an arg, but command didn't define any)") + testFail := func(cmd words, msg string) { + _, _, _, err := Parse(cmd, nil, rootCmd) + if err == nil { + t.Errorf("Should have failed: %v", msg) + } } - test([]string{"onearg", "value!"}, nil, []string{"value!"}) + test([]string{"noarg"}, nil, []string{}) + testFail([]string{"noarg", "value!"}, "provided an arg, but command didn't define any") - _, _, _, err = Parse([]string{"onearg"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (didn't provide any args, arg is required)") - } + test([]string{"onearg", "value!"}, nil, []string{"value!"}) + testFail([]string{"onearg"}, "didn't provide any args, arg is required") test([]string{"twoargs", "value1", "value2"}, nil, []string{"value1", "value2"}) - - _, _, _, err = Parse([]string{"twoargs", "value!"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (only provided 1 arg, needs 2)") - } - _, _, _, err = Parse([]string{"twoargs"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (didn't provide any args, 2 required)") - } + testFail([]string{"twoargs", "value!"}, "only provided 1 arg, needs 2") + testFail([]string{"twoargs"}, "didn't provide any args, 2 required") test([]string{"variadic", "value!"}, nil, []string{"value!"}) test([]string{"variadic", "value1", "value2", "value3"}, nil, []string{"value1", "value2", "value3"}) - - _, _, _, err = Parse([]string{"variadic"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (didn't provide any args, 1 required)") - } + testFail([]string{"variadic"}, "didn't provide any args, 1 required") test([]string{"optional", "value!"}, nil, []string{"value!"}) test([]string{"optional"}, nil, []string{}) @@ -212,14 +200,8 @@ func TestArgumentParsing(t *testing.T) { test([]string{"reversedoptional", "value1", "value2"}, nil, []string{"value1", "value2"}) test([]string{"reversedoptional", "value!"}, nil, []string{"value!"}) - _, _, _, err = Parse([]string{"reversedoptional"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (didn't provide any args, 1 required)") - } - _, _, _, err = Parse([]string{"reversedoptional", "value1", "value2", "value3"}, nil, rootCmd) - if err == nil { - t.Error("Should have failed (provided too many args, only takes 1)") - } + testFail([]string{"reversedoptional"}, "didn't provide any args, 1 required") + testFail([]string{"reversedoptional", "value1", "value2", "value3"}, "provided too many args, only takes 1") // Use a temp file to simulate stdin fileToSimulateStdin := func(t *testing.T, content string) (*os.File) { From 6f04302a48fe642e96afe7734230743cc59f2fe4 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 4 May 2015 03:12:17 -0700 Subject: [PATCH 13/14] remove logging of dup blocks, move to counters for bitswap stat --- core/commands/bitswap.go | 2 ++ exchange/bitswap/bitswap.go | 15 +++++++-------- exchange/bitswap/stat.go | 10 +++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/core/commands/bitswap.go b/core/commands/bitswap.go index f89f38d7f3d..cfc522bae8a 100644 --- a/core/commands/bitswap.go +++ b/core/commands/bitswap.go @@ -101,6 +101,8 @@ var bitswapStatCmd = &cmds.Command{ buf := new(bytes.Buffer) fmt.Fprintln(buf, "bitswap status") fmt.Fprintf(buf, "\tprovides buffer: %d / %d\n", out.ProvideBufLen, bitswap.HasBlockBufferSize) + fmt.Fprintf(buf, "\tblocks received: %d\n", out.BlocksReceived) + fmt.Fprintf(buf, "\tdup blocks received: %d\n", out.DupBlksReceived) fmt.Fprintf(buf, "\twantlist [%d keys]\n", len(out.Wantlist)) for _, k := range out.Wantlist { fmt.Fprintf(buf, "\t\t%s\n", k.B58String()) diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 937ee131e5f..8b12a472778 100644 --- a/exchange/bitswap/bitswap.go +++ b/exchange/bitswap/bitswap.go @@ -127,6 +127,9 @@ type Bitswap struct { newBlocks chan *blocks.Block provideKeys chan u.Key + + blocksRecvd int + dupBlocksRecvd int } type blockRequest struct { @@ -219,14 +222,6 @@ func (bs *Bitswap) HasBlock(ctx context.Context, blk *blocks.Block) error { return errors.New("bitswap is closed") default: } - has, err := bs.blockstore.Has(blk.Key()) - if err != nil { - return err - } - - if has { - log.Error(bs.self, "Dup Block! ", blk.Key()) - } if err := bs.blockstore.Put(blk); err != nil { return err @@ -351,6 +346,10 @@ func (bs *Bitswap) ReceiveMessage(ctx context.Context, p peer.ID, incoming bsmsg // Should only track *useful* messages in ledger for _, block := range incoming.Blocks() { + bs.blocksRecvd++ + if has, err := bs.blockstore.Has(block.Key()); err == nil && has { + bs.dupBlocksRecvd++ + } hasBlockCtx, cancel := context.WithTimeout(ctx, hasBlockTimeout) if err := bs.HasBlock(hasBlockCtx, block); err != nil { log.Debug(err) diff --git a/exchange/bitswap/stat.go b/exchange/bitswap/stat.go index 1c5fec62b98..ceab4b2ee2a 100644 --- a/exchange/bitswap/stat.go +++ b/exchange/bitswap/stat.go @@ -6,15 +6,19 @@ import ( ) type Stat struct { - ProvideBufLen int - Wantlist []u.Key - Peers []string + ProvideBufLen int + Wantlist []u.Key + Peers []string + BlocksReceived int + DupBlksReceived int } func (bs *Bitswap) Stat() (*Stat, error) { st := new(Stat) st.ProvideBufLen = len(bs.newBlocks) st.Wantlist = bs.GetWantlist() + st.BlocksReceived = bs.blocksRecvd + st.DupBlksReceived = bs.dupBlocksRecvd for _, p := range bs.engine.Peers() { st.Peers = append(st.Peers, p.Pretty()) From 9cf8c5cbc9c4c4679daf1f8b42739eb0e663c50b Mon Sep 17 00:00:00 2001 From: Jeromy Date: Sun, 3 May 2015 16:39:41 -0700 Subject: [PATCH 14/14] update iptb dependency and use different ports for each iptb cluster update iptb dependency again, and pick different ports for each iptb cluster try and fix godeps crap --- Godeps/Godeps.json | 2 +- .../src/github.com/whyrusleeping/iptb/main.go | 106 ++++++++++++++---- test/sharness/t0101-iptb-name.sh | 2 +- test/sharness/t0130-multinode.sh | 2 +- 4 files changed, 90 insertions(+), 22 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index bed22cc601d..a6da0635472 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -256,7 +256,7 @@ }, { "ImportPath": "github.com/whyrusleeping/iptb", - "Rev": "4fa36405d0baea7773676f83fba9695e9a560473" + "Rev": "3970c95a864f1a40037f796ff596607ce8ae43be" }, { "ImportPath": "golang.org/x/crypto/blowfish", diff --git a/Godeps/_workspace/src/github.com/whyrusleeping/iptb/main.go b/Godeps/_workspace/src/github.com/whyrusleeping/iptb/main.go index d6fa9d9c1e0..c5ede8d47bc 100644 --- a/Godeps/_workspace/src/github.com/whyrusleeping/iptb/main.go +++ b/Godeps/_workspace/src/github.com/whyrusleeping/iptb/main.go @@ -1,12 +1,13 @@ package main import ( + "encoding/json" "errors" "flag" "fmt" "io/ioutil" "log" - "net" + "net/http" "os" "os/exec" "path" @@ -15,7 +16,9 @@ import ( "syscall" "time" + ma "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" serial "github.com/ipfs/go-ipfs/repo/fsrepo/serialize" + manet "github.com/jbenet/go-multiaddr-net" ) // GetNumNodes returns the number of testbed nodes configured in the testbed directory @@ -66,6 +69,15 @@ type initCfg struct { Count int Force bool Bootstrap string + PortStart int +} + +func (c *initCfg) swarmAddrForPeer(i int) string { + return fmt.Sprintf("/ip4/0.0.0.0/tcp/%d", c.PortStart+i) +} + +func (c *initCfg) apiAddrForPeer(i int) string { + return fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", c.PortStart+1000+i) } func IpfsInit(cfg *initCfg) error { @@ -121,7 +133,7 @@ func IpfsInit(cfg *initCfg) error { return nil } -func starBootstrap(cfg *initCfg) error { +func starBootstrap(icfg *initCfg) error { // '0' node is the bootstrap node cfgpath := path.Join(IpfsDirN(0), "config") bcfg, err := serial.Load(cfgpath) @@ -129,15 +141,15 @@ func starBootstrap(cfg *initCfg) error { return err } bcfg.Bootstrap = nil - bcfg.Addresses.Swarm = []string{"/ip4/127.0.0.1/tcp/4002"} - bcfg.Addresses.API = "/ip4/127.0.0.1/tcp/5002" + bcfg.Addresses.Swarm = []string{icfg.swarmAddrForPeer(0)} + bcfg.Addresses.API = icfg.apiAddrForPeer(0) bcfg.Addresses.Gateway = "" err = serial.WriteConfigFile(cfgpath, bcfg) if err != nil { return err } - for i := 1; i < cfg.Count; i++ { + for i := 1; i < icfg.Count; i++ { cfgpath := path.Join(IpfsDirN(i), "config") cfg, err := serial.Load(cfgpath) if err != nil { @@ -147,9 +159,9 @@ func starBootstrap(cfg *initCfg) error { cfg.Bootstrap = []string{fmt.Sprintf("%s/ipfs/%s", bcfg.Addresses.Swarm[0], bcfg.Identity.PeerID)} cfg.Addresses.Gateway = "" cfg.Addresses.Swarm = []string{ - fmt.Sprintf("/ip4/0.0.0.0/tcp/%d", 4002+i), + icfg.swarmAddrForPeer(i), } - cfg.Addresses.API = fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", 5002+i) + cfg.Addresses.API = icfg.apiAddrForPeer(i) err = serial.WriteConfigFile(cfgpath, cfg) if err != nil { return err @@ -158,8 +170,8 @@ func starBootstrap(cfg *initCfg) error { return nil } -func clearBootstrapping(cfg *initCfg) error { - for i := 0; i < cfg.Count; i++ { +func clearBootstrapping(icfg *initCfg) error { + for i := 0; i < icfg.Count; i++ { cfgpath := path.Join(IpfsDirN(i), "config") cfg, err := serial.Load(cfgpath) if err != nil { @@ -168,10 +180,8 @@ func clearBootstrapping(cfg *initCfg) error { cfg.Bootstrap = nil cfg.Addresses.Gateway = "" - cfg.Addresses.Swarm = []string{ - fmt.Sprintf("/ip4/0.0.0.0/tcp/%d", 4002+i), - } - cfg.Addresses.API = fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", 5002+i) + cfg.Addresses.Swarm = []string{icfg.swarmAddrForPeer(i)} + cfg.Addresses.API = icfg.apiAddrForPeer(i) err = serial.WriteConfigFile(cfgpath, cfg) if err != nil { return err @@ -222,6 +232,7 @@ func IpfsKill() error { } func IpfsStart(waitall bool) error { + var addrs []string n := GetNumNodes() for i := 0; i < n; i++ { dir := IpfsDirN(i) @@ -259,22 +270,55 @@ func IpfsStart(waitall bool) error { // Make sure node 0 is up before starting the rest so // bootstrapping works properly if i == 0 || waitall { - err := waitForLive(fmt.Sprintf("localhost:%d", 5002+i)) + cfg, err := serial.Load(path.Join(IpfsDirN(i), "config")) + if err != nil { + return err + } + + maddr := ma.StringCast(cfg.Addresses.API) + _, addr, err := manet.DialArgs(maddr) + if err != nil { + return err + } + + addrs = append(addrs, addr) + + err = waitOnAPI(cfg.Identity.PeerID, addr) + if err != nil { + return err + } + } + } + if waitall { + for i := 0; i < n; i++ { + err := waitOnSwarmPeers(addrs[i]) if err != nil { return err } } + } return nil } -// waitForLive polls the given endpoint until it is up, or until -// a timeout -func waitForLive(addr string) error { +func waitOnAPI(peerid, addr string) error { for i := 0; i < 50; i++ { - c, err := net.Dial("tcp", addr) + resp, err := http.Get("http://" + addr + "/api/v0/id") if err == nil { - c.Close() + out := make(map[string]interface{}) + err := json.NewDecoder(resp.Body).Decode(&out) + if err != nil { + return fmt.Errorf("liveness check failed: %s", err) + } + id, ok := out["ID"] + if !ok { + return fmt.Errorf("liveness check failed: ID field not present in output") + } + idstr := id.(string) + if idstr != peerid { + return fmt.Errorf("liveness check failed: unexpected peer at endpoint") + } + return nil } time.Sleep(time.Millisecond * 200) @@ -282,6 +326,29 @@ func waitForLive(addr string) error { return fmt.Errorf("node at %s failed to come online in given time period", addr) } +func waitOnSwarmPeers(addr string) error { + for i := 0; i < 50; i++ { + resp, err := http.Get("http://" + addr + "/api/v0/swarm/peers") + if err == nil { + out := make(map[string]interface{}) + err := json.NewDecoder(resp.Body).Decode(&out) + if err != nil { + return fmt.Errorf("liveness check failed: %s", err) + } + + peers := out["Strings"].([]interface{}) + if len(peers) == 0 { + time.Sleep(time.Millisecond * 200) + continue + } + + return nil + } + time.Sleep(time.Millisecond * 200) + } + return fmt.Errorf("node at %s failed to bootstrap in given time period", addr) +} + // GetPeerID reads the config of node 'n' and returns its peer ID func GetPeerID(n int) (string, error) { cfg, err := serial.Load(path.Join(IpfsDirN(n), "config")) @@ -371,6 +438,7 @@ func handleErr(s string, err error) { func main() { cfg := new(initCfg) flag.IntVar(&cfg.Count, "n", 0, "number of ipfs nodes to initialize") + flag.IntVar(&cfg.PortStart, "p", 4002, "port to start allocations from") flag.BoolVar(&cfg.Force, "f", false, "force initialization (overwrite existing configs)") flag.StringVar(&cfg.Bootstrap, "bootstrap", "star", "select bootstrapping style for cluster") diff --git a/test/sharness/t0101-iptb-name.sh b/test/sharness/t0101-iptb-name.sh index f7330cb4d45..b0c1bc06eb4 100755 --- a/test/sharness/t0101-iptb-name.sh +++ b/test/sharness/t0101-iptb-name.sh @@ -11,7 +11,7 @@ test_description="Test ipfs repo operations" export IPTB_ROOT="`pwd`/.iptb" test_expect_success "set up an iptb cluster" ' - iptb -n=4 init && + iptb -n=4 -p=9000 init && iptb -wait start ' diff --git a/test/sharness/t0130-multinode.sh b/test/sharness/t0130-multinode.sh index 8080d0f2746..b193392dba4 100755 --- a/test/sharness/t0130-multinode.sh +++ b/test/sharness/t0130-multinode.sh @@ -11,7 +11,7 @@ test_description="Test multiple ipfs nodes" export IPTB_ROOT="`pwd`/.iptb" test_expect_success "set up a few nodes" ' - iptb -n=3 init && + iptb -n=3 -p=9200 init && iptb -wait start '