Skip to content

Fix subscription caching logic #515

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 2 commits into from
Dec 16, 2019
Merged

Conversation

smyrick
Copy link
Contributor

@smyrick smyrick commented Dec 13, 2019

📝 Description

Please merge this first: #514

Move the saving of subscriptions to a separate class so we can verify the logic with unit tests and simplify theApolloSubscriptionProtocolHandler. This also exposed a bug that we were not saving the operation subscriptions to be stopped properly. This is now covered by the unit tests

🔗 Related Issues

@smyrick smyrick added type: bug Something isn't working changes: patch Changes require a patch version labels Dec 13, 2019
@smyrick smyrick closed this Dec 13, 2019
@smyrick smyrick reopened this Dec 13, 2019
@gklijs
Copy link
Contributor

gklijs commented Dec 14, 2019

Just build the source branch, and then the test related to #499. Still the same result after about 120 subscriptions that have been started and stopped the websocket becomes unresponsive.

Shane Myrick added 2 commits December 16, 2019 12:48
Move the saving of subscriptions to a separate class so we can vefify the logic with unit tests and simplify the
ApolloSubscriptionProtocolHandler. This also exposed a bug that we were not saving the operation subscriptions to be stopped properly. This is now covered by the unit tests
Prioritize clearing memory over saving the small amount of operation we have to perform is the session stays open but there is no active operations
@dariuszkuc dariuszkuc merged commit 2259b9d into ExpediaGroup:master Dec 16, 2019
@smyrick smyrick deleted the perf-test-fix branch December 16, 2019 21:05
dariuszkuc pushed a commit to dariuszkuc/graphql-kotlin that referenced this pull request Aug 5, 2022
* Refactor subscription caching logic

Move the saving of subscriptions to a separate class so we can vefify the logic with unit tests and simplify the
ApolloSubscriptionProtocolHandler. This also exposed a bug that we were not saving the operation subscriptions to be stopped properly. This is now covered by the unit tests

* Clear cache of operations if no more left

Prioritize clearing memory over saving the small amount of operation we have to perform is the session stays open but there is no active operations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes: patch Changes require a patch version type: bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants