From 86b68db2a7ef639b85a250342d0e418b986955c7 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 22 Mar 2018 13:24:30 -0700 Subject: [PATCH] short-circuit publishing in MFS if there's nothing to publish If we flip-flop between two values, we'll trigger a useless publish. Also, fix the TestRepublisher test case: 1. Use actual, independent CIDs. 2. Don't assume that republish on close if nothing changes. License: MIT Signed-off-by: Steven Allen --- mfs/repub_test.go | 19 +++++++++++++------ mfs/system.go | 9 +++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/mfs/repub_test.go b/mfs/repub_test.go index 15400c9a3d9..a7413a47c00 100644 --- a/mfs/repub_test.go +++ b/mfs/repub_test.go @@ -2,10 +2,12 @@ package mfs import ( "context" + "fmt" "testing" "time" ci "gx/ipfs/QmVvkK7s5imCiq3JVbL3pGfnhcCnf3LrFJPF4GE2sAoGZf/go-testutil/ci" + mh "gx/ipfs/QmZyZDi491cCNTLfAhwcaDii2Kg4pwKRkhqQzURGDvY6ua/go-multihash" cid "gx/ipfs/QmcZfnkapfECQGcLZaf9B79NRg7cRa9EnZh4LSbkCzwNvY/go-cid" ) @@ -18,6 +20,14 @@ func TestRepublisher(t *testing.T) { pub := make(chan struct{}) + newCid := func(i int) *cid.Cid { + c, err := cid.NewPrefixV1(cid.Raw, mh.SHA2_256).Sum([]byte(fmt.Sprintf("%d", i))) + if err != nil { + t.Error(err) + } + return c + } + pf := func(ctx context.Context, c *cid.Cid) error { pub <- struct{}{} return nil @@ -29,7 +39,7 @@ func TestRepublisher(t *testing.T) { rp := NewRepublisher(ctx, pf, tshort, tlong) go rp.Run() - rp.Update(nil) + rp.Update(newCid(0)) // should hit short timeout select { @@ -41,8 +51,8 @@ func TestRepublisher(t *testing.T) { cctx, cancel := context.WithCancel(context.Background()) go func() { - for { - rp.Update(nil) + for i := 1; ; i++ { + rp.Update(newCid(i)) time.Sleep(time.Millisecond * 10) select { case <-cctx.Done(): @@ -71,7 +81,4 @@ func TestRepublisher(t *testing.T) { t.Fatal(err) } }() - - // final pub from closing - <-pub } diff --git a/mfs/system.go b/mfs/system.go index 95d77e3a206..4b5f8cd9064 100644 --- a/mfs/system.go +++ b/mfs/system.go @@ -325,8 +325,17 @@ func (np *Republisher) Run() { func (np *Republisher) publish(ctx context.Context) error { np.lk.Lock() topub := np.val + lastpub := np.lastpub np.lk.Unlock() + // Short circuit. This can happen if we flip-flop. + if topub == nil { + return fmt.Errorf("tried to publish nil") + } + if topub == lastpub || lastpub != nil && topub.Equals(lastpub) { + return nil + } + err := np.pubfunc(ctx, topub) if err != nil { return err