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

Limit the number of simultaneous connections per web seed #664

Merged

Conversation

@grunjol
Copy link
Contributor

grunjol commented Mar 9, 2016

PR #663 limits the amount of connections in case of low bandwidth web seed
But in case of per IP connection limits like

nginx's limit_conn addr
Wowza
Nimble

this prevents connection rejections (HTTP 50x) and consecuently web seed been discarded as discussed in #650

README.md Outdated
@@ -275,7 +275,8 @@ If `opts` is specified, then the default options (shown below) will be overridde
announce: [], // Torrent trackers to use (added to list in .torrent or magnet uri)
getAnnounceOpts: function, // Custom callback to allow sending extra parameters to the tracker
path: String, // Folder to download files to (default=`/tmp/webtorrent/`)
store: Function // Custom chunk store (must follow [abstract-chunk-store](https://www.npmjs.com/package/abstract-chunk-store) API)
store: Function // Custom chunk store (must follow [abstract-chunk-store](https://www.npmjs.com/package/abstract-chunk-store) API),

This comment has been minimized.

Copy link
@DiegoRBaquero

DiegoRBaquero Mar 9, 2016

Member

The comma is wrongly placed

This comment has been minimized.

Copy link
@grunjol

grunjol Mar 9, 2016

Author Contributor

you are right, fixed

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

DiegoRBaquero commented Mar 9, 2016

Looks good to me. This is a nice option to have so connection limits per IP don't break the whole thing.

@grunjol

This comment has been minimized.

Copy link
Contributor Author

grunjol commented Mar 10, 2016

@feross is continuous-integration/appveyor/branch working? it seems is stopping all PRs to be ready to merge

@feross

This comment has been minimized.

Copy link
Member

feross commented Mar 11, 2016

I'm fine with this as a temporary stop-gap. But this is gross. All this complexity should be handled for the user by the web seed implementation.

feross added a commit that referenced this pull request Mar 11, 2016
…imit

Limit the number of simultaneous connections per web seed
@feross feross merged commit a4d5d7f into webtorrent:master Mar 11, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@feross

This comment has been minimized.

Copy link
Member

feross commented Mar 11, 2016

0.83.1.

@lock

This comment has been minimized.

Copy link

lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.