Refactor the JSON-RPC cache and handling of legacy API subscriptions #854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the way the subscriptions (all heads, new heads, finalized heads, runtime version, and storage) of the legacy API are handled.
A task dedicated to this is now spawned in the background, and all these subscriptions now send a message to said task.
This task is also subscribed against the runtime service and propagates the blocks that the runtime service notifies to the JSON-RPC client.
This means that notifications are now sent in a precise order. In particular, a runtime version change is now sent immediately after a best block change when relevant. While this is not something the JSON-RPC client can rely upon, it is more clean to do this nonetheless.
When it comes to storage subscriptions, a side change is that all the keys of all the storage requests are now merged together into one network request. This change doesn't make things particularly faster, but is necessary before #779 can land.
This new task now completely owns the content of the
Cache(previously a field inBackground).In order to access the cache, one can send a message to the task rather than lock a mutex. In general I'd like to avoid mutexes in order to be able to guarantee a lack of deadlocks.
While the diff is pretty large, every commit is straight forward, and I'm very confident in the fact that the code works at least as well as before this PR.