Add series and backup number information to manifest. #3559
Conversation
There was a problem hiding this comment.
Reviewed 8 of 9 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @golangcibot and @martinmr)
ee/backup/backup.go, line 52 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
struct field
SeriesUidshould beSeriesUID(fromgolint)
Call it BackupId. Then the one below can be called BackupNum. Don't call it Uid.
ee/backup/backup.go, line 57 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
structtag: struct field tag
json"backup_num"not compatible with reflect.StructTag.Get: bad syntax for struct tag pair (fromgovet)
huh? Can you look into this?
ee/backup/handler.go, line 186 at r2 (raw file):
var filteredManifests []*Manifest for i := len(manifests) - 1; i >= 0; i-- { if len(seriesUid) > 0 && manifests[i].SeriesUid != seriesUid {
You most likely don't need the len(seriesUid) check. The not equals should be sufficient.
x/names-generator.go, line 1 at r2 (raw file):
// Taken from
Finish this comment.
Also, if you can copy over the license from Docker, please do and paste it here. Then add a comment that Copied from docker ..., and modified by Dgraph.
| Path string `json:"-"` | ||
| // BackupId is a unique ID assigned to all the backups in the same series | ||
| // (from the first full backup to the last incremental backup). | ||
| BackupId string `json:"backup_id"` |
There was a problem hiding this comment.
struct field BackupId should be BackupID (from golint)
| @@ -111,7 +111,7 @@ func (h *fileHandler) CreateManifest(uri *url.URL, req *pb.BackupRequest) error | |||
|
|
|||
| // Load uses tries to load any backup files found. | |||
| // Returns the maximum value of Since on success, error otherwise. | |||
| func (h *fileHandler) Load(uri *url.URL, lastDir string, fn loadFn) (uint64, error) { | |||
| func (h *fileHandler) Load(uri *url.URL, backupId string, fn loadFn) (uint64, error) { | |||
There was a problem hiding this comment.
method parameter backupId should be backupID (from golint)
| @@ -181,14 +179,16 @@ func ListManifests(l string) (map[string]*Manifest, error) { | |||
|
|
|||
| // filterManifests takes a list of manifests and returns the list of manifests | |||
| // that should be considered during a restore. | |||
| func filterManifests(manifests []*Manifest, lastDir string) ([]*Manifest, error) { | |||
| func filterManifests(manifests []*Manifest, backupId string) ([]*Manifest, error) { | |||
There was a problem hiding this comment.
func parameter backupId should be backupID (from golint)
| manifests[0].BackupNum) | ||
| } | ||
|
|
||
| seriesUid := manifests[0].BackupId |
There was a problem hiding this comment.
var seriesUid should be seriesUID (from golint)
| @@ -38,7 +38,7 @@ var Restore x.SubCommand | |||
| var LsBackup x.SubCommand | |||
|
|
|||
| var opt struct { | |||
| lastDir, location, pdir, zero string | |||
| backupId, location, pdir, zero string | |||
There was a problem hiding this comment.
struct field backupId should be backupID (from golint)
| @@ -216,7 +214,7 @@ func runRestoreCmd() error { | |||
| } | |||
|
|
|||
| // RunRestore calls badger.Load and tries to load data into a new DB. | |||
| func RunRestore(pdir, location, lastDir string) (uint64, error) { | |||
| func RunRestore(pdir, location, backupId string) (uint64, error) { | |||
There was a problem hiding this comment.
func parameter backupId should be backupID (from golint)
| @@ -239,7 +239,7 @@ func (h *s3Handler) readManifest(mc *minio.Client, object string, m *Manifest) e | |||
| // Load creates a new session, scans for backup objects in a bucket, then tries to | |||
| // load any backup objects found. | |||
| // Returns nil and the maximum Since value on success, error otherwise. | |||
| func (h *s3Handler) Load(uri *url.URL, lastDir string, fn loadFn) (uint64, error) { | |||
| func (h *s3Handler) Load(uri *url.URL, backupId string, fn loadFn) (uint64, error) { | |||
There was a problem hiding this comment.
method parameter backupId should be backupID (from golint)
There was a problem hiding this comment.
Reviewable status: 2 of 9 files reviewed, 19 unresolved discussions (waiting on @golangcibot, @manishrjain, and @martinmr)
ee/backup/backup.go, line 52 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Call it BackupId. Then the one below can be called BackupNum. Don't call it Uid.
Done.
ee/backup/backup.go, line 57 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
huh? Can you look into this?
Already fixed. The warning was triggered because I missed the colon in my first commit.
ee/backup/handler.go, line 186 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
You most likely don't need the len(seriesUid) check. The not equals should be sufficient.
If the backupId is empty, it means that option was not passed in the command line. So by default we want to restore the latest series of backups. If I remove this all the backups will be skipped, which is not what we want. I have added a comment explaining this.
x/names-generator.go, line 1 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Finish this comment.
Also, if you can copy over the license from Docker, please do and paste it here. Then add a comment that Copied from docker ..., and modified by Dgraph.
Done.
and to verify there are no gaps in the manifests to prevent restoring corrupted data.
This change is