-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(Dgraph): Online restores allows to restore a specific backup. #6411
Conversation
Disregard the changes to the docker-compose files and the makefiles. They are a workaround to an issue I'm having (my workstation has a different version of glibc than the docker image). I will revert these changes before I merge this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a stab. Lets do synchronous review so I can understand better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 13 files reviewed, 2 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)
systest/online-restore/online_restore_test.go, line 178 at r3 (raw file):
t.Logf(string(resp.GetJson())) t.Logf(string(bodies[1]))
revert this
worker/backup_handler.go, line 63 at r3 (raw file):
GetManfiest
typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments. Feel free to resolve.
Reviewable status: 0 of 13 files reviewed, 5 unresolved discussions (waiting on @manishrjain, @martinmr, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)
dgraph/Makefile, line 46 at r3 (raw file):
lastCommitTime = github.com/dgraph-io/dgraph/x.lastCommitTime BUILD_FLAGS ?= -ldflags '-X ${lastCommitSHA}=${BUILD} -X "${lastCommitTime}=${BUILD_DATE}" -X "${dgraphVersion}=${BUILD_VERSION}" -X "${dgraphCodename}=${BUILD_CODENAME}${MODIFIED}" -X ${gitBranch}=${BUILD_BRANCH}' -tags netgo
what is netgo tag for?
worker/backup_handler.go, line 66 at r3 (raw file):
// and backup number at the specified location. If backupNum is set to zero, // all the manifests for the backup series will be returned. GetManifests(*url.URL, string, uint64) ([]*Manifest, error)
Maybe clarify comment that given a backupNum, it returns manifests from 0 to thru the backupNum. Not clear as it is.
worker/backup_handler.go, line 318 at r3 (raw file):
if backupNum > 0 { if len(manifests) < int(backupNum) {
are we off by 1? e.g. if backupNum = 1, i expect 2 manifests at 0 and 1, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 11 files reviewed, 3 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @parasssh, @pawanrawal, and @vvbalaji-dgraph)
dgraph/Makefile, line 46 at r3 (raw file):
Previously, parasssh wrote…
what is netgo tag for?
Ignore that. It's just because I couldn't build Dgraph on my machine. I'll revert that.
But the tag is to build the net library without using the c standard library.
systest/online-restore/online_restore_test.go, line 178 at r3 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
t.Logf(string(resp.GetJson())) t.Logf(string(bodies[1]))
revert this
Done.
worker/backup_handler.go, line 63 at r3 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
GetManfiest
typo
Done.
worker/backup_handler.go, line 66 at r3 (raw file):
Previously, parasssh wrote…
Maybe clarify comment that given a backupNum, it returns manifests from 0 to thru the backupNum. Not clear as it is.
Done.
worker/backup_handler.go, line 318 at r3 (raw file):
Previously, parasssh wrote…
are we off by 1? e.g. if backupNum = 1, i expect 2 manifests at 0 and 1, no?
There's no backup with backupNum 0. We start the count at 1.
…6411) Currently online restores will restore the entire backup series. This PR allows to restore the backup series up to a specific backup. Fixes DGRAPH-1648
…6411) Currently online restores will restore the entire backup series. This PR allows to restore the backup series up to a specific backup. Fixes DGRAPH-1648
…6411) (#6742) Currently online restores will restore the entire backup series. This PR allows to restore the backup series up to a specific backup. Fixes DGRAPH-1648 Co-authored-by: Martin Martinez Rivera <[email protected]>
Currently online restores will restore the entire backup series. This PR allows to
restore the backup series up to a specific backup.
Fixes DGRAPH-1648
This change is
Docs Preview: