cipd: instance cache

client/cipd/client.go

Index: client/cipd/client.go
diff --git a/client/cipd/client.go b/client/cipd/client.go
index 0ac2d4b02cd0f161c15951a20a98e5cc54e1fd3e..838a1e0d1e557c72e1fe81e92498797c287a223c 100644
--- a/client/cipd/client.go
+++ b/client/cipd/client.go
@@ -272,7 +272,7 @@ type Client interface {
 	FetchInstanceRefs(pin common.Pin, refs []string) ([]RefInfo, error)
 
 	// FetchInstance downloads package instance file from the repository.
-	FetchInstance(pin common.Pin, output io.WriteSeeker) error
+	FetchInstance(pin common.Pin, output io.ReadWriteSeeker) error
 
 	// FetchAndDeployInstance fetches the package instance and deploys it into
 	// the site root. It doesn't check whether the instance is already deployed.
@@ -336,9 +336,9 @@ type ClientOptions struct {
 	// UserAgent is put into User-Agent HTTP header with each request.
 	UserAgent string
 
-	// CacheDir is a directory for shared cache. If empty, tags are cached
-	// inside the site root. If both Root and CacheDir are empty, tag cache
-	// is disabled.
+	// CacheDir is a directory for shared cache. If empty, instances are not
+	// cached and tags are cached inside the site root. If both Root and
+	// CacheDir are empty, tag cache is disabled.
 	CacheDir string
 }
 
@@ -393,6 +393,10 @@ type clientImpl struct {
 	tagCache     *internal.TagCache
 	tagCacheInit sync.Once
 
+	// instanceCache is a file-system based cache of instances.
+	instanceCache     *internal.InstanceCache
+	instanceCacheInit sync.Once
+
 	// authClient is a lazily created http.Client to use for authenticated
 	// requests. Thread safe, but lazily initialized under lock.
 	authClient *http.Client
@@ -486,6 +490,38 @@ func (client *clientImpl) closeTagCache() {
 	client.tagCache = nil
 }
 
+// instanceCachePath returns path to the instance cache directory or "" if
+// instance cache is disabled.
+func (client *clientImpl) instanceCachePath() string {
+	if client.CacheDir == "" {
+		return ""
+	}
+	return filepath.Join(client.CacheDir, "instances")
+}
+
+// getInstanceCache lazy-initializes instanceCache and returns it.
+func (client *clientImpl) getInstanceCache() *internal.InstanceCache {
+	client.instanceCacheInit.Do(func() {
+		if path := client.instanceCachePath(); path != "" {
+			cachePath := local.NewFileSystem(path, client.Logger)
+			client.instanceCache = internal.LoadInstanceCache(cachePath, client.Logger, client.clock.now())
+		}
+	})
+	return client.instanceCache
+}
+
+// closeInstanceCache dumps any changes made to instance cache to disk, if necessary.
+func (client *clientImpl) closeInstanceCache() {
+	if client.instanceCache == nil || !client.instanceCache.Dirty() {
+		return
+	}
+
+	if err := client.instanceCache.Save(); err != nil {
+		client.Logger.Warningf("cipd: failed to save instance cache - %s", err)
+	}
+	client.instanceCache = nil
+}
+
 func (client *clientImpl) FetchACL(packagePath string) ([]PackageACL, error) {
 	return client.remote.fetchACL(packagePath)
 }
@@ -748,22 +784,58 @@ func (client *clientImpl) FetchInstanceRefs(pin common.Pin, refs []string) ([]Re
 	return client.remote.fetchRefs(pin, refs)
 }
 
-func (client *clientImpl) FetchInstance(pin common.Pin, output io.WriteSeeker) error {
+func (client *clientImpl) FetchInstance(pin common.Pin, output io.ReadWriteSeeker) error {
 	err := common.ValidatePin(pin)
 	if err != nil {
 		return err
 	}
+
+	now := client.clock.now()
+
+	cache := client.getInstanceCache()
+	if cache != nil {
+		switch err := cache.Get(pin, output, now); {
+		case os.IsNotExist(err):
+			// output is not corrupted.
+
+		case err != nil:
+			client.Logger.Warningf("cipd: could not get %s from cache - %s", pin, err)
+			// Output may be corrupted. Rewinding back.
+			if _, err := output.Seek(0, 0); err != nil {

[Vadim Sh., 2016/04/11 16:52:02]

well, it may not be enough. We also need to truncate the file to zero length.
Otherwise we may end up with "<valid data>|<garbage left from reading cached
file>".

[nodir, 2016/04/11 22:16:16]

Done.

+				return err
+			}
+
+		default:
+			client.Logger.Infof("cipd: cache hit for %s", pin)
+			return nil
+		}
+	}
+
 	client.Logger.Infof("cipd: resolving fetch URL for %s", pin)
 	fetchInfo, err := client.remote.fetchInstance(pin)
-	if err == nil {
-		err = client.storage.download(fetchInfo.fetchURL, output)
-	}
 	if err != nil {
-		client.Logger.Errorf("cipd: failed to fetch %s - %s", pin, err)
-		return err
+		goto fetchFail
 	}
+
+	if err = client.storage.download(fetchInfo.fetchURL, output); err != nil {
+		goto fetchFail
+	}
+
 	client.Logger.Infof("cipd: successfully fetched %s", pin)
+
+	if cache != nil {
+		if _, err := output.Seek(0, 0); err != nil {

[Vadim Sh., 2016/04/11 16:52:02]

It may be simpler to always download into cache, and then do io.Copy from the
cache file into output. That way you don't need to change FetchInstance
signature (and no need to truncate files).

[nodir, 2016/04/11 22:16:16]

Done.

+			client.Logger.Warningf("cipd: could not seek output to the beginning for caching - %s", err)
+		} else if err := cache.Put(pin, output, now); err != nil {
+			client.Logger.Warningf("cipd: could not cache %s - %s", pin, err)
+		}
+	}
+
 	return nil
+
+fetchFail:
+	client.Logger.Errorf("cipd: failed to fetch %s - %s", pin, err)
+	return err
 }
 
 func (client *clientImpl) FetchAndDeployInstance(pin common.Pin) error {
@@ -948,6 +1020,7 @@ func (client *clientImpl) Close() {
 	client.lock.Lock()
 	defer client.lock.Unlock()
 	client.closeTagCache()
+	client.closeInstanceCache()
 	client.authClient = nil
 	client.anonClient = nil
 }