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

Initial commit to use badger instead of RocksDB #932

Closed
wants to merge 12 commits into from
Closed

Conversation

jchiu255
Copy link
Contributor

@jchiu255 jchiu255 commented May 12, 2017

Not committing until we are more sure and do more tests. Just sending out for review first.


This change is Reviewable

@ashwin95r
Copy link
Contributor

ashwin95r commented May 12, 2017

On loading golden data, killing and restarting the server, I'm seeing this crash:

2017/05/12 17:47:37 error.go:75: Arena too small, toWrite:72879 newTotal:77667241 limit:77594624
github.com/dgraph-io/badger/y.Errorf
	/home/ashwin/go/src/github.com/dgraph-io/badger/y/error.go:103
github.com/dgraph-io/badger/y.AssertTruef
	/home/ashwin/go/src/github.com/dgraph-io/badger/y/error.go:75
github.com/dgraph-io/badger/skl.(*Arena).PutVal
	/home/ashwin/go/src/github.com/dgraph-io/badger/skl/arena.go:41
github.com/dgraph-io/badger/skl.newNode
	/home/ashwin/go/src/github.com/dgraph-io/badger/skl/skl.go:122
github.com/dgraph-io/badger/skl.(*Skiplist).Put
	/home/ashwin/go/src/github.com/dgraph-io/badger/skl/skl.go:306
github.com/dgraph-io/badger/badger.NewKV.func1
	/home/ashwin/go/src/github.com/dgraph-io/badger/badger/kv.go:174
github.com/dgraph-io/badger/badger.(*logFile).iterate
	/home/ashwin/go/src/github.com/dgraph-io/badger/badger/value.go:170
github.com/dgraph-io/badger/badger.(*valueLog).Replay
	/home/ashwin/go/src/github.com/dgraph-io/badger/badger/value.go:512
github.com/dgraph-io/badger/badger.NewKV
	/home/ashwin/go/src/github.com/dgraph-io/badger/badger/kv.go:177
github.com/dgraph-io/dgraph/worker.StartRaftNodes
	/home/ashwin/go/src/github.com/dgraph-io/dgraph/worker/groups.go:127
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:2197

@manishrjain
Copy link
Contributor

Change looks good. Let's keep this in a branch until we have ironed out all the issues.

We'll push this out to our demo, and play etc., so it would live tested. Also, I'll try loading the entire Freebase dataset with this branch; and launch freebase.dgraph.io. That should also help test this thing.


Reviewed 44 of 44 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


worker/backup.go, line 258 at r1 (raw file):

		if pk.IsIndex() {
			// Seek to the end of index keys.
			it = pstore.NewIterator(badger.DefaultIteratorOptions) // TODO: Remove this.

why can't we just do it.Seek? Why did you create another iterator?


Comments from Reviewable

@pawanrawal pawanrawal mentioned this pull request May 22, 2017
@pawanrawal pawanrawal self-assigned this May 22, 2017
@pawanrawal
Copy link
Contributor

Review status: 32 of 1335 files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


worker/backup.go, line 258 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

why can't we just do it.Seek? Why did you create another iterator?

We do it now, I suppose the hack was put in place because of the bug in it.Seek


Comments from Reviewable

@pawanrawal
Copy link
Contributor

Merged to master

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.

5 participants