|
123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101 |
- # Refactoring
-
- This is document describes the Design Goals and Refactoring Strategies of our ongoing efforts to improve the architecture, and with it the stability and performance of Plume.
-
- ## Current Issues
-
- Let's look at the current architecture's problems, and the attempts we've made at resolving them.
-
- ### PlumeRocket
-
- This data structure was [introduced by yours truly](https://github.com/Plume-org/Plume/pull/462) with the intent to please Clippy, reduce the number of arguments passed around (the main thing Clippy complained about).
- We also tried reduce the amount of bytes being passed around, by using references.
-
- At the time, this seemed like a good idea.
- Right now, it's the main source of our problems.
-
- This `struct` bundles `DbConn`, which makes it difficult migrate Plume to `async` Rocket:
-
- Passing around a giant `struct` as `ref` in `async` world, means that different owners are waiting for it to be `.await`ed, so they can access them.
- This makes working with such a `struct` very unwieldy, if not impossible sometimes.
-
- ### DbConn, Searcher and Post
-
- `DbConn` and `Searcher` are deeply bundled via `Post`.
- `Searcher` is called every time a `Post` is added/updated/deleted.
- It needs access to `DbConn` to fill the data that `Post` does not have.
-
- While removing one or the other from `PlumeRocket` is possible, complications still arise with `AsObject`:
-
- Posts's `AsObject` APIs are called every time a Post is added/updated/deleted.
- It builds on `PlumeRocket` as main `Context`, and so we'd have to touch every API if we split either `DbConn` or `Searcher` out of `PlumeRocket`
-
- ## Solution Attempts and their Problems
-
- in the past couple of weeks, we've made the following attepts to at least partially dissolve `PlumeRocket`
-
- - [plume-model: refactor Searcher to have its own DbPool](https://git.joinplu.me/Plume/Plume/pulls/809)
- - [WIP: Experiment: extract Searcher into an Actor](https://git.joinplu.me/Plume/Plume/pulls/807)
- - [extract DbConn from PlumeRocket](https://git.joinplu.me/Plume/Plume/pulls/805)
-
- As soon as we attempted to delete out one of the members from `PlumeRocket`, compiles would fail all over the place, meaning we'd have to touch almost every single function's *signature* that uses `PlumeRocket`.
- This then means we'd have to touch every single function that in turn use those functions!
- That is a lot of broken code, before we've even started refactoring.
-
- ## Strategy
-
- Despite ambitions to use an [Actor System (Riker)](https://riker.rs/), it is not magnitude of the ambitions, but the size of the steps we've taken.
- So, given past failures we devise a strategy.
- We want to replace each of the systems in `PlumeRocket` with an `Actor`, accessed by a single reference to the `ActorSystem`.
- This way we don't have to touch every single function's parameters that `PlumeRocket` flows thru, while the code is still in motion.
-
- ### Actors
-
- in [#807](https://git.joinplu.me/Plume/Plume/pulls/807), we've already made our first attempt at extracting `Searcher` into a Riker `Actor`.
- Here, just like in [#809](https://git.joinplu.me/Plume/Plume/pulls/809), we've already given `Searcher` its own `DbPool`.
-
- Why?
-
- ### DbPool instead of DbConn
-
- In our previous attempts at this code, we've realized that `DbPool`, being wrapped in an `Arc` is very cheap to `.clone()`.
- This means that every `Actor` that needs a `DbConn`, could get its own `DbPool`.
- We also realized that `DbPool` acts like an `Actor` in its own right:
-
- - `DbPool` has a `.get()` message
- - when that message is sent, it responds with a `DbConn`
- - if the pool is exhausted, it does not!
-
- Thus, we conclude:
- We want to `.clone()` a `DbPool` for every Actor that we extract from `PlumeRocket` that needs it.
-
- ### Workers
-
- In [#799](https://git.joinplu.me/Plume/Plume/issues/799#issuecomment-4402), we've identified the following `workers`:
-
- > Here is the list of things the worker is used for:
- >
- > - sending activities to other instances (just needs the activity and a list of recipients)
- > - rotating user keypair when they delete a post (after 10 minutes), which requires the DB
- > - fetching posts for a blog/users, either because it is new or because it has not been updated in 24 hours (depends on the DB too, and on the searcher)
-
- For the first type of worker, we'll have to make *repeated* network requests.
- There's a [Riker issue](https://github.com/riker-rs/riker/issues/130) asking how to best do that (with an answer.)
-
- The two workers that need access to the database, should get their own `DbPool`.
- Being part of the same `ActorSystem`, the last type of worker will be able to access `Searcher` thru messages.
-
- ### Request Path vs DbConn vs async
-
- For the Rocket Request path, we want to wrap `DbPool` in an `Actor` of its own:
-
- Then, in the `RequestGuard` we'd [ask](https://riker.rs/patterns/#ask) for a `DbConn`.
- This brings us on-par with Rocket's current [`#[database]`](https://github.com/SergioBenitez/Rocket/pull/1375) approach, that puts database connection access into `spawn_blocking()`.
- However, unlike `#[database]`, Riker's `ask()` responds with a Future, which would mean that in `async` functions we could simply `.await` it!
-
- ## The long road
-
- Once we've refactored the main systems in `PlumeRocket` into their own Actors, and once we're down to only the `ActorSystem` being the main component of `PlumeRocket`, we can finally shed the husk.
- That means that we *do* go and touch all the functions that take `PlumeRocket` and only give them access to the things they need.
-
- This is also a good chance to look at functions that are too long, or are doing to much, and mark them for refactoring.
|