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

Optimise file-lock retry options #890

Merged

Conversation

GiDW
Copy link
Contributor

@GiDW GiDW commented Jan 8, 2025

When processes with testcontainers are running in parallel they become very slow to start because of the file-lock retry strategy in testcontainers.

proper-filelock uses node-retry (repo) and exposes options to configure node-retry behavior. The current option is { retries: { forever: true } }. This leaves the rest on defaults (factor 2, minTimeout 1000, maxTimeout infinity).

Set file-lock retry options to retry faster and more frequently, avoiding timeouts that become too long.

{
  retries: {
    forever: true,
    factor: 1,
    minTimeout: 500,
    maxTimeout: 3000,
    randomize: true
  }
}

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 7f10747
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/677e8ab52cdd340008010b0d
😎 Deploy Preview https://deploy-preview-890--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@GiDW GiDW force-pushed the chore-adjust-file-lock-retry-options branch from d73cd5f to 7f10747 Compare January 8, 2025 14:24
@cristianrgreco cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Jan 8, 2025
@GiDW
Copy link
Contributor Author

GiDW commented Jan 9, 2025

@cristianrgreco Could you retry this run? 🙏
Test failed on installing dependencies due to a network error. Seems unrelated to the actual change.

@GiDW
Copy link
Contributor Author

GiDW commented Jan 10, 2025

@cristianrgreco For the failing test on modules/weaviate. This test fails for me locally on main as well. The only way I can make this test pass is by waiting after const container = await new WeaviateContainer().start();. Is this a known flaky test?

@cristianrgreco
Copy link
Collaborator

@GiDW I don't see any difference in test times from my side, so if for your case this is a big help them I'm happy to merge it. Adding some randomness makes sense IMO because otherwise the processes will check for locks at the same time. Happy to merge if you are.

@GiDW
Copy link
Contributor Author

GiDW commented Jan 10, 2025

@cristianrgreco Yes, this helps my case. If you are willing to merge it, please do 🙂

@cristianrgreco cristianrgreco changed the title chore(file-lock): Set retry options to retry faster and more frequently Optimise file-lock retry options Jan 10, 2025
@cristianrgreco cristianrgreco merged commit 37616d1 into testcontainers:main Jan 10, 2025
168 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor Backward compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants