Improve api response times by doing send_activity asynchronously

[phiresky]

Right now, all API responses (esp. create comment, like, etc) wait for send_activity to complete before returning to the caller. The API response though is already known before this, so it makes sense to do it afterwards, whenever the scheduler has time. send_activity can be very expensive because it has to collect all the interested parties, fetch more rows from db, insert a row into activities, parse a pem private key, and loop through all inboxes in order to schedule the outgoing tasks.

This does change the semantics a bit because errors in send_activity will not propagate to the HTTP caller. I'm not sure if this is required somewhere though. It's kind of not great anyways though, since if send_activity fails, it will be shown to the user as a failed action even though ::perform() succeded and returned a response (that is lost in the current version).

This does not decrease the total load, but it should increase responsiveness. It also requires one clone on responses, but most responses seem to be tiny.

[Nutomic]

Its a good idea. Unfortunately it breaks federation tests (api_tests/run-federation-test.sh) because those rely on the blocking behaviour to know when the federation actions are completed. Maybe the tokio::spawn could be used only in prod builds but not in debug to fix this.

On a side note, we also need to get rid of the SendActivity and Perform traits. The latter were only necessary for websocket. Then we can switch to normal actix-web handler methods which are much more flexible. But so far I couldnt think of a good way to do that, because api methods need to call apub send methods which are in an independent, parallel crate. Maybe some kind of message queue would work, although it seems wrong to use a message queue inside another.

Btw you can also move the webmention send to a background thread along with this.

[dessalines]

Why not just move the apub sends into the API functions? That's more explicit anyway.

[dessalines]

Its a good idea. Unfortunately it breaks federation tests (api_tests/run-federation-test.sh) because those rely on the blocking behaviour to know when the federation actions are completed. Maybe the tokio::spawn could be used only in prod builds but not in debug to fix this.

Could probably be fixed by a wait / sleep in the api test code.

[phiresky]

Why not just move the apub sends into the API functions? That's more explicit anyway.

Sorry, i don't understand exactly what you mean? You mean get rid of the SendActivity trait and move the content of send_activity into what's currently the Perform impls?

[dessalines]

You mean get rid of the SendActivity trait and move the content of send_activity into what's currently the Perform impls?

Yep. SendActivity::send_activity(&data, &res, &apub_data).await?; could be moved inside the HTTP calls. (especially since not all calls send activities anyway) Then we could get rid of both of those traits. That's for @Nutomic to decide how to restructure tho.

[phiresky]

ok well makes sense, but i think it should maybe be a different PR? seems somewhat orthogonal

[Nutomic]

Why not just move the apub sends into the API functions? That's more explicit anyway.

Apub and api crates are currently completely independent, meaning code in api crates cant access any code in the apub crate. This means that both crates can be compiled in parallel which speeds up the build considerably.

Could probably be fixed by a wait / sleep in the api test code.

I would hate to do this because its much more fragile and slower.

[phiresky]

I'm looking for a clean way to fix the fed tests but I'm not sure. should i:

• add a fixed delay of 2s to all the tests
• loop fetching the data with an increasing delay with a loop that waits until a max time to see if the data was federated
• add an API parameter to all post calls to wait for federation
• add a environment variable to wait for federation
• always wait for federation in debug mode

Edit: I'll solve it by adding a env var LEMMY_SYNCHRONOUS_FEDERATION that defaults to cfg!(debug_assertions).

[phiresky]

I've fixed the tests hopefully. No idea why CI just suddenly seems to cancel now.

[Nutomic]

Looks good, thanks!