mastodon / flodgatt

A blazingly fast drop-in replacement for the Mastodon streaming API server
GNU Affero General Public License v3.0
87 stars 7 forks source link

[Architecture decision] change from actix to warp #2

Closed codesections closed 5 years ago

codesections commented 5 years ago

I've been giving a lot of thought to the project's overall architecture over the past week and am strongly considering changing from the Actix-web framework to the Warp framework. There are three main reasons I think this switch makes sense:

  1. Actix uses an actor model for managing concurrency (similar to Erlang or Elixir). That's a very powerful model, but it doesn't mix well with other models for concurrency—it wants everything to be an actor. And, since we're using Redis, we'll need to interact with Redis's streams. Given that, it seems like a framework that plays a bit better with streams would be helpful.
  2. Related to the above, the change to Warp will likely make the codebase more accessible. The actor model has many fans, but I think it's fair to say that it's less widely used than other models for managing concurrency. Further, Warp's syntax tends to be less verbose and (IMO) more readable for people who don't code in Rust as often, which should help make the codebase more maintainable. (See proof of concept below.)
  3. The Warp documentation is more complete. Actix has the start of some very good documentation, but many of the crucial pages—especially those relating to the details we'd need to manage to merge the concurrency styles—are very much a work in progress (as in, their current text is just "[WIP]")

There are a few downsides to switching frameworks. Most obviously, it means giving up some of the code that's already been written. But, if we're going to switch, it's hard to find a better time than when the codebase only has 229 lines of code. Second, Warp isn't as old or as widely used as Actix (for example, it has 862 GitHub stars compared to 3,418 for Actix-web). On the other hand, Warp is built on top of Hyper (a lower-level Rust HTTP framework with over 4,400 GitHub stars) and is maintained by Sean McArthur—the same developer who built Hyper. So I'm not to worried about Warp's quality even if it is a bit newer.

Based on the above, I think it makes sense to change to Warp. I put together a proof-of-concept that shows how a Server Sent Events stream from Redis could work in Warp; as you can see, it's fairly concise:

use futures::{Async, Poll};
use redis;
use std::time::Duration;
use warp::{path, Filter, Stream};

struct OutputStream {
    con: redis::Connection,
}

impl OutputStream {
    fn new() -> Self {
        let client = redis::Client::open("redis://127.0.0.1:6379").unwrap();
        let con = client.get_connection().unwrap();
        OutputStream { con }
    }
}

impl Stream for OutputStream {
    type Item = String;

    type Error = std::io::Error;

    fn poll(&mut self) -> Poll<Option<String>, std::io::Error> {
        let mut pubsub = self.con.as_pubsub();
        pubsub.subscribe("timeline:1").unwrap();
        pubsub.set_read_timeout(Some(Duration::new(1, 0))).unwrap();
        match pubsub.get_message() {
            Err(_) => Ok(Async::NotReady),
            Ok(msg) => Ok(Async::Ready(Some(msg.get_payload().unwrap()))),
        }
    }
}

fn main() {
    let routes = warp::path!("api" / "v1" / "streaming" / "public")
        .and(warp::sse())
        .map(|sse: warp::sse::Sse| {
            let stream = OutputStream::new().inspect(|_| {});
            sse.reply(warp::sse::keep(
                stream.map(|s| warp::sse::data(s)),
                Some(Duration::new(1, 0)),
            ))
        });

    warp::serve(routes).run(([127, 0, 0, 1], 3030));
}

Unless anyone raises strong objections, I plan to start converting the code to use Warp next week.

sphinxc0re commented 5 years ago

I don't see how this code is more understandable than what is already in the codebase, which is:

App::with_state(app_state.clone())
        .prefix("/api/v1")
        .resource("/streaming", |r| r.with(ws::index))
        .resource("/streaming/health", |r| {
            r.middleware(cors_middleware());
            r.get().f(|_| HttpResponse::Ok())
        })
        .resource("/streaming/user", |r| {
            r.middleware(cors_middleware());
            r.get().with(http::user::index)
        })
     ...
})

This seems pretty straight-forward.

Other than that, Actix has many ways of handling streams: https://docs.rs/actix/0.7.10/actix/prelude/trait.StreamHandler.html#method.add_stream.

The whole point of an actor system is to have everything be an actor to separate concerns in a really strict way and to avoid state globalization. Local state is much more maintainable. This also means that actors are very pluggable. You can write and actor once and reuse it all over the place without that much hassle.

I consider the current framework a really good and sustainable design decision. Actix is a highly developed project.

Also, is has been said, that the warp project and tower-web are going to merge, which would make writing code right now pointless because of the resulting API change.

codesections commented 5 years ago

Thanks for the reply and the pushback. I agree that Actix-web is a highly developed—and great!—project and I'll give it some more thought. A few replies to your specific comments are below:

I don't see how [the Warp] code is more understandable than what is already in the codebase

I agree that Actix-web and Warp have equally readable code for the main request-response cycle—that's not the part I was talking about. I was talking about the code that's required to actually communicate with the Redis stream. So, in the current codebase, not the bit from main.rs that you quoted, but the (currently unimplemented) bit from the endpoints (e.g. `api/http/public.rs).

For that logic, with Actix-web we need to set up the Redis stream, convert it to an Actix actor, and then send messages from that actor to the relevant Actix-web actor. With Warp, we just set up the Redis stream and use it directly. This saves a bit of conceptual overhead/boilerplate code (though of course it loses us the benefits of the actor system).

The whole point of an actor system is to have everything be an actor to separate concerns in a really strict way and to avoid state globalization. Local state is much more maintainable. This also means that actors are very pluggable. You can write and actor once and reuse it all over the place without that much hassle.

Yeah, I totally get that. There are a ton of advantages to an actor system and I'm a fan of the concept in general. If I were building a larger app with a lot of state, I'd love to have the advantages of an actor system. Here, though, we're not dealing with much state at all: we have a few (inherently global) config values like the Redis port, but that's pretty much it. So, as great as the actor system is in general, I'm not sure it's a good fit for this particular project.

Other than that, Actix has many ways of handling streams: https://docs.rs/actix/0.7.10/actix/prelude/trait.StreamHandler.html#method.add_stream.

Yep, definitely! I was actually thinking of methods like add_stream when I mentioned that the Actix docs are incomplete. A couple of thoughts about add_stream and similar methods:

the warp project and tower-web are going to merge, which would make writing code right now pointless because of the resulting API change.

Warp and Tower-web do plan to merge to share some backend functionality. However, they are going to support both APIs as "flavors" as discussed in this issue. So there shouldn't be any issue with backwards compatibility—combining the projects is just about avoiding code duplication for underlying glue code since they're both based on Hyper.

(One additional point that might be relevant: Warp has built-in support for Server Sent Events, with it's sse module. Actix doesn't have anything built in for SSEs, so we'll need to implement something ourselves. I'm sure that won't be too difficult, but it's probably not trivial either, judging by the fact that there's been a request for an SSE example since July in the actix-examples repo and no one has produced on yet. Just one more factor to consider.)

I consider the current framework a really good and sustainable design decision.

Even with all of the above, I really value your opinion/perspective in this, and I'll definitely give it more thought. At this point, I think I'll spend a bit more time with Actix-web (and it's API docs) and then see what makes sense from there.

brokenthorn commented 4 years ago

So it's been a while and this thread is really interesting. Have any follow-up experiences to share? Maybe not here but maybe a blog post or Reddit?

codesections commented 4 years ago

So it's been a while and this thread is really interesting. Have any follow-up experiences to share? Maybe not here but maybe a blog post or Reddit?

Yeah, I plan to have a full retrospective sometime after Flodgatt hits 1.0.0. The short version is that I've really enjoyed using Warp, but the speed of development/breaking changes has been somewhat exhausting. (Though a lot of that is because I'm using pre-release github versions of Warp to incorporate fixes and features I've needed). I suspect that this will get a lot better once Warp hits 0.2.x though.