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

LazySidekiq strategy #827

Merged
merged 17 commits into from
Mar 4, 2022
Merged

LazySidekiq strategy #827

merged 17 commits into from
Mar 4, 2022

Conversation

sl4vr
Copy link
Contributor

@sl4vr sl4vr commented Dec 10, 2021

This PR implements proof of concept for new Chewy strategy. The strategy allows to avoid synchronous evaluation of index queries. Instead all the callbacks will be saved in class attribute and model type and id will be stashed into strategy instance. When strategy block ends it will enqueue IndicesUpdateWorker Sidekiq job with serialized stash as argument. The job will load records and run all the callbacks within sidekiq strategy block, which in turn will enqueue Sidekiq jobs updating indices.
Since we cannot schedule IndicesUpdateWorker job for destroyed records, instead all the callbacks will be run synchronously and resulted index types and ids will be stashed into strategy instance and run on the lazy_sidekiq strategy block end like if they would be run on the end of sidekiq strategy block.

Some concerns to consider:

  • One cannot rely on state within update_index blocks because record will be reinstantiated on the time when callbacks will be run. We can however serialize changes and previous_changes to Sidekiq jobs, or even specify what to serialize.

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the changelog if the new code introduces user-observable changes. See changelog entry format for details.

@sl4vr sl4vr requested review from riffraff and paneq December 10, 2021 16:14
@paneq paneq removed their request for review December 13, 2021 08:00
@sl4vr sl4vr requested a review from rabotyaga December 13, 2021 10:54
@sl4vr sl4vr force-pushed the lazy_sidekiq_strategy branch 3 times, most recently from a77020e to 54afc7b Compare December 15, 2021 08:33
@sl4vr sl4vr marked this pull request as ready for review January 3, 2022 09:37
@sl4vr sl4vr added the WIP label Jan 3, 2022
Take into account  different ruby versions.
@sl4vr sl4vr removed the WIP label Feb 10, 2022
@sl4vr sl4vr requested a review from mrzasa February 10, 2022 14:26
README.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
barthez and others added 2 commits March 4, 2022 14:57
Fix typo

Co-authored-by: Ivan Rabotyaga <ivan.rabotyaga@toptal.com>
@barthez barthez merged commit d8c7bf7 into master Mar 4, 2022
@barthez barthez deleted the lazy_sidekiq_strategy branch March 4, 2022 14:58
cyucelen pushed a commit to cyucelen/chewy that referenced this pull request Jan 28, 2023
- Implement LazySidekiq strategy
- Implement AtomicNoRefresh strategy
- Add `no_refresh` chain call to `update_index` matcher

Co-authored-by: Slava Mefodin <slava.mefodin@toptal.com>
Co-authored-by: Bartek Bułat <bartek@toptal.com>
Co-authored-by: Ivan Rabotyaga <ivan.rabotyaga@toptal.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants