[Refactoring] send messages to dedicated actor instead of directly accessing objects
#799
Open
4 jaren geleden geopend door igalic
·
7 opmerkingen
Geen Branch/Tag gespecificeerd
paginate-search-init
main
s3
fix-delete-user
timeline-cli
blog-title
signature
remove-dup-images
ldap-non-anon
drone-ci
DearRude/force-lang
igalic/go/async-all-mut
go/async
floreal/translations-update
missing-docs
RAOF/fix-arm64-build
epsilon-phase/authorized-fetch
upgrade
improve-the-editor-once-again
igalic/feat/custom-fairing-domains
feature/ldap
test/dotenv_error
fix-mobile-margin
0.7.2
0.7.0
0.2.0-alpha-1
0.3.0-alpha-2
0.4.0-alpha-4
0.5.0
0.6.0
0.7.1
Labels
Verwijder labels
Related to the REST API
Code running on the server
Stuff related to Federation
Related to the front-end
Translations, and related code
More about project management or code than the project itself
The building, or installation process of Plume
Something isn't working
We need to talk
New feature or request
This is a new feature
Compatibility with different browsers, readers and OS
Related to an external package that Plume uses
UI/UX related issues and PRs
Good for newcomers
Extra attention is needed
Issues affecting only mobile UX
How elements're rendered out for the end user
Something else needs to be fixed first
This issue or pull request already exists
This PR is not complete yet
Issues concern a limited number of instances
This doesn't seem right
Need to be discussed by the community (on Loomio)
This PR is ready to be reviewed
Proposed ideas worth considering
This is issue has been created after a vote on Loomio
This will not be worked on
Labels toepassen
A: API
Related to the REST API
A: Backend
Code running on the server
A: Federation
Stuff related to Federation
A: Front-End
Related to the front-end
A: I18N
Translations, and related code
A: Meta
More about project management or code than the project itself
A: Security
Build
The building, or installation process of Plume
C: Bug
Something isn't working
C: Discussion
We need to talk
C: Enhancement
New feature or request
C: Feature
This is a new feature
Compatibility
Compatibility with different browsers, readers and OS
Dependency
Related to an external package that Plume uses
Design
UI/UX related issues and PRs
Documentation
Good first issue
Good for newcomers
Help welcome
Extra attention is needed
Mobile
Issues affecting only mobile UX
Rendering
How elements're rendered out for the end user
S: Blocked
Something else needs to be fixed first
S: Duplicate
This issue or pull request already exists
S: Incomplete
This PR is not complete yet
S: Instance specific
Issues concern a limited number of instances
S: Invalid
This doesn't seem right
S: Needs Voting/Discussion
Need to be discussed by the community (on Loomio)
S: Ready for review
This PR is ready to be reviewed
Suggestion
Proposed ideas worth considering
S: Voted on Loomio
This is issue has been created after a vote on Loomio
S: Wontfix
This will not be worked on
Geen label
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
Mijlpaal
Stel mijlpaal in
Verwijder mijlpaal
Geen items
Geen mijlpaal
Toegewezen aan
Gebruikers toewijzen
Verwijder toegewezen aan
Niet toegewezen
2 deelnemers
Notificaties
Vervaldatum
De deadline is ongeldig of buiten bereik. Gebruik het formaat 'jjjj-mm-dd'.
Geen vervaldatum ingesteld.
Afhankelijkheden
Geen afhankelijkheden ingesteld.
Referentie: Plume/Plume#799
Verwijs in nieuw issue
Er is nog geen inhoud.
Verwijder branch '%!s(<nil>)'
Het verwijderen van een branch is permanent. Het KAN NIET ongedaan gemaakt worden. Wil je toch doorgaan?
Nee
Ja
Instead of passing around references to directly access
worker
, andsearcher
, we could instead send messages to an actor!This would also help split responsibility of these functions, as they would no longer invoke these actions themselves, only sending messages to invoke them.
This would allow to remove searcher and worker from our PlumeRocket. Together with #797, this could help completely dissolve the struct.
How?
We could use an actor system, like riker.
Using multiple messages we could send different messages to different actors that then do what needs to be done
Honestly, it makes me wonder if actix wouldn't be more appropriate for our use case… we would get actors and a web framework out of the box. I think we already tried to migrate once and it didn't went very far, right? What was blocking?
But I'm okay with refactoring our code with Riker and keeping Rocket too.
the main reasons i abandoned the actix experiment were:
(the other one isn't even public yet, so that's actually quite nice :P)
I've created a basic example with riker and multi-type messages for how to replace Searcher: https://gist.github.com/igalic/16ec36a4c9277b3b46f6bea032226189
in a similar fashion, we can replace Worker, which currently mostly sents broadcast messages to all subscribers.
here's what I've found out in an experiment attempting to extract
Searcher
into anActor
:Searcher
is intimately bundled withPost
Searcher
is also intimately bundled withDbConn
, becausePost
doesn't actually have all the infoSearcher
needsgiven this asymmetry, i thought it's best to only pass
Post.id
but we still have to give it access to a
DbConn
perhaps on creation of a
Searcher
Actor
, we could give it its ownDbConn
?Some observations, which lead to the confidence with which we've started on #807:
DbPool
is anArc
, it can be easily shared via.clone()
.This would allow us to create a wrapper object around
Searcher
which can then have its ownDbConn
instead of passing one from a — hopefully — short-livedRequest
, potentially infinitely blocking on it.The general framework lends itself nicely to be solved with an actor framework, but if that doesn't pan out, we can use any old object, and send it any old message!
We've already done good work there in extracting the essence.
It bears repeating why the refactoring to break up PlumeRocket is so important:
We pass PlumeRocket around as
ref
.It's a giant struct with lots of maybe-related data, giving access to different parts of the system.
Passing it, by
ref
thru Rocket will become an issue as soon as we try to goasync
, because then we have a borrowed object sitting somewhere, waiting to be.await
ed.This is all a big mess, and best of all avoided, but it's also a good opportunity to see which parts of the system we can tear apart from there tight coupling so they will work better.
Here is the list of things the worker is used for:
for the first case, i already have opened a question: https://github.com/riker-rs/riker/issues/130
and received an answer