Skip to content

Commit 09b03c0

Browse files
author
Jacob Marble
committed
chore: integrate review feedback
1 parent 084e55b commit 09b03c0

File tree

12 files changed

+62
-55
lines changed

12 files changed

+62
-55
lines changed

‎authorizer/authorize.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,7 @@ import (
1111
// IsAllowed checks to see if an action is authorized by retrieving the authorizer
1212
// off of context and authorizing the action appropriately.
1313
func IsAllowed(ctx context.Context, p influxdb.Permission) error {
14-
a, err := influxdbcontext.GetAuthorizer(ctx)
15-
if err != nil {
16-
return err
17-
}
18-
19-
if !a.Allowed(p) {
20-
return &influxdb.Error{
21-
Code: influxdb.EUnauthorized,
22-
Msg: fmt.Sprintf("%s is unauthorized", p),
23-
}
24-
}
25-
26-
return nil
14+
return IsAllowedAll(ctx, []influxdb.Permission{p})
2715
}
2816

2917
// IsAllowedAll checks to see if an action is authorized by ALL permissions.

‎authorizer/backup.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ import (
44
"context"
55
"io"
66

7-
"github.com/influxdata/influxdb/kit/tracing"
8-
97
"github.com/influxdata/influxdb"
8+
"github.com/influxdata/influxdb/kit/tracing"
109
)
1110

1211
var _ influxdb.BackupService = (*BackupService)(nil)

‎backup.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@ import (
55
"io"
66
)
77

8+
// BackupService represents the data backup functions of InfluxDB.
89
type BackupService interface {
9-
CreateBackup(context.Context) (int, []string, error)
10+
// CreateBackup creates a local copy (hard links) of the TSM data for all orgs and buckets.
11+
// The return values are used to download each backup file.
12+
CreateBackup(context.Context) (backupID int, backupFiles []string, err error)
13+
// FetchBackupFile downloads one backup file, data or metadata.
1014
FetchBackupFile(ctx context.Context, backupID int, backupFile string, w io.Writer) error
15+
// InternalBackupPath is a utility to determine the on-disk location of a backup fileset.
1116
InternalBackupPath(backupID int) string
1217
}
1318

19+
// KVBackupService represents the meta data backup functions of InfluxDB.
1420
type KVBackupService interface {
21+
// Backup creates a live backup copy of the metadata database.
1522
Backup(ctx context.Context, w io.Writer) error
1623
}

‎bolt/bbolt.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@ import (
1414
"go.uber.org/zap"
1515
)
1616

17-
// OpPrefix is the prefix for bolt ops
18-
const OpPrefix = "bolt/"
17+
const (
18+
// OpPrefix is the prefix for bolt ops
19+
OpPrefix = "bolt/"
20+
21+
DefaultFilename = "influxd.bolt"
22+
)
1923

2024
func getOp(op string) string {
2125
return OpPrefix + op

‎bolt/kv.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import (
99
"time"
1010

1111
bolt "github.com/coreos/bbolt"
12-
"go.uber.org/zap"
13-
1412
"github.com/influxdata/influxdb/kit/tracing"
1513
"github.com/influxdata/influxdb/kv"
14+
"go.uber.org/zap"
1615
)
1716

1817
// KVStore is a kv.Store backed by boltdb.

‎cmd/influx/backup.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,26 @@ package main
33
import (
44
"context"
55
"fmt"
6+
"os"
7+
"path/filepath"
8+
69
"github.com/influxdata/influxdb"
10+
"github.com/influxdata/influxdb/bolt"
711
"github.com/influxdata/influxdb/http"
812
"github.com/spf13/cobra"
913
"github.com/spf13/viper"
1014
"go.uber.org/multierr"
11-
"os"
12-
"path/filepath"
1315
)
1416

1517
var backupCmd = &cobra.Command{
1618
Use: "backup",
1719
Short: "Backup the data in InfluxDB",
18-
Long: `Backs up data and meta data for the InfluxDB instance. OSS only.`,
19-
RunE: backupF,
20+
Long: fmt.Sprintf(
21+
`Backs up data and meta data for the running InfluxDB instance.
22+
Downloaded files are written to the directory indicated by --path.
23+
Data file have extension .tsm; meta data is written to %s in the same directory.`,
24+
bolt.DefaultFilename),
25+
RunE: backupF,
2026
}
2127

2228
var backupFlags struct {

‎cmd/influxd/launcher/launcher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func buildLauncherCommand(l *Launcher, cmd *cobra.Command) {
146146
{
147147
DestP: &l.boltPath,
148148
Flag: "bolt-path",
149-
Default: filepath.Join(dir, "influxd.bolt"),
149+
Default: filepath.Join(dir, bolt.DefaultFilename),
150150
Desc: "path to boltdb database",
151151
},
152152
{

‎cmd/influxd/launcher/launcher_helpers.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/influxdata/flux"
1818
"github.com/influxdata/flux/lang"
1919
platform "github.com/influxdata/influxdb"
20+
"github.com/influxdata/influxdb/bolt"
2021
influxdbcontext "github.com/influxdata/influxdb/context"
2122
"github.com/influxdata/influxdb/http"
2223
"github.com/influxdata/influxdb/kv"
@@ -74,7 +75,7 @@ func RunTestLauncherOrFail(tb testing.TB, ctx context.Context, args ...string) *
7475

7576
// Run executes the program with additional arguments to set paths and ports.
7677
func (tl *TestLauncher) Run(ctx context.Context, args ...string) error {
77-
args = append(args, "--bolt-path", filepath.Join(tl.Path, "influxd.bolt"))
78+
args = append(args, "--bolt-path", filepath.Join(tl.Path, bolt.DefaultFilename))
7879
args = append(args, "--engine-path", filepath.Join(tl.Path, "engine"))
7980
args = append(args, "--http-bind-address", "127.0.0.1:0")
8081
args = append(args, "--log-level", "debug")

‎http/backup_service.go

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import (
77
"io"
88
"net/http"
99
"os"
10-
"path"
10+
"path/filepath"
1111
"strconv"
12+
"time"
1213

1314
"github.com/influxdata/influxdb"
15+
"github.com/influxdata/influxdb/bolt"
1416
"github.com/influxdata/influxdb/kit/tracing"
1517
"github.com/julienschmidt/httprouter"
1618
"go.uber.org/multierr"
@@ -51,10 +53,12 @@ const (
5153
backupIDParamName = "backup_id"
5254
backupFileParamName = "backup_file"
5355
backupFilePath = backupPath + "/:" + backupIDParamName + "/file/:" + backupFileParamName
56+
57+
httpClientTimeout = time.Hour
5458
)
5559

5660
func composeBackupFilePath(backupID int, backupFile string) string {
57-
return path.Join(backupPath, fmt.Sprint(backupID), "file", fmt.Sprint(backupFile))
61+
return filepath.Join(backupPath, fmt.Sprint(backupID), "file", backupFile)
5862
}
5963

6064
// NewBackupHandler creates a new handler at /api/v2/backup to receive backup requests.
@@ -83,7 +87,6 @@ func (h *BackupHandler) handleCreate(w http.ResponseWriter, r *http.Request) {
8387
defer span.Finish()
8488

8589
ctx := r.Context()
86-
defer r.Body.Close()
8790

8891
id, files, err := h.BackupService.CreateBackup(ctx)
8992
if err != nil {
@@ -92,28 +95,26 @@ func (h *BackupHandler) handleCreate(w http.ResponseWriter, r *http.Request) {
9295
}
9396

9497
internalBackupPath := h.BackupService.InternalBackupPath(id)
95-
metaFilename := fmt.Sprintf("%s/influxd.bolt", internalBackupPath)
98+
metaFilename := filepath.Join(internalBackupPath, bolt.DefaultFilename)
9699
metaFile, err := os.OpenFile(metaFilename, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0660)
97100
if err != nil {
98101
err = multierr.Append(err, os.RemoveAll(internalBackupPath))
99102
h.HandleHTTPError(ctx, err, w)
100103
return
101104
}
102105

103-
err = h.KVBackupService.Backup(ctx, metaFile)
104-
if err != nil {
106+
if err = h.KVBackupService.Backup(ctx, metaFile); err != nil {
105107
err = multierr.Append(err, os.RemoveAll(internalBackupPath))
106108
h.HandleHTTPError(ctx, err, w)
107109
return
108110
}
109-
files = append(files, "influxd.bolt")
111+
files = append(files, bolt.DefaultFilename)
110112

111113
b := backup{
112114
ID: id,
113115
Files: files,
114116
}
115-
err = json.NewEncoder(w).Encode(&b)
116-
if err != nil {
117+
if err = json.NewEncoder(w).Encode(&b); err != nil {
117118
err = multierr.Append(err, os.RemoveAll(internalBackupPath))
118119
h.HandleHTTPError(ctx, err, w)
119120
return
@@ -125,7 +126,6 @@ func (h *BackupHandler) handleFetchFile(w http.ResponseWriter, r *http.Request)
125126
defer span.Finish()
126127

127128
ctx := r.Context()
128-
defer r.Body.Close()
129129

130130
params := httprouter.ParamsFromContext(ctx)
131131
backupID, err := strconv.Atoi(params.ByName("backup_id"))
@@ -135,13 +135,13 @@ func (h *BackupHandler) handleFetchFile(w http.ResponseWriter, r *http.Request)
135135
}
136136
backupFile := params.ByName("backup_file")
137137

138-
err = h.BackupService.FetchBackupFile(ctx, backupID, backupFile, w)
139-
if err != nil {
138+
if err = h.BackupService.FetchBackupFile(ctx, backupID, backupFile, w); err != nil {
140139
h.HandleHTTPError(ctx, err, w)
141140
return
142141
}
143142
}
144143

144+
// BackupService is the client implementation of influxdb.BackupService.
145145
type BackupService struct {
146146
Addr string
147147
Token string
@@ -165,6 +165,7 @@ func (s *BackupService) CreateBackup(ctx context.Context) (int, []string, error)
165165
req = req.WithContext(ctx)
166166

167167
hc := NewClient(u.Scheme, s.InsecureSkipVerify)
168+
hc.Timeout = httpClientTimeout
168169
resp, err := hc.Do(req)
169170
if err != nil {
170171
return 0, nil, err
@@ -176,8 +177,7 @@ func (s *BackupService) CreateBackup(ctx context.Context) (int, []string, error)
176177
}
177178

178179
var b backup
179-
err = json.NewDecoder(resp.Body).Decode(&b)
180-
if err != nil {
180+
if err = json.NewDecoder(resp.Body).Decode(&b); err != nil {
181181
return 0, nil, err
182182
}
183183

@@ -200,6 +200,7 @@ func (s *BackupService) FetchBackupFile(ctx context.Context, backupID int, backu
200200
SetToken(s.Token, req)
201201

202202
hc := NewClient(u.Scheme, s.InsecureSkipVerify)
203+
hc.Timeout = httpClientTimeout
203204
resp, err := hc.Do(req)
204205
if err != nil {
205206
return err

‎storage/engine.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"os"
1111
"path/filepath"
1212
"sync"
13-
"syscall"
1413
"time"
1514

1615
"github.com/influxdata/influxdb"
@@ -667,12 +666,15 @@ func (e *Engine) deleteBucketRangeLocked(ctx context.Context, orgID, bucketID pl
667666
return e.engine.DeletePrefixRange(ctx, name, min, max, pred)
668667
}
669668

669+
// CreateBackup creates a "snapshot" of all TSM data in the Engine.
670+
// 1) Snapshot the cache to ensure the backup includes all data written before now.
671+
// 2) Create hard links to all TSM files, in a new directory within the engine root directory.
672+
// 3) Return a unique backup ID (invalid after the process terminates) and list of files.
670673
func (e *Engine) CreateBackup(ctx context.Context) (int, []string, error) {
671674
span, ctx := tracing.StartSpanFromContext(ctx)
672675
span.LogKV("path", e.path)
673676

674-
err := e.engine.WriteSnapshot(ctx, tsm1.CacheStatusBackup)
675-
if err != nil {
677+
if err := e.engine.WriteSnapshot(ctx, tsm1.CacheStatusBackup); err != nil {
676678
return 0, nil, err
677679
}
678680

@@ -693,6 +695,8 @@ func (e *Engine) CreateBackup(ctx context.Context) (int, []string, error) {
693695
return id, filenames, nil
694696
}
695697

698+
// FetchBackupFile writes a given backup file to the provided writer.
699+
// After a successful write, the internal copy is removed.
696700
func (e *Engine) FetchBackupFile(ctx context.Context, backupID int, backupFile string, w io.Writer) error {
697701
span, ctx := tracing.StartSpanFromContext(ctx)
698702
span.LogKV("path", e.path)
@@ -703,10 +707,8 @@ func (e *Engine) FetchBackupFile(ctx context.Context, backupID int, backupFile s
703707
return errors.Errorf("backup %d not found", backupID)
704708
}
705709
return errors.WithMessagef(err, "failed to locate backup %d", backupID)
706-
} else {
707-
if !fi.IsDir() {
708-
return errors.Errorf("error in filesystem path of backup %d", backupID)
709-
}
710+
} else if !fi.IsDir() {
711+
return errors.Errorf("error in filesystem path of backup %d", backupID)
710712
}
711713

712714
backupFileFullPath := filepath.Join(backupPath, backupFile)
@@ -725,14 +727,15 @@ func (e *Engine) FetchBackupFile(ctx context.Context, backupID int, backupFile s
725727
return errors.WithMessagef(err, "failed to copy backup file %d/%s to writer", backupID, backupFile)
726728
}
727729

728-
err = syscall.Unlink(backupFileFullPath)
729-
if err != nil {
730-
e.logger.Info("failed to unlink backup file after fetch", zap.Error(err), zap.Int("backup_id", backupID), zap.String("backup_file", backupFile))
730+
if err = os.Remove(backupFileFullPath); err != nil {
731+
e.logger.Info("Failed to remove backup file after fetch", zap.Error(err), zap.Int("backup_id", backupID), zap.String("backup_file", backupFile))
731732
}
732733

733734
return nil
734735
}
735736

737+
// InternalBackupPath provides the internal, full path directory name of the backup.
738+
// This should not be exposed via API.
736739
func (e *Engine) InternalBackupPath(backupID int) string {
737740
return e.engine.FileStore.InternalBackupPath(backupID)
738741
}

0 commit comments

Comments
 (0)