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: refactor persistent term usage to behaviour #344

Merged
merged 5 commits into from
Nov 29, 2024

Conversation

Ziinc
Copy link
Contributor

@Ziinc Ziinc commented Nov 27, 2024

This PR adds in an ETS configuration storage option, for #342 .

Our use case has many dynamically-created Broadway servers (each with different non-static names). This results in global GC getting triggered. Although fb69a7a has some improvements, it is insufficient for our situation.

If we were to continue using persistent term using the above mentioned change, it will result in large buildup in the hash table over time. As time to update/store the term is proportional to hash table size, resulting in degrading storage speeds over time. In our case, the memory buildup over time may also be non-negligible.

However, without the above change (where the persistent term is not cleaned up), it will result in the GC getting triggered each time the server goes down (due to our autoscaling logic).

Switching over to :ets based storage would avoid the GC-ing issues.

@josevalim
Copy link
Member

Thank you! Although I don't think we need modules and behaviours. A simple :config_storate with either :persistent_term or :ets is enough. :)

@Ziinc
Copy link
Contributor Author

Ziinc commented Nov 28, 2024

👍 i've switched it to atom-based config, and moved the docs out of each module. Also removed the module docs for those to keep them internal. It is technically possible for users to pass in their own custom module as an option, just leaving that option open but undocumented.

@josevalim josevalim merged commit 1151db2 into dashbitco:main Nov 29, 2024
2 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@starbelly
Copy link

Hey @josevalim @wojtekmach and other maintainers. Could a release be cut ? Really need this feature per dynamic starts and stops in our system and the just the right configuration to cause literal area gc pressure 😄

@josevalim
Copy link
Member

v1.2.0 is out.

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

Successfully merging this pull request may close these issues.

3 participants