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

Reworked proxy tests to use requests_futures instead of custom threads. #2118

Merged
merged 10 commits into from Jul 12, 2017

Conversation

Projects
None yet
3 participants
@sebastienvercammen
Member

sebastienvercammen commented Jul 6, 2017

Description

Consistent with how we send webhook data, proxy tests now use requests_futures' FutureSession for asynchronous requests and a proper worker pool.

FutureSession with urllib3's Retry allows for async requests that can automatically be retried, although retrying is disabled for now until we properly implement automatically retrying API requests to allow unstable proxies.

Also cleaned up proxy.py and separated proxy loading from proxy checking.

Added config items:

  • -pxre, --proxy-test-retries: Number of times to retry sending proxy test requests on failure. Default: 0.
  • -pxbf, --proxy-test-backoff-factor: Factor (in seconds) by which the delay until next retry will increase. Default: 0.25.
  • -pxc, --proxy-test-concurrency: Async requests pool size. Default: 0 (= automatically set pool size to the number of proxies we have to test).

Tested performance difference is between 10% and 30%.

Motivation and Context

Readability, consistency, performance.

Types of changes

  • Enhancement

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@sebastienvercammen sebastienvercammen added this to the 4.0.0 milestone Jul 6, 2017

@sebastienvercammen sebastienvercammen changed the title from [WIP] Reworked proxy tests to use requests_futures instead of custom threads. to Reworked proxy tests to use requests_futures instead of custom threads. Jul 8, 2017

@jagauthier

I tested this, extensively. With a myriad of proxies. 5, 15, 20, 50, 80. The tests themselves were interesting as some proxies would be "ok" one moment and "timeout" the next. But this is not due to the PR, but the proxy. It just made testing interesting. The overall performance increase in the proxy checks themselves is good. With 7 fully functioning proxies, (no timeouts) , I experienced a about 25-30% performance increase in the proxy check. With random proxies that worked sometimes, I would experience 10-15% increase. Also, cuts down on thread usage which is a huge bonus.

@friscoMad

Codewise seems ok, I don't like the flag creep we have but otherwise it's ok. After your tests, arengo and Aspirax I think it is working without issues, but did not test myself

@sebastienvercammen

This comment has been minimized.

Member

sebastienvercammen commented Jul 12, 2017

Received more test reports from Alderon, Arengo, and some other people on Discord. All test results are positive, with all of them showing improvement (on minimum, median and average timing) between 10% and 30%.

I've also renamed all config items to start with --proxy-test- rather than --proxy- so they are clearer and can't be confused with general proxy settings not related to proxy testing.

@sebastienvercammen sebastienvercammen merged commit 2024fae into RocketMap:develop Jul 12, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@friscoMad friscoMad referenced this pull request Jul 13, 2017

Merged

Fix proxy regression #2134

2 of 6 tasks complete

ixotech1 added a commit to ixotech1/RocketMap that referenced this pull request Jul 24, 2017

Reworked proxy tests to use requests_futures instead of custom thread…
…s. (RocketMap#2118)

* Rework proxy test to use requests_futures.

* Update missed references.

* Updated commandline docs.

* Added missing refernce.

* Removed unnecessary param.

* Separate sessions per host + support automatic pool sizing.

* Update missed reference.

* Remove duplicate code.

* Rename arguments for clarity.

@sebastienvercammen sebastienvercammen deleted the sebastienvercammen:efficient-proxies branch Aug 14, 2017

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