Skip to content

Datastore copy util#469

Merged
sanderpick merged 6 commits intomasterfrom
sander/datastore-migration
Dec 9, 2020
Merged

Datastore copy util#469
sanderpick merged 6 commits intomasterfrom
sander/datastore-migration

Conversation

@sanderpick
Copy link
Copy Markdown
Contributor

Adds a small program for copying a badger/mongo datastore to another badger/mongo datastore.

@sanderpick sanderpick self-assigned this Dec 8, 2020
Signed-off-by: Sander Pick <sanderpick@gmail.com>
@sanderpick sanderpick force-pushed the sander/datastore-migration branch from 75147de to bdd8cab Compare December 8, 2020 08:24
Copy link
Copy Markdown
Contributor

@jsign jsign left a comment

Choose a reason for hiding this comment

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

LGTM! I left two somewhat important comments to consider.

Comment on lines +117 to +118
for r := range results.Next() {
if err := to.Put(ds.NewKey(r.Key), r.Value); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Between these two lines we need to add:

if r.Error != nil {
   log.Fatal(...)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, thanks!

for r := range results.Next() {
if err := to.Put(ds.NewKey(r.Key), r.Value); err != nil {
log.Fatalf("copying %s: %v", r.Key, err)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I discovered while working and testing Powergate migrations that this is too slow for copying (at least for my taste).

Here is what I did for migrations; which reduced times by some orders of magnitude. Nothing crazy, ignoring the migration logic, basically some rate-limiting to Read-and-Put up to 1000 in concurrently instead of serially.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice, copied your approach there

Signed-off-by: Sander Pick <sanderpick@gmail.com>
Signed-off-by: Sander Pick <sanderpick@gmail.com>
Signed-off-by: Sander Pick <sanderpick@gmail.com>
Signed-off-by: Sander Pick <sanderpick@gmail.com>
@sanderpick
Copy link
Copy Markdown
Contributor Author

Sorry the big code update post review here, but I realized using this tool would be much easier if it wasn't a generic datastore copier, but was specific to the structure of go-threads's datastores.

},
{
name: "ipfslite",
files: []string{"key"},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Move the key as well...

Copy link
Copy Markdown
Contributor

@jsign jsign left a comment

Choose a reason for hiding this comment

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

LGTM! Left a comment.

lock.Lock()
count++
lock.Unlock()
if count%parallel == 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: move lock.Unlock() to include the if case since count is being read without lock and might be a data-race with other goroutine owning the lock. (tongue twister)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice, done

Signed-off-by: Sander Pick <sanderpick@gmail.com>
@sanderpick sanderpick merged commit 4b2596f into master Dec 9, 2020
@sanderpick sanderpick deleted the sander/datastore-migration branch December 9, 2020 21:29
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.

2 participants