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

Use a sync.Pool to allocate KVs during backup. #5579

Merged
merged 3 commits into from
Jun 12, 2020

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Jun 4, 2020


This change is Reviewable

Docs Preview: Dgraph Preview

@martinmr
Copy link
Contributor Author

Copied from ESPLUS-75

Memory profiles for master and my branch:

➜  Downloads go tool pprof -source_path $DGRAPH/dgraph pprof.dgraph.alloc_objects.alloc_space.inuse_objects.inuse_space.097.pb
File: dgraph
Build ID: b5ee2012a3b2face27eee0f8b42ec3df9a404c7b
Type: inuse_space
Time: May 26, 2020 at 6:07am (PDT)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 1353.81MB, 96.80% of 1398.53MB total
Dropped 102 nodes (cum <= 6.99MB)
Showing top 10 nodes out of 57
     flat  flat%   sum%        cum   cum%
 504.08MB 36.04% 36.04%   550.08MB 39.33%  github.com/dgraph-io/dgraph/posting.(*List).SingleListRollup
    256MB 18.30% 54.35%      256MB 18.30%  github.com/dgraph-io/ristretto.newCmRow
 166.41MB 11.90% 66.25%   166.41MB 11.90%  github.com/dgraph-io/badger/v2/skl.newArena
 130.04MB  9.30% 75.54%   130.04MB  9.30%  github.com/dgraph-io/ristretto/z.(*Bloom).Size
     97MB  6.94% 82.48%       97MB  6.94%  github.com/dgraph-io/dgraph/protos/pb.(*BackupKey).Marshal
  88.81MB  6.35% 88.83%    88.81MB  6.35%  github.com/dgraph-io/dgraph/protos/pb.(*BackupPostingList).Marshal
     43MB  3.07% 91.91%       43MB  3.07%  github.com/dgraph-io/dgraph/codec.(*Encoder).Add
  26.24MB  1.88% 93.78%    27.12MB  1.94%  github.com/dgraph-io/badger/v2.(*Stream).streamKVs.func1
  21.62MB  1.55% 95.33%    32.62MB  2.33%  github.com/dgraph-io/badger/v2/pb.(*TableIndex).Unmarshal
  20.61MB  1.47% 96.80%    26.61MB  1.90%  github.com/dgraph-io/dgraph/codec.Decode

➜  Downloads go tool pprof -source_path $DGRAPH/dgraph pprof.dgraph.alloc_objects.alloc_space.inuse_objects.inuse_space.004.pb.gz
File: dgraph
Build ID: 7f6de48d803258423cc253acb49882ca5fa31d81
Type: inuse_space
Time: Jun 9, 2020 at 8:18am (PDT)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 1150.58MB, 99.38% of 1157.74MB total
Dropped 57 nodes (cum <= 5.79MB)
Showing top 10 nodes out of 60
     flat  flat%   sum%        cum   cum%
 386.37MB 33.37% 33.37%   386.37MB 33.37%  github.com/dgraph-io/ristretto.newCmRow
    192MB 16.58% 49.96%      192MB 16.58%  github.com/dgraph-io/ristretto/z.(*Bloom).Size
 166.41MB 14.37% 64.33%   166.41MB 14.37%  github.com/dgraph-io/badger/v2/skl.newArena
 164.52MB 14.21% 78.54%   165.02MB 14.25%  github.com/dgraph-io/dgraph/posting.(*List).SingleListRollup
 133.80MB 11.56% 90.10%   133.80MB 11.56%  github.com/dgraph-io/badger/v2/pb.(*KVList).Marshal
     48MB  4.15% 94.25%       48MB  4.15%  github.com/dgraph-io/dgraph/protos/pb.(*BackupKey).Marshal
  20.83MB  1.80% 96.04%    36.83MB  3.18%  github.com/dgraph-io/badger/v2/pb.(*TableIndex).Unmarshal
     16MB  1.38% 97.43%       16MB  1.38%  github.com/dgraph-io/badger/v2/pb.(*BlockOffset).Unmarshal
     15MB  1.30% 98.72%       15MB  1.30%  github.com/dgraph-io/dgraph/codec.(*Decoder).UnpackBlock
   7.64MB  0.66% 99.38%   253.25MB 21.87%  github.com/dgraph-io/badger/v2.(*Stream).produceKVs.func1
(pprof)

Looks like the utilization of SingleListRollup goes from 500MB to 150MB with my PR. So the in-use memory is improving but we are not seeing those improvements in system memory. My guess is that the golang runtime is not releasing that memory, either because it will try to reuse it.

I think it’s fine to go ahead with these changes since they improved in-use memory. We have run into similar situations before and we didn’t find a way to tell the runtime to release memory back to the OS.

@martinmr martinmr marked this pull request as ready for review June 11, 2020 19:25
@martinmr martinmr requested a review from a team June 11, 2020 19:26
Copy link
Contributor

@parasssh parasssh left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @manishrjain and @vvbalaji-dgraph)

Copy link
Contributor

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

:lgtm:

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @vvbalaji-dgraph)

@martinmr martinmr merged commit 3202d24 into master Jun 12, 2020
@martinmr martinmr deleted the martinmr/backup-kv-pool branch June 12, 2020 21:27
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 18, 2020
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.

3 participants