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

feat(notifier): subscription kit history limit #5403

Closed
wants to merge 4 commits into from

Conversation

michaelfig
Copy link
Member

refs: #5346

Description

Implements "prefix lossy subscriptions" via makeSubscriptionKit(historyLimit) parameter (default=Infinity).

Security Considerations

Improves memory profile, given cooperating subscribers (an old and slow subscription may still hold references to the entire history, but new ones won't).

Documentation Considerations

Documented in README.md and backwards compatible.

Testing Considerations

The test suite tests several important values of historyLimit.

@michaelfig michaelfig requested a review from erights May 20, 2022 01:24
@michaelfig michaelfig self-assigned this May 20, 2022
@erights
Copy link
Member

erights commented May 20, 2022

@michaelfig be aware that @dtribble also has a sketch of a prefix lossy subscriber at https://gist.github.com/dtribble/4e7cc65fb0fa8aab5a4069e80d915454 , that we've been meaning to talk about. I think it is very close. Your approach seems very different. We should understand these differences before proceeding with an approach.

@michaelfig
Copy link
Member Author

Withdrawing in favour of the new-and-improved subscriptions/notifiers @erights is working on.

@michaelfig michaelfig closed this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants