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

Remove actix_rt & use standard tokio spawn #3158

Merged
merged 15 commits into from
Jun 26, 2023

Conversation

cetra3
Copy link
Contributor

@cetra3 cetra3 commented Jun 17, 2023

Sister PR to LemmyNet/activitypub-federation-rust#42. Shows how lemmy can use the standard tokio work stealing queue.

At the moment it's a draft as it's dependent on the git branch rather than crates.io

Cargo.toml Outdated Show resolved Hide resolved
@@ -252,6 +253,8 @@ pub struct EditSite {
pub federation_debug: Option<bool>,
/// The number of federation workers.
pub federation_worker_count: Option<i32>,
/// The number of federation retry workers.
pub federation_retry_worker_count: Option<i32>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that your PR was adding this. In fact I want to get rid of the worker count setting, not add another one...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've raised a PR to have this as an option: LemmyNet/activitypub-federation-rust#45 if you set both these values to 0 then there isn't any limit.

@cetra3
Copy link
Contributor Author

cetra3 commented Jun 21, 2023

@Nutomic for the diesel/schema changes, I did diesel migration create and then diesel migration run and the CLI did the rest (adjusting the imports etc..) if you know what cli version you're using, maybe I can downgrade to see if it's giving the same results, otherwise I can fudge it a little. The cli version I used is 2.1.0

Same with the TOML, I think the formatter I use isn't quite right. I'll adjust both accordingly.

(edit: I've adjusted both manually for now)

@Nutomic
Copy link
Member

Nutomic commented Jun 21, 2023

Regarding the worker count settings, how about default to unlimited? Lemmy doesnt use a lot of memory, and if it does reach memory limits then most likely due to postgres and local api requests.

@dessalines What do you think about this? I would also prefer to move worker counts to the config file, as its something for the hardware admin, not to be messed with by someone who is tasked to help with moderation.

@cetra3
Copy link
Contributor Author

cetra3 commented Jun 22, 2023

@Nutomic I've adjusted them to be unlimited (i.e, 0) by default & moved them to the Settings struct.

@Nutomic
Copy link
Member

Nutomic commented Jun 22, 2023

You need to run ./scripts/update_config_defaults.sh

@Nutomic Nutomic marked this pull request as ready for review June 23, 2023 10:09
@Nutomic Nutomic requested a review from dessalines as a code owner June 23, 2023 10:09
@Nutomic Nutomic merged commit d7da911 into LemmyNet:main Jun 26, 2023
@Nutomic
Copy link
Member

Nutomic commented Jun 26, 2023

Thank you!

Nutomic pushed a commit that referenced this pull request Jun 26, 2023
* Remove `actix_rt` & use standard tokio spawn

* Adjust rust log back down

* Format correctly

* Update cargo lock

* Add DB settings

* Change name and update to latest rev

* Clean up formatting changes

* Move `worker_count` and `worker_retry_count` to settings

* Update defaults

* Use `0.4.4` instead of git branch
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.

2 participants