Skip to content

fix: concurrent issue when bulk-generate with db lock retry and change concurrent to for loop for db lock safe thread. #282

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

Closed
wants to merge 1 commit into from

Conversation

mrdhira
Copy link

@mrdhira mrdhira commented Jun 22, 2025

Addresses #268

What does it do?

  • Add db lock retry for concurrent safe
  • Change Promise.all to for loop for db lock safe thread.

Why is it needed?

There is issue if you have multiple locale and many data that need generated (mine problem when there is around 5000 data) url alias, there will be deadlock issue because strapi manage data using 2 row of data with same id for manage draft and published state

How to test it?

Generate with in one content types with multiple locales active and set prefix for each locales. and then generate it via webtools or API.

Related issue(s)/PR(s)

#268

…e concurrent to for loop for db lock safe thread.
@Copilot Copilot AI review requested due to automatic review settings June 22, 2025 17:31
Copy link

changeset-bot bot commented Jun 22, 2025

⚠️ No Changeset found

Latest commit: 659bab8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a retry helper to handle database deadlocks and converts parallel batch operations into sequential loops to ensure DB lock safety when bulk-generating URL aliases.

  • Adds withDbLockRetry to automatically retry on common deadlock or lock-timeout errors.
  • Replaces Promise.all concurrency with for…of loops, wrapping each DB operation in the retry helper.
  • Refactors language initialization flow for clarity.
Comments suppressed due to low confidence (2)

packages/core/server/services/bulk-generate.ts:18

  • [nitpick] Add JSDoc comments describing the purpose, parameters (fn, retries, delay), and return value of withDbLockRetry to improve readability and maintainability.
async function withDbLockRetry<T>(

packages/core/server/services/bulk-generate.ts:18

  • [nitpick] Consider adding unit tests that simulate deadlock or lock-timeout errors to verify that withDbLockRetry correctly retries and eventually throws after max attempts.
async function withDbLockRetry<T>(

Copy link
Member

@boazpoolman boazpoolman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mrdhira,

Thank you for submitting this pull request!
I think you're touching an important aspect of the plugin here, which is:

What to do about running large database queries.

There is no issue reported for this matter just yet, but I'm conscious about this limitation. Especially in the bulk generation, but also in upcoming features (like #280) we need a way to run these large tasks without crashing our database or Strapi application as a whole.

Just so we're on the same page here, this is not the same as #268. That was just a bug in the code, which is solved by #276.

That being said, I think we need to make a good plan before we start tackling this problem, as it's not an easy one to solve. While I appreciate your efforts, I don't think implementing a delay when we're facing deadlocks is the way to solve this. We should try to prevent those deadlocks from happening in the first place.

@boazpoolman
Copy link
Member

I've created an issue to host our discussions. If you have input on how we can solve this problem, please post them here:

#284

@boazpoolman
Copy link
Member

I'm going to close this PR as we're still in the RFC phase of solving this problem.

If you (or anybody reading this) has any thoughts on the matter, please put your comments here #284.

@boazpoolman boazpoolman closed this Jul 3, 2025
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.

2 participants