-
Notifications
You must be signed in to change notification settings - Fork 46
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
fix: prune subscriptions trie when topics are unsubscribed #1671
base: main
Are you sure you want to change the base?
Conversation
Binary incompatibility detected for commit e17d003. com.aws.greengrass.util.NucleusPaths is binary incompatible and is source incompatible because of CONSTRUCTOR_REMOVED Produced by binaryCompatability.py |
Unit Tests Coverage Report
Minimum allowed coverage is Generated by 🐒 cobertura-action against e17d003 |
Integration Tests Coverage Report
Minimum allowed coverage is Generated by 🐒 cobertura-action against e17d003 |
Issue #, if available:
#1670
Description of changes:
Background
PubSubIPCEventStreamAgent
handles all the IPC local pubsub requests. It implements the handlers forSubscribeToTopicRequest
andPublishToTopicRequest
where the topics are stored in aSubscriptionTrie
trie.When a topic is subscribed to, it is added to the
SubscriptionTrie
trie where the topic levels as represented as nodes of the trie. When you subscribe to a topic with a handler, the handler is registered as a listener of that topic where the last topic level node of that topic increments its listeners count by 1. So, for a valid topic in a trie, the number of subscriptions count at the last topic level of that topic is greater than or equal to one.Eg. When topics like a/b, a/b/c, a/# and d/+ are registered with a callback, the trie representation is as follows where the nodes
b
,c
,#
and+
maintain the number of subscription handlers held by a topic that ends at their topic level.Problem
When an IPC subscription for a topic is closed, it is expected that topic references are removed from the trie if there are no others listeners for the same topic. But, currently, we only remove the registered subscription handler from the list of handlers maintained at that topic level, leaving behind all the nodes as is. That means, once a topic is subscribed to, it is never removed from the heap memory even when it is closed/unsubscribed to and no longer used.
Changes
When a topic is unsubscribed to, we clear up the references from the
SubscriptionTrie
, which manages topics along with their listeners for matching and filtering.If a topic node has no children or no callbacks registered, it means that the path to that topic node is not a complete topic or a prefix of other topic(s). So, it can be removed.
We modify the trie in the following way when a topic is unsubscribed to or a subscription is closed:
Why is this change necessary:
#1670
The heap memory keeps growing by retaining the object references even when they're no longer needed. If the heap size continues to grows due to
SubscribeToTopicRequest
object, the IPC event loop thread which handles IPC requests is killed due to out of memory error leaving the JVM in a bad state.How was this change tested:
Any additional information or context required to review the change:
Documentation Checklist:
Compatibility Checklist:
any deprecated method or type.
Refer to Compatibility Guidelines for more information.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.