Improved downloader webseed performance #10715
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erigon-lib/downloader/downloader.go
Outdated
| // if we have created a torrent bit it has no info assume that the | ||
| // webseed download either has not been called yet or has failed and | ||
| // try again here - otherwise the torrent will be left with no info | ||
| if t.Info() == nil { |
There was a problem hiding this comment.
select {
case <-t.GotInfo():
continue
default:
}
is better than t.Info() from "mutex contention" perspective.
There was a problem hiding this comment.
Info now has its own mutex - so contention is minimal - as its only locked written by set info. So I think that t.Info() should be preferred for call simplicity.
| if ok && err == nil { | ||
| _, _, err = addTorrentFile(d.ctx, ts, d.torrentClient, d.db, d.webseeds) | ||
| if err != nil { | ||
| continue |
| @@ -65,7 +65,7 @@ func Default() *torrent.ClientConfig { | |||
| // better don't increase because erigon periodically producing "new seedable files" - and adding them to downloader. | |||
| // it must not impact chain tip sync - so, limit resources to minimum by default. | |||
| // but when downloader is started as a separated process - rise it to max | |||
| //torrentConfig.PieceHashersPerTorrent = max(1, runtime.NumCPU()-1) | |||
| torrentConfig.PieceHashersPerTorrent = max(1, runtime.NumCPU()-2) | |||
There was a problem hiding this comment.
because there is no global limiter for hashers - on non-nvme drives it may crash with > 10K threads created panic. because all threads will wait for overloaded/just_slow disk (and go creating dedicated thread for each IO-blocked goroutine).
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
|
|
||
| tx, err := m.db.BeginRwNosync(context.Background()) |
There was a problem hiding this comment.
please use BeginRw instead of BeginRwNosync. otherwise you can loose last committed transaction on power-off.
|
|
||
| m.putFlushed(tx, infoHash, flushed) | ||
|
|
||
| tx.Commit() |
There was a problem hiding this comment.
what if commit returns error?
AskAlexSharov
approved these changes
Jun 13, 2024
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
| //fmt.Println("FLUSH", infoHash) | ||
| m.db.Batch(func(tx kv.RwTx) error { |
There was a problem hiding this comment.
Batch - is designed for much parallel calls. but currently it's behind mutex. In this case Update will perform better.
Or maybe it's possible to do behind mutext only map get/put.
Or just don't use mdbxPieceCompletionBatch if Update inside Set is not a problem anymore.
up to you.
There was a problem hiding this comment.
I've removed this so now we're using the original unbatched version of the code - with the alterations you suggested.
I have found with sepolia things seems to run ok with default settings, for bor-mainnet I found I need to set DL hashers to 20 in order to avoind memory growth - the hashers can't keep up with the data download. Too may hahsers though and CPU's get maxed out, althouhg hashing iteslf only seems to hake ~25% of the processing time. The rest is copying memory and GC. |
I have resolved all of the DATA races I see for the moment. |
This is resolved - it was caused by a cancelled flag being incorrectly set. I have seen several other panics - which I have investigated and resolved. I think there may be a couple of issue left since I made the lib more active - but think its worth merging and investigating. The reamining issue seem to be mostly associated with peer flow. Which I'll start to test this week. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This contains fixes, mainly in erigontech/torrent which improve the parallelization of the lib to support downloads speeds over ~25MB per second on a reliable basis.
--torrent.download.rate=256mb --torrent.download.slots=32should now run a download to completion at a consistent ~250MB/second download rate, assuming64GBof available memory.If more memory is available
--torrent.download.rate=512mb --torrent.download.slots=48will around ~400MB/second. Its not yet clear where the loss in performance is but for this version ofthe code it seems 400MB/second is around the maximum it can support.Outstanding Issues
The current version of the code is memory hungry at high bandwidths the reason behind this is the way the http data is dealt with under high load. The buffer model is currently:
http->hashing->mmapWhere both the http connection and the torrent hasher will retain intermediate buffers until they are finally flushed to te memory mapped file. A more memory efficient model would be to get the http connection to write directly to the memory mapped segment which can the be directly hashed. This will require further code modification - which is outside of the scope of this change.
Downloader changes
Along with the torrent lib changes a number of changes have been made to the downloader code. The most significant are:
mdbxPieceCompletionnow has aFlushed(infoHash infohash.T, flushed *roaring.Bitmap)method which is used to commit the completion status to the db after an asynchronous flush of the mmap files has been made. This means that the completion state will only be confirmed once the data is flushed. (This may lead to re-downloading of peices in the case of a crash.