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

Optimize XidtoUID map used by live and bulk loader #2998

Merged
merged 11 commits into from
Feb 11, 2019
Merged

Conversation

manishrjain
Copy link
Contributor

@manishrjain manishrjain commented Feb 10, 2019

I've spent the last few days looking at how to optimize the live mutation path in Dgraph server. While trying many things in the server (past commits included), I realized my optimizations in the server are not improving things much, the throughput saturating at 20-30K NQuads/sec.

Turns out, it was the live loader which was causing the saturation. The XID to UID assigner was the bottleneck causing the throughput to stagnate, despite the server being underutilized.

This PR fixes that by optimizing the assigner. In particular, I've removed the slow LRU cache. Added buffer to newRanges channel to ensure we always have a range handy when we run out. Made passing badger DB instance optional, so we can avoid doing disk writes if not required. And made other optimizations around how we lock, etc. I also added benchmarks for the assigner, which shows each allocation (tested via parallel benchmark) takes 350 ns/op on my desktop.

With these changes, the live loader throughput jumps to 100K-120K NQuads/sec on my desktop. In particular, pre-assigning UIDs to the RDF/JSON file yields maximum throughput. I can load 140M friend graph RDFs in 25 mins.

Helps with #2975 .


This change is Reviewable

Copy link
Contributor

@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.

:lgtm:

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain)


xidmap/xidmap.go, line 60 at r1 (raw file):

}

// This must already have a write lock.

Change comment format to match what the linter expects. Something like:

assign assumes the lock is already acquired at call time.


xidmap/xidmap.go, line 198 at r1 (raw file):

// BumpTo can be used to make Zero allocate UIDs up to this given number. Attempts are made to
// ensure all future allocations of UIDs be higher than this one, but result is not guaranteed.

nit: results are not guaranteed

Copy link
Contributor Author

@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.

Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on @martinmr)


xidmap/xidmap.go, line 60 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

Change comment format to match what the linter expects. Something like:

assign assumes the lock is already acquired at call time.

Done.

@manishrjain manishrjain merged commit b718993 into master Feb 11, 2019
@manishrjain manishrjain deleted the mrjn/xiduidmap branch February 11, 2019 23:47
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
I've spent the last few days looking at how to optimize the live mutation path in Dgraph server. While trying many things in the server (past commits included), I realized my optimizations in the server are not improving things much, the throughput saturating at 20-30K NQuads/sec.

Turns out, it was the live loader which was causing the saturation. The XID to UID assigner was the bottleneck causing the throughput to stagnate, despite the server being underutilized.

This PR fixes that by optimizing the assigner. In particular, I've removed the slow LRU cache. Added buffer to `newRanges` channel to ensure we always have a range handy when we run out. Made passing badger DB instance optional, so we can avoid doing disk writes if not required. And made other optimizations around how we lock, etc. I also added benchmarks for the assigner, which shows each allocation (tested via parallel benchmark) takes 350 ns/op on my desktop.

With these changes, the live loader throughput jumps to 100K-120K NQuads/sec on my desktop. In particular, pre-assigning UIDs to the RDF/JSON file yields maximum throughput. I can load 140M friend graph RDFs in 25 mins.

Helps with dgraph-io#2975 .

Changes:
* Work on optimizing XidToUid map.
* Add the test and benchmark for xid to uid map
* Working code with decreased memory usage. Includes a new BumpUp API.
* Working live loader, which can optionally just keep all the mapping in memory.
* Adding shards back to XidMap speed up operations by a huge factor. Benchmark shows each allocation is 300ns.
* Make BumpTo much faster by calling Zero directly, instead of looping through the newRanges channel.
* Improve how BumpTo() happens by using a maxSeenUid variable.
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.

2 participants