remove cachelife parameter and cleanup (thanks @ion1)

License: MIT
Signed-off-by: Jeromy <[email protected]>

core/commands/ipns.go

@@ -76,11 +76,11 @@ Resolve the value of another name:
 
 		if local {
 			offroute := offline.NewOfflineRouter(n.Repo.Datastore(), n.PrivateKey)
-			resolver = namesys.NewRoutingResolver(offroute, 0, 0)
+			resolver = namesys.NewRoutingResolver(offroute, 0)
 		}
 
 		if nocache {
-			resolver = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), 0, 0)
+			resolver = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), 0)
 		}
 
 		var name string

core/core.go

@@ -226,13 +226,13 @@ func (n *IpfsNode) startOnlineServicesWithHost(ctx context.Context, host p2phost
 	bitswapNetwork := bsnet.NewFromIpfsHost(n.PeerHost, n.Routing)
 	n.Exchange = bitswap.New(ctx, n.Identity, bitswapNetwork, n.Blockstore, alwaysSendToPeer)
 
-	cachelife, size, err := n.getCacheParams()
+	size, err := n.getCacheSize()
 	if err != nil {
 		return err
 	}
 
 	// setup name system
-	n.Namesys = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), cachelife, size)
+	n.Namesys = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), size)
 
 	// setup ipns republishing
 	err = n.setupIpnsRepublisher()
@@ -243,33 +243,21 @@ func (n *IpfsNode) startOnlineServicesWithHost(ctx context.Context, host p2phost
 	return nil
 }
 
-// getCacheParams returns cache life and cache size
-func (n *IpfsNode) getCacheParams() (time.Duration, int, error) {
+// getCacheSize returns cache life and cache size
+func (n *IpfsNode) getCacheSize() (int, error) {
 	cfg, err := n.Repo.Config()
 	if err != nil {
-		return 0, 0, err
-	}
-
-	ct := cfg.Ipns.ResolveCacheTime
-	var d time.Duration
-	if ct == "" {
-		d = namesys.DefaultResolverCacheLife
-	} else {
-		parsed, err := time.ParseDuration(ct)
-		if err != nil {
-			return 0, 0, fmt.Errorf("error parsing cache life from Ipns.ResolveCacheTime: %s", err)
-		}
-		d = parsed
+		return 0, err
 	}
 
 	cs := cfg.Ipns.ResolveCacheSize
 	if cs == 0 {
 		cs = 128
 	}
 	if cs < 0 {
-		return 0, 0, fmt.Errorf("cannot specify negative resolve cache size")
+		return 0, fmt.Errorf("cannot specify negative resolve cache size")
 	}
-	return d, cs, nil
+	return cs, nil
 }
 
 func (n *IpfsNode) setupIpnsRepublisher() error {
@@ -490,12 +478,12 @@ func (n *IpfsNode) SetupOfflineRouting() error {
 
 	n.Routing = offroute.NewOfflineRouter(n.Repo.Datastore(), n.PrivateKey)
 
-	cachelife, size, err := n.getCacheParams()
+	size, err := n.getCacheSize()
 	if err != nil {
 		return err
 	}
 
-	n.Namesys = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), cachelife, size)
+	n.Namesys = namesys.NewNameSystem(n.Routing, n.Repo.Datastore(), size)
 
 	return nil
 }

fuse/ipns/ipns_test.go

@@ -113,7 +113,7 @@ func setupIpnsTest(t *testing.T, node *core.IpfsNode) (*core.IpfsNode, *fstest.M
 		}
 
 		node.Routing = offroute.NewOfflineRouter(node.Repo.Datastore(), node.PrivateKey)
-		node.Namesys = namesys.NewNameSystem(node.Routing, node.Repo.Datastore())
+		node.Namesys = namesys.NewNameSystem(node.Routing, node.Repo.Datastore(), 0)
 
 		ipnsfs, err := nsfs.NewFilesystem(context.Background(), node.DAG, node.Namesys, node.Pinning, node.PrivateKey)
 		if err != nil {

namesys/namesys.go

@@ -26,20 +26,20 @@ type mpns struct {
 }
 
 // NewNameSystem will construct the IPFS naming system based on Routing
-func NewNameSystem(r routing.IpfsRouting, ds ds.Datastore, cachelife time.Duration, cachesize int) NameSystem {
+func NewNameSystem(r routing.IpfsRouting, ds ds.Datastore, cachesize int) NameSystem {
 	return &mpns{
 		resolvers: map[string]resolver{
 			"dns":      newDNSResolver(),
 			"proquint": new(ProquintResolver),
-			"dht":      NewRoutingResolver(r, cachelife, cachesize),
+			"dht":      NewRoutingResolver(r, cachesize),
 		},
 		publishers: map[string]Publisher{
 			"/ipns/": NewRoutingPublisher(r, ds),
 		},
 	}
 }
 
-const DefaultResolverCacheLife = time.Minute
+const DefaultResolverCacheTTL = time.Minute
 
 // Resolve implements Resolver.
 func (ns *mpns) Resolve(ctx context.Context, name string) (path.Path, error) {

namesys/republisher/repub_test.go

@@ -36,7 +36,7 @@ func TestRepublish(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		nd.Namesys = namesys.NewNameSystem(nd.Routing, nd.Repo.Datastore(), 0, 0)
+		nd.Namesys = namesys.NewNameSystem(nd.Routing, nd.Repo.Datastore(), 0)
 
 		nodes = append(nodes, nd)
 	}

namesys/resolve_test.go

@@ -19,7 +19,7 @@ func TestRoutingResolve(t *testing.T) {
 	d := mockrouting.NewServer().Client(testutil.RandIdentityOrFatal(t))
 	dstore := ds.NewMapDatastore()
 
-	resolver := NewRoutingResolver(d, 0, 0)
+	resolver := NewRoutingResolver(d, 0)
 	publisher := NewRoutingPublisher(d, dstore)
 
 	privk, pubk, err := testutil.RandTestKeyPair(512)
@@ -53,7 +53,7 @@ func TestPrexistingExpiredRecord(t *testing.T) {
 	dstore := ds.NewMapDatastore()
 	d := mockrouting.NewServer().ClientWithDatastore(context.Background(), testutil.RandIdentityOrFatal(t), dstore)
 
-	resolver := NewRoutingResolver(d, 0, 0)
+	resolver := NewRoutingResolver(d, 0)
 	publisher := NewRoutingPublisher(d, dstore)
 
 	privk, pubk, err := testutil.RandTestKeyPair(512)
@@ -90,7 +90,7 @@ func TestPrexistingRecord(t *testing.T) {
 	dstore := ds.NewMapDatastore()
 	d := mockrouting.NewServer().ClientWithDatastore(context.Background(), testutil.RandIdentityOrFatal(t), dstore)
 
-	resolver := NewRoutingResolver(d, 0, 0)
+	resolver := NewRoutingResolver(d, 0)
 	publisher := NewRoutingPublisher(d, dstore)
 
 	privk, pubk, err := testutil.RandTestKeyPair(512)

namesys/routing.go

@@ -23,8 +23,7 @@ var log = logging.Logger("namesys")
 type routingResolver struct {
 	routing routing.IpfsRouting
 
-	cache     *lru.Cache
-	cachelife time.Duration
+	cache *lru.Cache
 }
 
 func (r *routingResolver) cacheGet(name string) (path.Path, bool) {
@@ -40,13 +39,15 @@ func (r *routingResolver) cacheGet(name string) (path.Path, bool) {
 	entry, ok := ientry.(cacheEntry)
 	if !ok {
 		// should never happen, purely for sanity
-		return "", false
+		log.Panicf("unexpected type %T in cache for %q.", ientry, name)
 	}
 
 	if time.Now().Before(entry.eol) {
 		return entry.val, true
 	}
 
+	r.cache.Remove(name)
+
 	return "", false
 }
 
@@ -55,11 +56,11 @@ func (r *routingResolver) cacheSet(name string, val path.Path, rec *pb.IpnsEntry
 		return
 	}
 
-	ttl := r.cachelife
+	// if completely unspecified, just use one minute
+	ttl := DefaultResolverCacheTTL
 	if rec.Ttl != nil {
-		// if the record has a ttl set, and its less than ours, use it instead
 		recttl := time.Duration(rec.GetTtl())
-		if recttl < ttl {
+		if recttl >= 0 {
 			ttl = recttl
 		}
 	}
@@ -83,20 +84,21 @@ type cacheEntry struct {
 
 // NewRoutingResolver constructs a name resolver using the IPFS Routing system
 // to implement SFS-like naming on top.
-func NewRoutingResolver(route routing.IpfsRouting, cachelife time.Duration, cachesize int) *routingResolver {
+// cachesize is the limit of the number of entries in the lru cache. Setting it
+// to '0' will disable caching.
+func NewRoutingResolver(route routing.IpfsRouting, cachesize int) *routingResolver {
 	if route == nil {
 		panic("attempt to create resolver with nil routing system")
 	}
 
-	if cachesize < 0 {
-		cachesize = 0
+	var cache *lru.Cache
+	if cachesize > 0 {
+		cache, _ = lru.New(cachesize)
 	}
-	cache, _ := lru.New(cachesize)
 
 	return &routingResolver{
-		routing:   route,
-		cache:     cache,
-		cachelife: cachelife,
+		routing: route,
+		cache:   cache,
 	}
 }
 

repo/config/init.go

@@ -65,7 +65,7 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) {
 		},
 
 		Ipns: Ipns{
-			ResolveCacheTime: "1m",
+			ResolveCacheSize: 128,
 		},
 
 		// tracking ipfs version used to generate the init folder and adding

repo/config/ipns.go

@@ -4,6 +4,5 @@ type Ipns struct {
 	RepublishPeriod string
 	RecordLifetime  string
 
-	ResolveCacheTime string
 	ResolveCacheSize int
 }

test/sharness/t0240-republisher.sh

@@ -26,7 +26,7 @@ setup_iptb() {
 		for i in $(test_seq 0 3)
 		do
 			ipfsi $i config Ipns.RepublishPeriod 20s
-			ipfsi $i config Ipns.ResolveCacheTime 0s
+			ipfsi $i config --json Ipns.ResolveCacheSize 0
 		done
 	'