accept tracker connections#187
Conversation
852c04f to
ad725e2
Compare
dc0797b to
f9e6a92
Compare
| selector.select(); | ||
| for (SelectionKey key : selector.selectedKeys()) { | ||
| if (key.isAcceptable()) { | ||
| this.listenChannel.accept(); |
There was a problem hiding this comment.
Note no I/O is done here at the moment, we only accept the connection. Any ideas how to proceed, if at all?
There was a problem hiding this comment.
The macro vision is:
- When a peer connect though bittorrent protocol => send a choke message
But "connect through bittorrent protocol" is easier said than done 😆 .
it's something related to : https://github.com/mpetazzoni/ttorrent/blob/2c8bb62f9f69b74c014358d446554df96ec91c7c/ttorrent-client/src/main/java/com/turn/ttorrent/client/network/HandshakeReceiver.java#L148
Instead of adding peers to whatever store you'll just choke them (maybe close connection?).
This feature require a deep comprehension of the peer section of BitTorrent RFC (and also to investigate the source code of libtorrent and other OpenSource clients to understand how they individualy do their job).
|
Ye this is going to need quite a bit of research/know-how. |
Two users. :-) I have been waiting for this merge, and updated Docker image, since the opening of this PR. |
|
Hello there, i've not forgotten about you. But i'm lacking time to review that. I try to give it a look ASAP |
|
Do you have any idea if and when you'll have the time to look at this, @anthonyraymond? I don't want to rush you, I'm already very grateful for what you did with this project so far, I'm just looking for an ETA. |
|
Hello, An analogy for this PR is:
|
|
That is a valid point. Would it be possible to make it optional in config? @laur89 @anthonyraymond |
|
@anthonyraymond i think it's best to just add a toggle in settings. for me it works great but maybe others can have issues, indeed. can't wait to see this merged and published <3 |
|
I don't think the config option will come anytime soon either, right? 😔 |
|
@anthonyraymond can we please merge this as well? been using it since then and it's all good. maybe add a config toggle in the settings just in case? |
|
@rursache i don't think this is ever going to be added as is. It's sounds a bit dangerous for most people. I'll include a real and complete implementation in the next version (built from scratch), but for now i prefer not to include it as is |
thank you, looking forward to your implementation! |
|
@anthonyraymond i've been using it for 2 years already, works fine. most trackers refuse joal with the normal release |
f9e6a92 to
14cf7b0
Compare
|
Hello :) After a period of testing I noticed a higher RAM usage that keeps increasing over time when merging this PR (see here #268 ). Can someone confirm this behavior on their side ? |
|
@Neurology0443 saw it as well but had a similar issue before the PR as well. for years i simply had a hourly cronjob to restart the contaier and that's fine so far. or it's probably easier to limit ram usage via docker compose and let it handle this |
|
Thanks for your answer 👍 I tried with the following docker-compose env. variable: From the docker logs it seems to pick-up the variable, however it's ignored: Yeah, a hourly cronjob is more of a workarround than a real solution, but I guess that's how it is... Invidious has a similar issue anyway :/ Thanks ! |
Note this change is on top of #172 -- merge that beforehand.merged