chore: integrate review feedback

Author: Jacob Marble

cmd/influx/backup.go

@@ -20,6 +20,7 @@ var backupCmd = &cobra.Command{
 	Long: fmt.Sprintf(
 		`Backs up data and meta data for the running InfluxDB instance.
 Downloaded files are written to the directory indicated by --path.
+The target directory, and any parent directories, are created automatically.
 Data file have extension .tsm; meta data is written to %s in the same directory.`,
 		bolt.DefaultFilename),
 	RunE: backupF,
@@ -58,7 +59,7 @@ func backupF(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("must specify path")
 	}
 
-	err := os.Mkdir(backupFlags.Path, 0770)
+	err := os.MkdirAll(backupFlags.Path, 0770)
 	if err != nil && !os.IsExist(err) {
 		return err
 	}

http/backup_service.go

@@ -211,7 +211,7 @@ func (s *BackupService) FetchBackupFile(ctx context.Context, backupID int, backu
 		return err
 	}
 
-	_, err = io.CopyBuffer(w, resp.Body, make([]byte, 1024*1024))
+	_, err = io.Copy(w, resp.Body)
 	if err != nil {
 		return err
 	}

storage/engine.go

@@ -26,6 +26,7 @@ import (
 	"github.com/influxdata/influxql"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
+	"go.uber.org/multierr"
 	"go.uber.org/zap"
 )
 
@@ -672,7 +673,13 @@ func (e *Engine) deleteBucketRangeLocked(ctx context.Context, orgID, bucketID pl
 //   3) Return a unique backup ID (invalid after the process terminates) and list of files.
 func (e *Engine) CreateBackup(ctx context.Context) (int, []string, error) {
 	span, ctx := tracing.StartSpanFromContext(ctx)
-	span.LogKV("path", e.path)
+	defer span.Finish()
+
+	e.mu.RLock()
+	defer e.mu.RUnlock()
+	if e.closing == nil {
+		return 0, nil, ErrEngineClosed
+	}
 
 	if err := e.engine.WriteSnapshot(ctx, tsm1.CacheStatusBackup); err != nil {
 		return 0, nil, err
@@ -699,7 +706,13 @@ func (e *Engine) CreateBackup(ctx context.Context) (int, []string, error) {
 // After a successful write, the internal copy is removed.
 func (e *Engine) FetchBackupFile(ctx context.Context, backupID int, backupFile string, w io.Writer) error {
 	span, ctx := tracing.StartSpanFromContext(ctx)
-	span.LogKV("path", e.path)
+	defer span.Finish()
+
+	e.mu.RLock()
+	defer e.mu.RUnlock()
+	if e.closing == nil {
+		return ErrEngineClosed
+	}
 
 	backupPath := e.engine.FileStore.InternalBackupPath(backupID)
 	if fi, err := os.Stat(backupPath); err != nil {
@@ -721,12 +734,15 @@ func (e *Engine) FetchBackupFile(ctx context.Context, backupID int, backupFile s
 	}
 	defer file.Close()
 
-	buf := make([]byte, 1024*1024)
-	_, err = io.CopyBuffer(w, file, buf)
-	if err != nil {
+	if _, err = io.Copy(w, file); err != nil {
+		err = multierr.Append(err, file.Close())
 		return errors.WithMessagef(err, "failed to copy backup file %d/%s to writer", backupID, backupFile)
 	}
 
+	if err = file.Close(); err != nil {
+		return errors.WithMessagef(err, "failed to close backup file %d/%s", backupID, backupFile)
+	}
+
 	if err = os.Remove(backupFileFullPath); err != nil {
 		e.logger.Info("Failed to remove backup file after fetch", zap.Error(err), zap.Int("backup_id", backupID), zap.String("backup_file", backupFile))
 	}

tsdb/tsm1/file_store.go

@@ -1208,7 +1208,7 @@ func (f *FileStore) locations(key []byte, t int64, ascending bool) []*location {
 
 // CreateSnapshot creates hardlinks for all tsm and tombstone files
 // in the path provided.
-func (f *FileStore) CreateSnapshot(ctx context.Context) (int, string, error) {
+func (f *FileStore) CreateSnapshot(ctx context.Context) (backupID int, backupDirFullPath string, err error) {
 	span, _ := tracing.StartSpanFromContext(ctx)
 	defer span.Finish()
 
@@ -1228,31 +1228,31 @@ func (f *FileStore) CreateSnapshot(ctx context.Context) (int, string, error) {
 	// increment and keep track of the current temp dir for when we drop the lock.
 	// this ensures we are the only writer to the directory.
 	f.currentTempDirID += 1
-	backupID := f.currentTempDirID
+	backupID = f.currentTempDirID
 	f.mu.Unlock()
 
-	path := f.InternalBackupPath(backupID)
+	backupDirFullPath = f.InternalBackupPath(backupID)
 
 	// create the tmp directory and add the hard links. there is no longer any shared
 	// mutable state.
-	err := os.Mkdir(path, 0777)
+	err := os.Mkdir(backupDirFullPath, 0777)
 	if err != nil {
 		return 0, "", err
 	}
 	for _, tsmf := range files {
-		newpath := filepath.Join(path, filepath.Base(tsmf.Path()))
+		newpath := filepath.Join(backupDirFullPath, filepath.Base(tsmf.Path()))
 		if err := os.Link(tsmf.Path(), newpath); err != nil {
 			return 0, "", fmt.Errorf("error creating tsm hard link: %q", err)
 		}
 		for _, tf := range tsmf.TombstoneFiles() {
-			newpath := filepath.Join(path, filepath.Base(tf.Path))
+			newpath := filepath.Join(backupDirFullPath, filepath.Base(tf.Path))
 			if err := os.Link(tf.Path, newpath); err != nil {
 				return 0, "", fmt.Errorf("error creating tombstone hard link: %q", err)
 			}
 		}
 	}
 
-	return backupID, path, nil
+	return backupID, backupDirFullPath, nil
 }
 
 func (f *FileStore) InternalBackupPath(backupID int) string {