Skip to content
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

Export backup loader interface and rename Load method. #901

Merged
merged 4 commits into from
Jul 1, 2019

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Jun 26, 2019

The loader type has been exported as KVLoader. This will allow Dgraph to implement
custom restore logic (i.e change the format of the keys and values before writing the data).

Originally I deleted the Load method but it's still required by the "badger restore" command.
To indicate that this method just performs a very basic restore, I renamed the method to
DefaultLoad and indicated that more complex restore logic should be implemented using
the KVLoader instead.


This change is Reviewable

The logic in the load method will be moved to Dgraph so that it can
support converting keys and values from a different format. To
compensate, the loader interface has been exported as KVLoader.
@martinmr martinmr requested a review from a team June 26, 2019 23:48
Also add a not telling users to use the KVLoader if more complex logic
is needed to restore a backup.
@martinmr martinmr changed the title Export backup loader interface and remove Load method. Export backup loader interface and rename Load method. Jun 26, 2019
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the APIs in Badger. Rest is good. :lgtm:

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @manishrjain and @martinmr)


backup.go, line 199 at r1 (raw file):

// DB.DefaultLoad() should be called on a database that is not running any other
// concurrent transactions while it is running.
func (db *DB) DefaultLoad(r io.Reader, maxPendingWrites int) error {

Keep the name same.

Copy link
Contributor Author

@martinmr martinmr left a 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 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain)


backup.go, line 199 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Keep the name same.

Done.

@martinmr martinmr merged commit 50ccc86 into master Jul 1, 2019
@martinmr martinmr deleted the martinmr/expose-loader branch July 1, 2019 23:45
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
The logic in the load method will be copied to Dgraph so that it can
support converting keys and values from a different format. To
allow this, the loader interface has been exported as KVLoader.

Also add a note telling users to use the KVLoader if more complex logic
is needed to restore a backup.


(cherry picked from commit 50ccc86)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants