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

check_hash = yes, means rtorrent will not fire the "completed" event to tracker #437

Open
aeular opened this Issue Jun 8, 2016 · 13 comments

Comments

Projects
None yet
4 participants
@aeular

aeular commented Jun 8, 2016

With the settings at the end of this post, when rtorrent finished a download (added via watch folder), it will not fire the "completed" event to the tracker. If you change the check_hash to no, it will properly fire the completed event to the tracker.

directory = /downloads/torrents/
session = /downloads/.rtorrent/
bind = 10.0.2.15
port_range = 65500-65500
peer_exchange = no

dht = disable

min_peers = 40
max_peers = 100

min_peers_seed = 10
max_peers_seed = 30
max_uploads = 15

max_uploads_global=500
max_downloads_global=50
download_rate = 10240
upload_rate = 10240

scgi_local = /tmp/test.rtorrent.sock
use_udp_trackers = yes
encryption = allow_incoming,try_outgoing,enable_retry
check_hash = yes

schedule = watch_directory,5,5,load_start=/downloads/watch/*.torrent
schedule = low_diskspace,60,60,close_low_diskspace=100G
schedule = chmod,0,0,"execute=chmod,777,/tmp/test.rtorrent.sock"

@chros73

This comment has been minimized.

chros73 commented Jun 13, 2016

Is this intended? 100GB? schedule = low_diskspace,60,60,close_low_diskspace=100G

@aeular

This comment has been minimized.

aeular commented Jun 13, 2016

Not that it matters for the problem, but for the system it is on, yes

@Speeddymon

This comment has been minimized.

Contributor

Speeddymon commented Jul 9, 2016

@chros73 I think this one is caused by missing confirm_finished(download); just after Line 609 in download_list.cc

Can you confirm?

@chros73

This comment has been minimized.

chros73 commented Jul 9, 2016

@Speeddymon Thanks for asking me, but I'm not sure :)

I think (after a quick look) this would be better added here somehow (but you have to understand how the whole chain works):

@Speeddymon

This comment has been minimized.

Contributor

Speeddymon commented Jul 9, 2016

I was looking at that too, but the concern would be if the torrent isn't finished and someone hits ^r then it will send a completed event to the tracker even though the torrent is still downloading.

I'm not a code guru by any means (I'm better with bash and LUA) so I'll leave it to you. :-)

@chros73

This comment has been minimized.

chros73 commented Jul 9, 2016

Can be. But with your solution (I think) if hash checking fails it will still send the finished event to the tracker. So that's also not good.
If you agree then you have to revert your pull request changes. :(

@Speeddymon

This comment has been minimized.

Contributor

Speeddymon commented Jul 9, 2016

Yes, you are correct it would. I didn't make a pull request tho. :-)

@chros73

This comment has been minimized.

chros73 commented Jul 9, 2016

@Speeddymon

This comment has been minimized.

Contributor

Speeddymon commented Jul 9, 2016

Yep. I realized after I posted that my pull request for updating the documentation ended up taking in the change, so I have adjusted the change now and updated the name.

Not sure how to separate pull requests out.

@chros73

This comment has been minimized.

chros73 commented Jul 9, 2016

Not sure how to separate pull requests out.

You have to delete the code from your branch, commit, push it. Then create the pull request again.
After that you can create a new branch for the other mod.

But:

  • do you understand fully what you're doing?
  • have you tried out the change how it works?
  • have you used it in a production environment for at least for 1 day?

I don't want to discourage you, but if not, then remove it instantly, and only add back when you're 100% sure that it works the intended way and it won't break anything else!
There are only a few people (I'm not one of them) who understand the whole system. With adding something crucial like this you can easily trigger some other errors.

@Speeddymon

This comment has been minimized.

Contributor

Speeddymon commented Jul 9, 2016

You have to delete the code from your branch, commit, push it.

Done

Then create the pull request again.

Done

do you understand fully what you're doing?

With this site? Not really. With git in general, a little bit.
I thought that changes I made after making the pull request would not merge into the existing pull request. But I see now that I need a separate branch for each independent pull request.

have you tried out the change how it works?
have you used it in a production environment for at least for 1 day?

No. Since I was trying to only test it in my local copy, I was going to install a VM and clone my local repo to that VM and test it though.

I don't want to discourage you, but if not, then remove it instantly, and only add back when you're 100% sure that it works the intended way and it won't break anything else!
There are only a few people (I'm not one of them) who understand the whole system. With adding something crucial like this you can easily trigger some other errors.

I understand and no offense or discouragement has been taken. :-)

@chros73

This comment has been minimized.

chros73 commented Jul 9, 2016

do you understand fully what you're doing?

Under this I didn't mean git but rtorrent related mods. :)
And thanks for trying to contribute!

@pyroscope

This comment has been minimized.

Contributor

pyroscope commented Jul 10, 2016

That PR branches are "live" is essential for code reviews and fixes caused by them. Just to make clear why they are… 😀

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