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

gx: Update badgerds to 1.0 #4327

Merged
merged 1 commit into from
Oct 28, 2017
Merged

gx: Update badgerds to 1.0 #4327

merged 1 commit into from
Oct 28, 2017

Conversation

magik6k
Copy link
Member

@magik6k magik6k commented Oct 19, 2017

This PR updates badgerds to use new transaction API. On-disk format hasn't changed (at least this is what I observed)

Fixes #4322

@ghost ghost assigned magik6k Oct 19, 2017
@ghost ghost added the status/in-progress In progress label Oct 19, 2017
@whyrusleeping
Copy link
Member

Should we wait for the badger guys to resolve the 'minor issues' they mentioned before merging this? I guess its pretty easy to update again anyways

@magik6k
Copy link
Member Author

magik6k commented Oct 19, 2017

Yep, we definitely want to wait..

Just finished converting one of my nodes, ds-convert used current ds-badger (0.4.1), then tried to start ipfs daemon with 1.0, got this:

2017/10/20 01:10:59 "/" "!badger!head\x00\x00\x00\x00\x00\x00\x00\x00"
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/y.AssertTruef
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/y/error.go:62
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/y.CompareKeys
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/y/y.go:104
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table.(*blockIterator).Seek
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table/iterator.go:86
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table.(*Iterator).seekHelper
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table/iterator.go:269
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table.(*Iterator).seekFrom
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table/iterator.go:300
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table.(*Iterator).seek
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table/iterator.go:316
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table.(*Iterator).Seek
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/table/iterator.go:415
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger.(*levelHandler).get
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/level_handler.go:252
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger.(*levelsController).get
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/levels.go:668
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger.(*DB).get
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/db.go:456
gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger.Open
        /home/magik6k/.opt/go/src/gx/ipfs/QmWjtyUi7XPHMnUVBJLz9vWqKqZ7zCzni3jBwxMwb3zBVq/badger/db.go:264
gx/ipfs/QmfSNC13ywm9XybFdatZ7Kg1s6QmKVW4KxQ2skwWUxomdR/go-ds-badger.NewDatastore
        /home/magik6k/.opt/go/src/gx/ipfs/QmfSNC13ywm9XybFdatZ7Kg1s6QmKVW4KxQ2skwWUxomdR/go-ds-badger/datastore.go:35
github.com/ipfs/go-ipfs/repo/fsrepo.(*badgerdsDatastoreConfig).Create
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/datastores.go:393
github.com/ipfs/go-ipfs/repo/fsrepo.measureDatastoreConfig.Create
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/datastores.go:335
github.com/ipfs/go-ipfs/repo/fsrepo.(*measureDatastoreConfig).Create
        <autogenerated>:1
github.com/ipfs/go-ipfs/repo/fsrepo.(*FSRepo).openDatastore
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/fsrepo.go:407
github.com/ipfs/go-ipfs/repo/fsrepo.open
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/fsrepo.go:166
github.com/ipfs/go-ipfs/repo/fsrepo.Open.func1
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/fsrepo.go:110
github.com/ipfs/go-ipfs/repo.(*OnlyOne).Open
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/onlyone.go:35
github.com/ipfs/go-ipfs/repo/fsrepo.Open
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/repo/fsrepo/fsrepo.go:112
main.daemonFunc
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/cmd/ipfs/daemon.go:237
github.com/ipfs/go-ipfs/commands.(*Command).Call
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/commands/command.go:116
main.callCommand
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:355
main.(*cmdInvocation).Run
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:192
main.mainRet
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:157
main.main
        /home/magik6k/.opt/go/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:64
runtime.main
        /usr/lib/go/src/runtime/proc.go:185
runtime.goexit
        /usr/lib/go/src/runtime/asm_amd64.s:2337

I need to dig a bit deeper here as it worked for other nodes.

@magik6k
Copy link
Member Author

magik6k commented Oct 20, 2017

So it didn't work on that nodes either, it only seemed to do so, did some more testing:

badger   command
v0.8     ipfs init
v0.8     ipfs daemon
v0.8     ipfs pin (ipfs 0.4.11 hash)
v0.8     ipfs pin ls -> 846 nodes
v0.8     (kill daemon)
current  ipfs daemon --offline
current  ipfs pin ls -> 0 nodes
current  ipfs ls (ipfs 0.4.11 hash) -> not found
current  (kill daemon)
v0.8     ipfs daemon
v0.8     ipfs pin ls -> 0 nodes

Datastore opens without errors, but the data is lost. Using current version on it's own works just fine. I'm assuming the 'minor issues' include this one.

@whyrusleeping
Copy link
Member

Hrm... thats quite odd...

cc @manishrjain

@manishrjain
Copy link

The key expectations have changed since v0.8, so previous repos won't work with v1.0. I think we might have forgotten to update the magic number, but we can't update it anymore now.

Can you try building data with master and then reading it back?

@magik6k
Copy link
Member Author

magik6k commented Oct 20, 2017

Using badger master works as expected, the only problem is when switching version from 0.8 to current master.

We can increment the magic number in our fork to at least mitigate unexpected data loss.

@manishrjain Is there some migration tool we can use to upgrade badger disk format?

@Kubuxu
Copy link
Member

Kubuxu commented Oct 21, 2017

@magik6k IMO we should not fork badger, this will lead to more confusion.

@whyrusleeping
Copy link
Member

Yeah, forking isnt a great option... @manishrjain what do you suggest here? is there any way we can tell an old disk format from a new one?

@manishrjain
Copy link

We're working on providing a backup and restore tool. But, that'd be aimed at v1.0 and onwards.

I don't have full context here about how you control the data storage. If you do, I'd suggest doing a key-value iteration over the DB in v0.8, dumping out the key-values, and re-importing them in v1.0.

@whyrusleeping
Copy link
Member

@manishrjain users have used an older version of badger to store data. Now we're updating to a newer version, if users who used the old version run the new version, they lose data. How do we avoid this?

@whyrusleeping
Copy link
Member

doing what you suggest would mean that ipfs would have to import two different versions of badger, and even then, we would have a hard time telling what an 'old' badger install was and what a 'new' one is to know when we need to even do the conversion.

@manishrjain
Copy link

Before a user updates IPFS to pull in updated Badger, would it be possible to get their data exported to some file? And then, reimport it when IPFS runs?

@whyrusleeping
Copy link
Member

Not easily, most users update by simply running a new binary. We have no way of telling if they updated, or if they just started a new repository with the updated code. Basically, we need a way to tell when to run the process you describe (of exporting the data and reimporting it into the new badger).

Even if we don't decide to support that (because we marked the feature as experimental), we still need to be able to know that we need to reset all the data in a given repository and start over with the new badger instance.

@whyrusleeping
Copy link
Member

I'm starting to think our only real option is (as @magik6k said) to fork badger, update the magic number, and then just maintain that moving forward.

@manishrjain
Copy link

Maybe we can update the magic number in Badger, at the same time as we release v1.0; which should be soon. CC: @deepakjois

@deepakjois
Copy link

Just checking to see if dgraph-io/badger#282 will help you folks here. If so, we can prioritise it for v1.0. Otherwise, we don’t want to make this change in the run-up to v1.0

@magik6k
Copy link
Member Author

magik6k commented Oct 24, 2017

@deepakjois Yes, it will definitely help, we well be able to tell users to upgrade instead of losing data.

@deepakjois
Copy link

@magik6k The change itself is trivial, so I will probably send out a PR for it soon. But before we merge it, I want to confirm it will offer you a clear benefit.

So when you say,

Yes, it will definitely help, we well be able to tell users to upgrade instead of losing data.

could you please elaborate what you mean by telling users to upgrade. You still have to take care of the data that was recorded in the Badger DB with an older version, isn't it (either rewrite it in the new db with the newer magic version, or just ignore it)? Do you already take care of this situation somehow?

@magik6k
Copy link
Member Author

magik6k commented Oct 25, 2017

We have a datastore conversion tool, the plan here is to create a script which will first migrate data from old badger to flatfs/level and then to new badger.

When badger-ds detects unsupported magic number error it will then simply point users to that script, allowing them to convert to newer format

@whyrusleeping
Copy link
Member

@deepakjois currently, if a user with the latest release of ipfs is using badger, and then updates to this PR, They will lose data without warning. At the very least we will need to warn them.

@deepakjois
Copy link

deepakjois commented Oct 25, 2017

After a bit of discussion internally, we have decided to provide a way to backup for v1.0 after all, both for v0.8, and pre v1.0 users.

So there will be a change in the magic number and a way to backup/restore. The details are being worked out: dgraph-io/badger#291

License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@whyrusleeping whyrusleeping merged commit 60bf4fb into master Oct 28, 2017
@ghost ghost removed the status/in-progress In progress label Oct 28, 2017
@whyrusleeping whyrusleeping deleted the gx/badgerds1.0 branch October 28, 2017 14:40
@magik6k magik6k restored the gx/badgerds1.0 branch November 27, 2017 03:34
@magik6k magik6k deleted the gx/badgerds1.0 branch November 27, 2017 03:49
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.

5 participants