Do not recompress chunks if we already have the compressed form (#289)

Author: matshch

chunk.go

@@ -86,3 +86,17 @@ func (c *Chunk) ID() ChunkID {
 	c.idCalculated = true
 	return c.id
 }
+
+// Storage returns the chunk data in compressed form. If the chunk was created
+// with compressed data and same modifiers, this data will be returned as is. The
+// caller must not modify the data in the returned slice.
+func (c *Chunk) Storage(modifiers Converters) ([]byte, error) {
+	if len(c.storage) > 0 && modifiers.equal(c.converters) {
+		return c.storage, nil
+	}
+	b, err := c.Data()
+	if err != nil {
+		return nil, err
+	}
+	return modifiers.toStorage(b)
+}

gcs.go

@@ -143,12 +143,7 @@ func (s GCStore) StoreChunk(chunk *Chunk) error {
 		})
 	)
 
-	b, err := chunk.Data()
-	if err != nil {
-		log.WithError(err).Error("Cannot retrieve chunk data")
-		return err
-	}
-	b, err = s.converters.toStorage(b)
+	b, err := chunk.Storage(s.converters)
 	if err != nil {
 		log.WithError(err).Error("Cannot retrieve chunk data")
 		return err

httphandler.go

@@ -57,18 +57,7 @@ func (h HTTPHandler) get(id ChunkID, w http.ResponseWriter) {
 	var b []byte
 	chunk, err := h.s.GetChunk(id)
 	if err == nil {
-		// Optimization for when the chunk modifiers match those
-		// of the chunk server. In that case it's not necessary
-		// to convert back and forth. Just use the raw data as loaded
-		// from the store.
-		if len(chunk.storage) > 0 && h.converters.equal(chunk.converters) {
-			b = chunk.storage
-		} else {
-			b, err = chunk.Data()
-			if err == nil {
-				b, err = h.converters.toStorage(b)
-			}
-		}
+		b, err = chunk.Storage(h.converters)
 	}
 	h.HTTPHandlerBase.get(id.String(), b, err, w)
 }

local.go

@@ -68,11 +68,7 @@ func (s LocalStore) RemoveChunk(id ChunkID) error {
 // StoreChunk adds a new chunk to the store
 func (s LocalStore) StoreChunk(chunk *Chunk) error {
 	d, p := s.nameFromID(chunk.ID())
-	b, err := chunk.Data()
-	if err != nil {
-		return err
-	}
-	b, err = s.converters.toStorage(b)
+	b, err := chunk.Storage(s.converters)
 	if err != nil {
 		return err
 	}

protocolserver.go

@@ -61,11 +61,7 @@ func (s *ProtocolServer) Serve(ctx context.Context) error {
 				}
 				return errors.Wrap(err, "unable to read chunk from store")
 			}
-			b, err := chunk.Data()
-			if err != nil {
-				return err
-			}
-			b, err = Compressor{}.toStorage(b)
+			b, err := chunk.Storage([]converter{Compressor{}})
 			if err != nil {
 				return err
 			}

remotehttp.go

@@ -247,11 +247,7 @@ func (r *RemoteHTTP) HasChunk(id ChunkID) (bool, error) {
 // StoreChunk adds a new chunk to the store
 func (r *RemoteHTTP) StoreChunk(chunk *Chunk) error {
 	p := r.nameFromID(chunk.ID())
-	b, err := chunk.Data()
-	if err != nil {
-		return err
-	}
-	b, err = r.converters.toStorage(b)
+	b, err := chunk.Storage(r.converters)
 	if err != nil {
 		return err
 	}

s3.go

@@ -125,11 +125,7 @@ retry:
 func (s S3Store) StoreChunk(chunk *Chunk) error {
 	contentType := "application/zstd"
 	name := s.nameFromID(chunk.ID())
-	b, err := chunk.Data()
-	if err != nil {
-		return err
-	}
-	b, err = s.converters.toStorage(b)
+	b, err := chunk.Storage(s.converters)
 	if err != nil {
 		return err
 	}

sftp.go

@@ -191,11 +191,7 @@ func (s *SFTPStore) StoreChunk(chunk *Chunk) error {
 	c := <-s.pool
 	defer func() { s.pool <- c }()
 	name := c.nameFromID(chunk.ID())
-	b, err := chunk.Data()
-	if err != nil {
-		return err
-	}
-	b, err = s.converters.toStorage(b)
+	b, err := chunk.Storage(s.converters)
 	if err != nil {
 		return err
 	}