Skip to content
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

Search: Fix potential prolonged rate-limiting #5524

Merged
merged 7 commits into from
Apr 30, 2024

Conversation

rinatkhaziev
Copy link
Contributor

@rinatkhaziev rinatkhaziev commented Apr 30, 2024

Description

In some high-concurrency cases rate-limiting window never expires and results in prolonged rate-limit.

Changelog Description

Fixed

  • Address Enterprise Search potential prolonged rate-limiting in high concurrency scenarios.

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally or in Codespaces (or has an appropriate fallback).
  • This change works and has been tested on a sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

…ct didn't have an expiration which may prevent the ratelimit window reset
@rinatkhaziev rinatkhaziev requested a review from a team as a code owner April 30, 2024 16:37
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.80%. Comparing base (0b231ce) to head (45f851b).

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #5524      +/-   ##
=============================================
+ Coverage      28.77%   28.80%   +0.03%     
  Complexity      4844     4844              
=============================================
  Files            284      284              
  Lines          21017    21010       -7     
=============================================
+ Hits            6047     6052       +5     
+ Misses         14970    14958      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Apr 30, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@rinatkhaziev rinatkhaziev changed the title Fix potential prolonged rate-limiting: the rate-limit start time object didn't have an expiration which may prevent the ratelimit window reset Search: Fix potential prolonged rate-limiting Apr 30, 2024
@rinatkhaziev rinatkhaziev merged commit e224282 into develop Apr 30, 2024
40 checks passed
@rinatkhaziev rinatkhaziev deleted the update/search-index-ratelimiting branch April 30, 2024 19:11
sjinks added a commit that referenced this pull request May 7, 2024
* Exclude versioned Jetpacks from the `-built` version. (#5496)

* Don't delete jetpack-* recursively, do the root only. Supply --delete-excluded flag (#5498)

* CI: Preserve the .git folder when running --delete-excluded (#5500)

* Don't delete jetpack-* recursively, do the root only. Supply --delete-excluded flag

* Preserve .git

* Use full path

* Add debug to rsync output in Deploy workflow (#5501)

* Move backup one folder up to prevent the deletion during the rsync (#5502)

* Add debug to rsync output in Deploy workflow

* Move .git backup one level up

* Remove debug info from Deploy action (#5503)

* fix: `count_users()` returns an array (#5499)

* fix: `count_users()` returns an array

* test: fix tests

* CI: set minimum Parse.ly version to 3.5 (#5506)

* CI: update Parse.ly minimum version, fix some typos

* Remove debug-bar and debug-bar-cron as they're not the part of mu-plugins

* Add the ability to define a custom VIP integrations config directory (#5505)

* Search: Defaults Enterprise Search to ElasticPress 4.2.2 (#4607)

* Search: Remove exception list

* Search: Switch to new EP as default

* Sync changes down

* Update submodule change to latest one

* Update the PR template to include the new Changelog format (#5477)

* Remove limitation on "wp db size" (#5507)

* Revert "Remove limitation on "wp db size" (#5507)" (#5522)

This reverts commit 2efcab4.

Revert "Remove limitation on "wp db size" (#5507)"

This reverts commit 720c8b9.

* Search: Fix potential prolonged rate-limiting (#5524)

* Fix potential prolonged rate-limiting: the rate-limit start time object didn't have an expiration which may prevent the ratelimit window reset

* Fix context

* Apply settings on after_setup_theme to make sure customer code is included

* Only consider requests in current window when enabling ratelimiting

* Address feedback

* Apply SonarCloud suggestions

* remove ttl for indexing ratelimit start. More static over self.

---------

Co-authored-by: Rinat K <rinat@automattic.com>
Co-authored-by: Volodymyr Kolesnykov <volodymyr.kolesnykov@automattic.com>
Co-authored-by: Luis Henrique Mulinari <luis.henrique.mulinari@automattic.com>
Co-authored-by: Luiz Tiago Oliveira <luiztiago@gmail.com>
sjinks added a commit to Automattic/vip-enterprise-search that referenced this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants