downloader: added downloaded torrents notifier #11850
Conversation
| @@ -345,6 +353,7 @@ func New(ctx context.Context, cfg *downloadercfg.Cfg, logger log.Logger, verbosi | |||
| downloading: map[string]*downloadInfo{}, | |||
| webseedsDiscover: discover, | |||
| logPrefix: "", | |||
| completedTorrents: make(map[string]completedTorrentInfo), | |||
There was a problem hiding this comment.
This duplicates the info in downloading above and the info held locally in the local maps here:
could you rationalize this so we end up with a single collection which is held by the downloader which keeps the whole download lifecycle.
This is a rationalization which is well overdue - seems like as you are adding another collection it would be a good time to make this change. Note that you may need to protect access to this map with the downloader mutex
There was a problem hiding this comment.
I did not do this as at some point we are deleting from the complete array.
erigon/erigon-lib/downloader/downloader.go
Line 1110 in ba451aa
This meant that it may not contains completed parts and the complete array may be using for some other stuff so I decided to create own collection.
I think such changes must be done as separate task as it will be to many changes to logic in the same PR.
There was a problem hiding this comment.
The problem is every time we add new collections it makes the code gradually more complex - I don't think the change is that big. You can just add a status with the following states
complete
checking
failed
waiting
to the downloadInfo struct them you only need one map. If you want to add more info you need to do the refactor. I'm ok if you do 2 PR's one with the refactor and the second adding the notifier - but I don't think we should make the code change like this.
There was a problem hiding this comment.
If you want to simplify the change you can just do complete.
At the moment you are setting 2 variable with an external function doing the same thing here:
complete[t.Name()] = struct{}{}
...
d.torrentCompleted(t.Name(), t.InfoHash())
I don't think you should add this code duplication.
erigon-lib/downloader/downloader.go
Outdated
| startTime time.Time | ||
| logPrefix string | ||
| startTime time.Time | ||
| broadcast func(name string, hash *prototypes.H160) |
There was a problem hiding this comment.
can you rename this to notifyCompleted - you don't need an extra wrapper to call a function
Added notifier which notify that torrent downloading completed.