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

"fresh" garbage stays in the release buffer long after going stale; bad for LiveState #4834

Open
subtleGradient opened this issue Oct 25, 2024 · 0 comments

Comments

@subtleGradient
Copy link

subtleGradient commented Oct 25, 2024

as of 18.1.0
if the release buffer is >0
when something's ref count hits zero, it'll be added to the release buffer if it isn't stale yet.
if it's stale already, it'll be garbage collected immediately
but if it's fresh when disposed (based on queryCacheExpirationTime), then it'll get put into the release buffer
but then it may hang around in the release buffer for a very long time

the default release buffer size is 10,
so if a LiveState value is disposed while still fresh,
then it won't get unsubscribed until another 10 more FRESH things are disposed (pushing it off the end of the release buffer pier).

Seems like there should be some documentation or something for LiveState subscriptions so that devs have more control over when those subscriptions are cleaned up.

RELATED: #4830 #4831 #4832


Gemini and I would like to add...

Potentially Indefinite Retention of LiveState Subscriptions in Release Buffer (RelayModernStore)

Relay Version: 18.1.0

Issue:

Currently, LiveState subscriptions may be retained indefinitely in the release buffer of RelayModernStore, leading to unnecessary resource consumption and potential memory leaks.

Description:

As of Relay 18.1.0, when a query's retain count reaches zero, it's added to the release buffer if it's not stale (based on queryCacheExpirationTime). However, the release buffer does not periodically check for staleness of entries within it. This means that if a LiveState value is disposed while still "fresh" (not stale), its associated subscription will remain active within the store's _resolverCache until:

  • _gcReleaseBufferSize more fresh queries are disposed, pushing the original query off the end of the release buffer.
  • The query is explicitly retained again.

Example:

Consider a LiveState value with a subscription to an external data source. This value is used by a component that then unmounts, causing the query's retain count to reach zero. If the query is still considered fresh based on queryCacheExpirationTime (default is indefinite), the following occurs:

  1. The query ID is added to the release buffer.
  2. The LiveState subscription remains active.
  3. The subscription will not be unsubscribed until the release buffer reaches capacity (_gcReleaseBufferSize more fresh queries are disposed) or the query is retained again.

This behavior is problematic because:

  • Indefinite Subscription: The LiveState subscription remains active indefinitely, potentially consuming resources associated with the external data source (e.g., open connections, event listeners).
  • Unpredictable Unsubscription: The cleanup of the LiveState subscription is tied to the release buffer capacity and the staleness of other unrelated queries. This makes it difficult to predict or control when the subscription will be unsubscribed.

Code Snippets:

  • RelayModernStore.retain.dispose():
    if (rootEntry.refCount === 0) {
        // ...
        if (rootEntryIsStale) { 
            _this2._roots.delete(id);
            _this2.scheduleGC();
        } else {
            // Add to release buffer if NOT stale
            _this2._releaseBuffer.push(id); 
            // ... (eviction logic if buffer is full)
        }
    }
  • RelayModernStore._collect(): (Note: This function is responsible for garbage collection, but does not interact with the release buffer for staleness checks)

Proposed Solutions:

  • Periodic Staleness Check: Implement a mechanism within RelayModernStore to periodically check the staleness of queries in the release buffer and evict expired ones, even if the buffer is not full. This would ensure that LiveState subscriptions are unsubscribed promptly when their data becomes stale.
  • Explicit LiveState Unsubscription: Provide a public API (e.g., environment.unsubscribeLiveState(dataID)) to allow developers to explicitly unsubscribe from LiveState values. This would give developers fine-grained control over subscription lifecycles.
  • Documentation: Clearly document the current behavior of LiveState subscriptions and their interaction with the release buffer, including potential implications and mitigation strategies.

Additional Considerations:

  • Performance: The proposed solutions should consider the performance impact of additional checks or operations within the store.
  • API Design: The design of any new API for LiveState management should be consistent with Relay's principles and existing APIs.

By addressing this issue, Relay can better manage resources associated with LiveState subscriptions, prevent potential memory leaks, and provide developers with more control over the lifetime of their subscriptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant