Skip to content

Storage refactorings #902

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

Merged
merged 11 commits into from
Jul 19, 2020
Merged

Storage refactorings #902

merged 11 commits into from
Jul 19, 2020

Conversation

pietroalbini
Copy link
Member

This PR includes the first batch of refactorings to the storage subsystem to better support build tests. The most notable change is we're now using a single instance of Storage across the whole application, instead of creating a new one every time we need to use it.

This doesn't contain all the changes to the storage I need to make: namely I first need to move all the configuration to Config. This PR is starting to get big though and rebasing it is a pain, so I submitted it earlier.

This is best reviewed commit-by-commit.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Thanks so much! This should finally unblock the switch to tokio 🎉 and improve our response times by a lot now that we're no longer creating a new runtime for each request.

@jyn514
Copy link
Member

jyn514 commented Jul 19, 2020

Wait hold on, it looks like a runtime is still created each time start_storage_transaction is called. Can you make the runtime a field on the s3 backend instead and use OnceCell::Lazy so it's only created once?

@pietroalbini
Copy link
Member Author

Addressed all review comments.

@jyn514 jyn514 merged commit c9e59af into rust-lang:master Jul 19, 2020
@pietroalbini pietroalbini deleted the refactor-storage branch July 19, 2020 16:28
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