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

fix: key value stores emitting an error when multiple write promises ran in parallel #1460

Merged

Conversation

vladfrangu
Copy link
Member

@vladfrangu vladfrangu commented Aug 12, 2022

Should make #1435 less probable (or outright fix it), however, as I have been unable to consistently replicate the issue mentioned (i can replicate it ~ once every 10-15 tries), this is still in the air

Fixes #1435

@vladfrangu vladfrangu requested a review from B4nan August 12, 2022 11:47
@B4nan
Copy link
Member

B4nan commented Aug 12, 2022

@vladfrangu
Copy link
Member Author

Well, the bulk of the issues happen because we purge the old data in the background, if we'd await it I doubt we'd have any issues, but well... then it could take seconds of waiting and not "doing anything"

@B4nan
Copy link
Member

B4nan commented Aug 12, 2022

Seconds would be fine, but it could took half a minute easily, that dx sucks without any progress bar.

But I would say locking can help. First lock the folder, then move it and schedule removal. If locking it fails (second promise), you dont even try to do anything.

@vladfrangu vladfrangu changed the title fix: possibly lower the error rate for memory storage purging fix: key value stores emitting an error when multiple write promises ran in parallel Aug 12, 2022
@B4nan
Copy link
Member

B4nan commented Aug 12, 2022

Needs manual rebase, sorry, forgot to merge it before bumping the version.

@vladfrangu
Copy link
Member Author

Will do, sec

I tried this locally and haven't encountered any errors over tens of retries... I guess the solution
is sometimes going sync instead of async 😢
@vladfrangu vladfrangu force-pushed the fix/possibly-lower-the-rate-of-errors-for-memory-storage-cleanup branch from f2c1a30 to 3a09366 Compare August 12, 2022 15:06
@B4nan B4nan merged commit f201cca into master Aug 12, 2022
@B4nan B4nan deleted the fix/possibly-lower-the-rate-of-errors-for-memory-storage-cleanup branch August 12, 2022 15:23
Shan5152
Shan5152 approved these changes Aug 28, 2022
@apify apify deleted a comment from Shan5152 Aug 28, 2022
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.

Racing multiple storage promises fail to purge the default storages
3 participants