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

Memory usage is too high #20

Closed
pboothe opened this issue Nov 19, 2019 · 5 comments · Fixed by #21
Closed

Memory usage is too high #20

pboothe opened this issue Nov 19, 2019 · 5 comments · Fixed by #21

Comments

@pboothe
Copy link
Contributor

pboothe commented Nov 19, 2019

No description provided.

pboothe added a commit that referenced this issue Nov 19, 2019
Instead, buffer packet headers in in-memory files.

Fixes #20
@autolabel autolabel bot removed the review/triage label Nov 19, 2019
@stephen-soltesz
Copy link
Contributor

Memory usage is still too high, causing node failure at less than 10 simultaneous ndt5 download clients.

Using pprof heap profiling, the culprit is the very generous allocation in newTCP() for the buffered channel pchan with 833333 capacity.

Due to ndt5 protocol, every "test" generates two tcp connections that remain allocated for at least 60s, the load test at ~10 streams, will generate around 100 connections / minute (assuming ~12sec / test).

Once there are ~150 streams allocated, that's ~2GB just for pointers.

Writing directly do disk also eliminates the in-memory buffering.

@stephen-soltesz
Copy link
Contributor

@stephen-soltesz
Copy link
Contributor

(I'm working on patches for this)

@pboothe
Copy link
Contributor Author

pboothe commented Nov 27, 2019

Let's not wait for the UUID. Either it arrives by the time we stop capturing, or we can assume it will never arrive.

@stephen-soltesz
Copy link
Contributor

While theoretically it may be possible to create a fixed-memory profile for packet-headers (rather than one that grows proportionately with number of connections) the memory usage of the latest version (v0.5.4) is not too high.

I believe this issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants