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

refactor(experimental): a transport that coalesces multiple subscriptions behind a single subscription #1623

Conversation

steveluscher
Copy link
Collaborator

Summary

This is a transport that, given two identical subscription requests, will issue the subscription request the first time only, and return a the cached iterator for that request each subsequent time, finally tearing the subscription down when the last consumer aborts.

This means that if you slotSubscribe from 10 places in your app, those will only ever produce one subscription over the RPC and will fan the notifications out to the callers.

Test Plan

cd packages/library
pnpm test:unit:browser
pnpm test:unit:node

@steveluscher
Copy link
Collaborator Author

Oh wow. This whole stack was braindead. You can't do this work at the connection level. You have to do it at the RPC level.

omgno

@steveluscher steveluscher marked this pull request as draft September 23, 2023 05:35
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from f541747 to 7b7d498 Compare September 23, 2023 07:58
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from a95a4f6 to b5ebb78 Compare September 23, 2023 07:58
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 7b7d498 to 9df9187 Compare September 29, 2023 01:02
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from b5ebb78 to f68a7a1 Compare September 29, 2023 01:02
@steveluscher
Copy link
Collaborator Author

OK! This is all fixed up, and I fixed a ton of bugs with the socket transport along the way.

This is ready to review!

@@ -55,66 +52,3 @@ describe('getSolanaRpcPayloadDeduplicationKey', () => {
);
});
});

describe('getSolanaRpcSubscriptionPayloadDeduplicationKey', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't end up needing this stuff.

try {
const iterator = iterable[Symbol.asyncIterator]();
while (true) {
const iteratorResult = await Promise.race([iterator.next(), abortPromise]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This whole thing is mega intense, but it all boils down to this.

  1. Create me a new iterable distinct from all the other ones that share this subscription.
  2. Make it so that each one is abortable separately.
  3. Every pass through the iterator, race the iterator and the abort promise; If the user aborts the subscription, it will return right away without waiting for the next message from the subscription.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, this was a help actually 😅

@steveluscher steveluscher marked this pull request as ready for review September 29, 2023 01:14
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 9df9187 to 61bc3ba Compare September 29, 2023 18:45
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from f68a7a1 to e2f215f Compare September 29, 2023 18:46
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 61bc3ba to ccfbd70 Compare September 29, 2023 21:37
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from e2f215f to 256e002 Compare September 29, 2023 21:37
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from ccfbd70 to 7c09215 Compare September 29, 2023 22:45
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from 256e002 to c80a372 Compare September 29, 2023 22:45
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 7c09215 to b072aae Compare September 29, 2023 22:51
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from c80a372 to 0fc0266 Compare September 29, 2023 22:51
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from b072aae to ec57cab Compare September 29, 2023 23:13
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from 0fc0266 to 39de3f2 Compare September 29, 2023 23:13
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from ec57cab to 38ab0c7 Compare September 29, 2023 23:27
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from 39de3f2 to c913f59 Compare September 29, 2023 23:27
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 38ab0c7 to 31a626e Compare September 29, 2023 23:34
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from c913f59 to 6ccb70f Compare September 29, 2023 23:34
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation branch from 31a626e to d6d8690 Compare September 30, 2023 00:22
@steveluscher steveluscher changed the base branch from 09-22-refactor_experimental_extract_the_request_cache_logic_into_a_base_implementation to 09-28-refactor_experimental_drop_messages_the_moment_a_socket_connection_is_aborted September 30, 2023 00:22
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from 6ccb70f to e2bdaf6 Compare September 30, 2023 00:22
@steveluscher steveluscher force-pushed the 09-28-refactor_experimental_drop_messages_the_moment_a_socket_connection_is_aborted branch from 2e69790 to 8698f76 Compare September 30, 2023 00:56
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from e2bdaf6 to 64f33e9 Compare September 30, 2023 00:56
@steveluscher steveluscher force-pushed the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch from 64f33e9 to bd8d5b4 Compare September 30, 2023 00:58
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

brain

try {
const iterator = iterable[Symbol.asyncIterator]();
while (true) {
const iteratorResult = await Promise.race([iterator.next(), abortPromise]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, this was a help actually 😅

@steveluscher
Copy link
Collaborator Author

steveluscher commented Oct 3, 2023

Merge Activity

Base automatically changed from 09-28-refactor_experimental_drop_messages_the_moment_a_socket_connection_is_aborted to master October 3, 2023 16:14
@steveluscher steveluscher merged commit 556b856 into master Oct 3, 2023
2 checks passed
@steveluscher steveluscher deleted the 09-22-refactor_experimental_a_transport_that_coalesces_multiple_subscriptions_behind_a_single_subscription branch October 3, 2023 16:14
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

🎉 This PR is included in version 1.78.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants