Skip to content
Snippets Groups Projects

Add issuable searches to search rate limit

Merged Heinrich Lee Yu requested to merge 340836-rate-limit-issuable-searches into master
All threads resolved!

What does this MR do and why?

Rate limits issuable searches so that we can re-enable searching made by anonymous users. This uses the existing search rate limit settings.

This is behind a feature flag rate_limit_issuable_searches.

Note: I also considered rate limiting at the finder level, but I think there may be other places that use the finder but we don't want to include in the limit. For example, in the recent issues autocomplete finder, it adds a scope with a list of issue IDs so the searches here are fast and we do not need to limit them.

Related to #340836

How to set up and validate locally

  1. Enable rate_limit_issuable_searches
  2. (Optional) Lower the search rate limit in Admin > Settings > Network > Search rate limits
  3. Load a page like http://127.0.0.1:3000/dashboard/issues?scope=all&state=opened&search=test many times to go over the limit
  4. Get a 429 response

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Heinrich Lee Yu

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • Resolved by Heinrich Lee Yu
      1 Message
      :book: CHANGELOG missing:

      If you want to create a changelog entry for GitLab FOSS, add the Changelog trailer to the commit message you want to add to the changelog.

      If you want to create a changelog entry for GitLab EE, also add the EE: true trailer to your commit message.

      If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

      Reviewer roulette

      Changes that require review have been detected!

      Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

      Category Reviewer Maintainer
      backend Michał Wielich current availability (@michold) (UTC+1, 7 hours behind @engwan) Mehmet Emin Inac current availability (@minac) (UTC+1, 7 hours behind @engwan)
      UX Ali Ndlovu current availability (@andlovu) (UTC+2, 6 hours behind @engwan) Maintainer review is optional for UX

      To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

      To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

      Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

      If needed, you can retry the :repeat: danger-review job that generated this comment.

      Generated by :no_entry_sign: Danger

  • Heinrich Lee Yu added 246 commits

    added 246 commits

    Compare with previous version

  • added 1 commit

    • fca9ced8 - Add issuable searches to search rate limit

    Compare with previous version

  • Allure report

    allure-report-publisher generated test report!

    e2e-package-and-test: :x: test report for 11dd7315

    expand test summary
    +-----------------------------------------------------------------------+
    |                            suites summary                             |
    +------------------+--------+--------+---------+-------+-------+--------+
    |                  | passed | failed | skipped | flaky | total | result |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Configure        | 0      | 0      | 6       | 0     | 6     | ➖     |
    | Create           | 318    | 0      | 10      | 0     | 328   | ✅     |
    | Govern           | 84     | 0      | 2       | 0     | 86    | ✅     |
    | Fulfillment      | 4      | 0      | 28      | 0     | 32    | ✅     |
    | Verify           | 96     | 0      | 8       | 0     | 104   | ✅     |
    | Manage           | 126    | 2      | 6       | 0     | 134   | ❌     |
    | Plan             | 120    | 0      | 0       | 0     | 120   | ✅     |
    | Release          | 12     | 0      | 0       | 0     | 12    | ✅     |
    | Secure           | 14     | 0      | 2       | 6     | 16    | ❗     |
    | Analytics        | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Package          | 0      | 0      | 6       | 0     | 6     | ➖     |
    | Framework sanity | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Monitor          | 2      | 0      | 0       | 0     | 2     | ✅     |
    | ModelOps         | 0      | 0      | 2       | 0     | 2     | ➖     |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Total            | 780    | 2      | 72      | 6     | 854   | ❌     |
    +------------------+--------+--------+---------+-------+-------+--------+

    e2e-review-qa: :exclamation: test report for 58447005

    expand test summary
    +-----------------------------------------------------------------------------------------+
    |                                     suites summary                                      |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    |                                    | passed | failed | skipped | flaky | total | result |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    | Govern                             | 10     | 0      | 5       | 1     | 15    | ❗     |
    | Plan                               | 49     | 0      | 1       | 0     | 50    | ✅     |
    | Manage                             | 39     | 0      | 4       | 2     | 43    | ❗     |
    | Verify                             | 12     | 0      | 1       | 0     | 13    | ✅     |
    | Create                             | 28     | 0      | 1       | 0     | 29    | ✅     |
    | Feature flag handler sanity checks | 9      | 0      | 0       | 0     | 9     | ✅     |
    | Configure                          | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Version sanity check               | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Package                            | 0      | 0      | 1       | 0     | 1     | ➖     |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    | Total                              | 147    | 0      | 15      | 3     | 162   | ❗     |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
  • Heinrich Lee Yu added 298 commits

    added 298 commits

    Compare with previous version

  • Ali Ndlovu marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

    marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

  • Ali Ndlovu approved this merge request

    approved this merge request

  • :wave: @andlovu, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.

    For more info, please refer to the following links:

  • 🤖 GitLab Bot 🤖 added 1 deleted label

    added 1 deleted label

  • Heinrich Lee Yu added 207 commits

    added 207 commits

    Compare with previous version

  • added 1 commit

    • 22589065 - Add issuable searches to search rate limit

    Compare with previous version

  • 🤖 GitLab Bot 🤖 changed milestone to %15.7

    changed milestone to %15.7

  • Heinrich Lee Yu added 517 commits

    added 517 commits

    Compare with previous version

  • Heinrich Lee Yu resolved all threads

    resolved all threads

  • added 1 commit

    • 14b24e7e - Add issuable searches to search rate limit

    Compare with previous version

  • added 1 commit

    • f4069c10 - Add issuable searches to search rate limit

    Compare with previous version

  • added 1 commit

    • 58447005 - Add issuable searches to search rate limit

    Compare with previous version

  • Heinrich Lee Yu added 3414 commits

    added 3414 commits

    Compare with previous version

  • added 1 commit

    • fad29a27 - Add issuable searches to search rate limit

    Compare with previous version

  • added 1 commit

    • 5a8cfe3b - Add issuable searches to search rate limit

    Compare with previous version

  • Heinrich Lee Yu added 1267 commits

    added 1267 commits

    Compare with previous version

  • Heinrich Lee Yu
  • Heinrich Lee Yu
  • Heinrich Lee Yu
  • requested review from @pedropombeiro

  • Heinrich Lee Yu changed the description

    changed the description

  • 🤖 GitLab Bot 🤖 changed milestone to %15.8

    changed milestone to %15.8

  • added 1 commit

    • 11dd7315 - Add issuable searches to search rate limit

    Compare with previous version

  • approved this merge request

  • Pedro Pombeiro (OOO from Feb 17th-21st) requested review from @mkozono and removed review request for @pedropombeiro

    requested review from @mkozono and removed review request for @pedropombeiro

  • @engwan LGTM! Since I don't have any blocking feedback, I opened a feature flag rollout issue, but please add it to the feature flag definition when you have a chance. Thank you! :rocket:

  • Michael Kozono approved this merge request

    approved this merge request

  • Michael Kozono resolved all threads

    resolved all threads

  • Michael Kozono enabled an automatic merge when the pipeline for 2ffd60ee succeeds

    enabled an automatic merge when the pipeline for 2ffd60ee succeeds

  • Michael Kozono mentioned in commit 79ba45a4

    mentioned in commit 79ba45a4

  • added workflowstaging label and removed workflowcanary label

  • Please register or sign in to reply
    Loading