Source tracking issue: Distributed Rate Limiter v2

[mrnugget]

Goal

• Admins don't get confused by it
  • "I don't want to send more than X request to this given server"
• Developers at Sourcegraph don't have to worry about rate limits again

Milestones/Tasks

• [ ] Understand how we use rate limiting exactly and how we could switch from URN-based to host-based 2d
  • [ ] Package repo external_services usually connect to more than one code host, URN is system-wide
  • [ ] What happens if we switch away from per-connection? That'll mean you cannot go as granular.
  • [ ] This is blocking switching to host-based limits and moving to the HTTP CLI
• [x] Remove rateLimit field from perforce; no respect 1d
• [ ] Move rate limits to the per-URL limit 5d++
  • [ ] Have an entity that stores the code-host wide rate-limit
  • [ ] Implement UIs to view and edit the rate-limits
  • [ ] Update rate limit sync worker to read from that entity instead
  • [ ] Update rate limiter code to read from code-host-wide rate-limits
  • [ ] Requires: Knowing the host for the request to be made
  • [ ] Do NOT support per code host connection rate limits anymore
• [ ] Live in the HTTP CLI so that integrators with external services never need to worry about it or could forget it Xd
  • [ ] Should there be a bypass?
  • [x] The same should be true for the GitHub.com singleflight lock
• [ ] Once we are in the HTTP CLI, it would be great to have an HTTP grafana dashboard to see what services we wait on, how many requests, and so forth 3d
• [x] Wrap up removal of GitHub Proxy 4d
  • [x] Finish removing it from all deployment types 1d
  • [x] Move the lock into the HTTP CLI layer 3d
• [ ] Consolidate setting names and move them onto the right entities
  • [x] siteConfig.gitMaxConcurrentClones is not across gitservers, make it distributed 2d
  • [ ] Rename gitMaxCodehostRequestsPerSecond to operations. Also, per second is a bad metric, doesn't allow less than 3600 an hour
  • [ ] Do that in site config 1.5d
  • [ ] OR
  • [ ] Move it to the set of limits defined per code host as well 1.5d
  • [x] Consider moving siteConfig.repoListUpdateInterval into the code host connection config 4d
  • [x] Maybe make rate limit burst configurable?
  • [x] Thought: It might just confuse customers more and give then even more knobs to try to get around issues - this should be pretty opaque and not change a lot in practice. If at all, disabling burst at all might be useful.

[eseliger]

Closing the ticket. We landed the smaller improvements we wanted to make, we did not switch to URL-based limiting or code host model because of the way packages are designed today. We can come back to that later.

GitHub proxy and GitHub concurrency limiting have been fully removed and tested extensively through our largest GitHub-connected instance, sourcegraph.com.

QA plan to follow.

[eseliger]

QA Test Plan

Summary

We've made multiple adjustments to rate limits in Sourcegraph. The most notable are the full removal of the github-proxy service, making deployments simpler and a tad cheaper. In addition, the GitHub.com concurrency limiter is no longer used, removing one source of confusion. Perforce rate limits are no longer configurable, as they weren't having any effect (and they can not because perforce doesn't use the HTTP protocol).

Test Plan

Integration testing: Verify that all the components of the application integrate and work together seamlessly.

Sourcegraph.com and S2 have not changed their behavior after these changes, and they have been running for months.

Performance testing: Verify that the application performs efficiently and effectively under different loads and stress levels.

We verified that the new rate limiters and GitHub.com limiter don't run into issues on either S2 or dotcom, through monitoring of our prometheus metrics.

Usability testing: Verify that the application has met design guidelines and meets the needs of our users. If you did not work with a designer, or you have visual or UX changes that have not been QAed by a designer, please reach out at #ask-design as soon as possible!

This is not a user-facing feature, so no UX/UI changes to consider here.

Compatibility testing: Verify that the application works on all supported distribution environments (Compose, K8s, Helm, Dotcom, Cloud, etc).

The changes have been tested with kubernetes, dotcom, and cloud.

Cody Checklist (If applicable)

1. Does your feature respect the global Cody feature flag? Default experience is OFF for Self Hosted

   • [ ] Yes
   • [ ] No
   • [x] N/A

2. Have you backtested prompts and questions that have worked before? IE - Can you summarize the following file?

   • [ ] Yes
   • [ ] No
   • [x] N/A

   If yes, please specify your test prompts and the response:

3. Have you made any client-side changes to Cody? To prevent breakages, please ensure your changes have been tested on other Cody platforms (Web, App, etc)

   • [ ] Yes, I've tested my changes across the different applicable Cody platforms
   • [ ] No
   • [x] N/A

4. Which clients have your changes been tested on?

   • [ ] VS Code
   • [ ] Cody App
   • [ ] Sourcegraph Web
   • [ ] IntelliJ
   • [ ] Other

   If other, please specify:

   5. Please provide any additional testing you've done that has not been covered above:

   If you notice any bugs related to the setup experience or Cody in general please raise them in #team-cody and tag @teamcody & @design-team.

QA Checklist

1. Have you made any infra related changes to environment variables, new services, or deployment methods that could affect customers?

   • [ ] Yes - I've informed #team-cloud and #team-delivery of the changes.
   • [x] Yes - Changelog or documentation has been updated, this includes changes to defaults and site config flags.
   • [ ] No

   If your change is non-trivial, please review the Cloud Launch process.

   If you've made changes to documentation, please link them in the comments below.

   Comments: The change has already been partially applied in 5.2, in 5.3 all deployment types completed this change.

2. Which environments have the changes been tested on?

   • [ ] rctest.sourcegraph.com
   • [x] sourcegraph.sourcegraph.com
   • [x] sourcegraph.com
   • [ ] cody-dev.sgdev.dev (running based off 5.2 release branch)
   • [ ] scaletesting.sgdev.org
   • [ ] other

   If other, please specify:

3. Experimental features have been marked and behind a feature flag?

   • [ ] Yes
   • [ ] No
   • [x] N/A

   If no, please specify why:

4. Has telemetry been added as part of the product requirements?

   • [ ] Yes
   • [ ] No
   • [x] N/A

5. Completed entry to release post.

   • [ ] Yes
   • [x] No
   • [ ] N/A

6. Is a feature flagged in a way when turns the feature off, it behaves as-if the feature does not exist?

   • [ ] Yes
   • [ ] No
   • [x] N/A

   If no, please specify why:

7. A CHANGELOG entry has been added for the feature/change?

   • [x] Yes
   • [ ] No
   • [ ] N/A

8. Please provide any additional testing you've done that has not been covered above:

---

Tech Lead/DRI sign off: @eseliger