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

Add multi-query version of subscribe messages, and dedup queries on the backend. #2278

Merged
merged 10 commits into from
Feb 21, 2025

Conversation

jsdt
Copy link
Contributor

@jsdt jsdt commented Feb 18, 2025

Description of Changes

This adds a new version of the websocket API for subscribing that takes multiple queries. This just has the backend changes.

Most of the changes are fairly mechanical, other than the changes in module_subscription_manager. The ClientInfo struct now tracks a set of query hashes for each subscription id, and it tracks a reference count for each query hash that it is subscribed to.

When adding and removing subscriptions, we use the reference counts to figure out which queries are actually being remove and added, and the SubscriptionManager functions return those queries, so that the code in module_subscription_manager.rs can figure out which queries to evaluate (and send to clients).

API and ABI breaking changes

This should just be adding new types, but once all the client SDKs are updated, we should probably make a breaking change to remove the old single versions.

Expected complexity level and risk

2.171

Testing

This has a few basic unit tests.

@jsdt jsdt marked this pull request as ready for review February 20, 2025 23:41
@jsdt jsdt requested a review from Centril as a code owner February 20, 2025 23:41
@jsdt jsdt assigned gefjon and unassigned jsdt Feb 21, 2025
@jsdt jsdt added this pull request to the merge queue Feb 21, 2025
Merged via the queue into master with commit 0f03e34 Feb 21, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants