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

feat(core-p2p): add hapi-rate-limit plugin #1314

Merged
merged 7 commits into from Nov 8, 2018

Conversation

Projects
None yet
2 participants
@faustbrian
Collaborator

faustbrian commented Nov 7, 2018

Proposed changes

Adds https://github.com/wraithgar/hapi-rate-limit as rate-limiting/throttling plugin. A list of available options can be found here https://github.com/wraithgar/hapi-rate-limit#options.

I marked this PR was WIP because we need to play with the value of the userLimit variable to find a good number that makes sense for throttling but won't cause any network issues.

Partially resolves #1313

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes

@faustbrian faustbrian requested a review from supaiku0 Nov 7, 2018

@wafflebot wafflebot bot added the review label Nov 7, 2018

faustbrian added some commits Nov 7, 2018

@codecov-io

This comment has been minimized.

codecov-io commented Nov 7, 2018

Codecov Report

Merging #1314 into develop will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1314      +/-   ##
===========================================
+ Coverage    74.02%   74.12%   +0.09%     
===========================================
  Files          422      420       -2     
  Lines         7400     7365      -35     
  Branches      1006     1000       -6     
===========================================
- Hits          5478     5459      -19     
+ Misses        1692     1679      -13     
+ Partials       230      227       -3
Impacted Files Coverage Δ
packages/core-p2p/lib/defaults.js 100% <ø> (ø) ⬆️
packages/core-p2p/lib/server/index.js 93.75% <100%> (-6.25%) ⬇️
...kages/core-p2p/lib/server/versions/remote/index.js 0% <0%> (-100%) ⬇️
...p/lib/server/versions/remote/schemas/blockchain.js 0% <0%> (-100%) ⬇️
.../lib/server/versions/remote/handlers/blockchain.js 0% <0%> (-50%) ⬇️
packages/core-forger/lib/manager.js 70.1% <0%> (-9.28%) ⬇️
...p2p/lib/server/versions/internal/handlers/utils.js 80% <0%> (-6.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9245249...4d4b9ff. Read the comment docs.

faustbrian added some commits Nov 7, 2018

@faustbrian faustbrian changed the title from [WIP] feat(core-p2p): add hapi-rate-limit plugin to feat(core-p2p): add hapi-rate-limit plugin Nov 8, 2018

@faustbrian

This comment has been minimized.

Collaborator

faustbrian commented Nov 8, 2018

I've set the default rate limit to 800 for now as the average number of requests per minute between 2 peer seems to be around 600 requests.

faustbrian added some commits Nov 8, 2018

@faustbrian faustbrian merged commit a1f430f into develop Nov 8, 2018

1 check passed

ci/circleci: test-node10 Your tests passed on CircleCI!
Details

@faustbrian faustbrian deleted the p2p-rate-limit branch Nov 8, 2018

@wafflebot wafflebot bot removed the review label Nov 8, 2018

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