bitbucketserver: Document rate limit change and rationale #9048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 128069c. @tsenart in this commit you increased the Bitbucket Server rate limit, which is OK, but: 1. There isn't a CHANGELOG entry. Telling folks we plan to make up to 4x the number of requests to their Bitbucket instance (and why we think this isn't an issue) is _important_, especially because many customers are aware of us in the past having issues with taking down / causing harm to code hosts due to excessive API requests. 2. Even if we don't believe this will cause issues (which seems reasonable to me), we should have some documented form of recourse. This can be as simple as "If you encounter issues, please downgrade and contact support." in the CHANGELOG entry. 3. You removed the comment explaining how we chose these limits. Anyone reading this code will feel it is arbitrary, and anyone looking at the history will think we went from something logical ("same as bitbucket cloud") to something arbitrary (explanation removed). 4. You removed the `// 120/min or 7200/hr` comment, which is helpful when linking customers/users to this code as an explanation of the max we will do. Note: I know I could've just fixed these for you more easily, my hope is this clarifies why I think this is important to communicate to our consumers.
|
tsenart
changed the title
tsenart
approved these changes
Mar 17, 2020
unknwon
reviewed
Mar 17, 2020
| @@ -34,6 +34,7 @@ All notable changes to Sourcegraph are documented in this file. | |||
| - Archived repositories are excluded from search by default. Adding `archived:yes` includes archived repositories. | |||
| - Forked repositories are excluded from search by default. Adding `fork:yes` includes forked repositories. | |||
| - CSRF and session cookies now set `SameSite=None` when Sourcegraph is running behind HTTPS and `SameSite=Lax` when Sourcegraph is running behind HTTP in order to comply with a [recent IETF proposal](https://web.dev/samesite-cookies-explained/#samesitenone-must-be-secure). As a side effect, the Sourcegraph browser extension and GitLab/Bitbucket native integrations can only connect to private instances that have HTTPS configured. If your private instance is only running behind HTTP, please configure your instance to use HTTPS in order to continue using these. | |||
| - The Bitbucket Server rate limit that Sourcegraph self-imposes has been raised from 120 req/min to 480 req/min to account for Sourcegraph instances that make use of Sourcegraphs' Bitbucket Server repository permissions and campaigns at the same time (which require a larger number of API requests aginst Bitbucket). The new number is based on us consuming roughly 8% the average API request rate against at large customers' Bitbucket Server instances. [#9048](https://github.com/sourcegraph/sourcegraph/pull/9048/files) | |||
There was a problem hiding this comment.
Is this a typo? s/aginst/against/?
There was a problem hiding this comment.
Omg, I typed a typo while fixing a typo 🤦♂️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR documents the change made in #9038.
@tsenart in this commit you increased the Bitbucket Server rate limit, which is OK, but:
// 120/min or 7200/hrcomment, which is helpful when linking customers/users to this code as an explanation of the max we will do.Note: I know I could've just fixed these for you more easily, my hope is this clarifies why I think this is important to communicate to our consumers.