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

Feature/backup #10

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

benoitc
Copy link
Contributor

@benoitc benoitc commented Jan 15, 2016

This PR add the erocksdb:backup/2 and erocksdb:restore/2 function to backup the database incrementally and restore it:

https://github.com/facebook/rocksdb/wiki/How-to-backup-RocksDB%3F

Any feedback is welcome.

This change add the possibility to backup and restore a database incrementally:
https://github.com/facebook/rocksdb/wiki/How-to-backup-RocksDB%3F
@yosukehara
Copy link
Member

Thank you for your request. We'll check it.

@benoitc
Copy link
Contributor Author

benoitc commented Jan 18, 2016

since I have implemented benoitc@49978f7

which is using the new checkpoint feature. It's quite more efficient and used by the mysql version of facebook that uses rocksdb. Let me know.

@mocchira
Copy link
Member

@benoitc Thank you for your great PR.
Since we've also wanted this features, this PR is very welcome.

Anyway,
I wrote the review result below so please check it out.

  1. Make backup/restore async as the snapshot feature you are PRing now.
    As you know,
    Since backup/restore may take a long time, an Erlang scheduler thread executing those NIFs may block for an undesirable time.
  2. Keep BackupEngine alive and not to recreate it every time.
    The below is picked up from https://github.com/facebook/rocksdb/wiki/How-to-backup-RocksDB%3F
    Also we recommend to keep BackupEngine alive and not to recreate it every time you need to do a backup.
    So I'd recommend you to make its lifetime the same with DbObject.
  3. Ensure your code running properly on Linux, FreeBSD and SmartOS (if possible)
    Since we've supported those 3 platforms and already have users,
    I'd like you to test on those if possible or tell us what platforms you've tested.
    We will test other platforms instead of you.

@benoitc
Copy link
Contributor Author

benoitc commented Jan 19, 2016

@mocchira the code have been tested on OX and linux. not yet on freebsd.

About the implementation obviously it would be better to implement the backup as an asynchronous task. I will check it. However benoitc@49978f7 is probably a lot more efficient since it allows you backup only and only copy needed file, hard-linking others which is very fast. Maybe the patch above could be skipped in favour of such solution?

@mocchira
Copy link
Member

@benoitc

However benoitc/erocksdb@49978f7 is probably a lot more efficient since it allows you backup only and only
copy needed file, hard-linking others which is very fast. Maybe the patch above could be skipped in
favour of such solution?

I've checked how the both of features works/implements.

Long story short,

  • Backup
    • Copy full databases with a consistent state for the first time
    • Incremental backup for the next
  • Checkpoint
    • Just Keep full databases with a consistent state
    • Can be faster by using hard link if the destination path is on a same file system with the source one (or just leaving SST files this checkpoint is referring).

Since the checkpoint handles a SST file as a hard link or just leaving it (NOT copying) and needless to say, backup should be copied to other physical device normally,
Checkpoint alone seems NOT to be an complete backup solution but an building block to make an original backup solution (mysql adopts Checkpoint for this reason).

On the other hand,
Backup is regarded as a builtin complete backup solution.

So in my conclusion,
We'd like to have both APIs if possible.

So if you only need the checkpoint,
It'd be OK to leave PR for the checkpoint and close this (We will take over your PR.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants