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 <[email protected]>
ipfs · Mar 22, 2018 · 86b68db · 86b68db
1 parent 09d9e7d
commit 86b68db
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 6 deletions.
diff --git 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
@@ -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