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(MemoryStorage): cache requests in RequestQueue #1899

Merged
merged 4 commits into from
May 9, 2023

Conversation

vladfrangu
Copy link
Member

@vladfrangu vladfrangu commented May 5, 2023

This is added because request queues tend to have a lot of activity on one entry, until the request is finished. This should ensure that no more invalid JSON errors are thrown, while also keeping the memory footprint low once a request entry is no longer accessed (+re-loading from fs if that happens)

@vladfrangu vladfrangu requested review from B4nan and barjin May 5, 2023 14:12
Copy link
Member

@mnmkng mnmkng left a comment

Choose a reason for hiding this comment

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

I think the sweep timeout would deserve some comments. Why it's needed, what it achieves.

@vladfrangu vladfrangu requested a review from mnmkng May 5, 2023 14:20
@vladfrangu
Copy link
Member Author

I think the sweep timeout would deserve some comments. Why it's needed, what it achieves.

I think what I just pushed should be enough, but let me know if it isn't

@B4nan B4nan changed the title fix(MemoryStorage): RequestQueue file system entries should have RAM … fix(MemoryStorage): RequestQueue file system entries should have RAM buffer May 5, 2023
@B4nan B4nan changed the title fix(MemoryStorage): RequestQueue file system entries should have RAM buffer fix(MemoryStorage): cache requests in RequestQueue May 9, 2023
@B4nan B4nan merged commit 063dcd1 into master May 9, 2023
@B4nan B4nan deleted the fix/rq-should-have-memory-buffer branch May 9, 2023 13:49
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.

3 participants