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

ref(actix): Update Healthcheck Actor [INGEST-1481] #1374

Merged
merged 23 commits into from
Aug 2, 2022

Conversation

tobias-wilfert
Copy link
Contributor

General

As part of the effort to future-proof Relay, this PR updates the
Healthcheck Actor to work with standard
Futures
instead of the former futures crate. It also moves away from
actix internally.

The bulk of the changes are to the healthcheck.rs file with minor
changes to other files to make it work with the reaming system. Some
code that is needed to interface with the current system can be removed
once the remaining system has been updated as well.

Design Choices

  • Moving away from Actix lead us to introduce our own message handle
    loop in healthcheck.rs using tokio. Note that this does currently
    not allow for concurrent execution, which was deemed sufficient for
    the Healthcheck service for the moment.
  • The Healthcheck service does not only need to handle the IsHealthy
    messages but also handle a Shutdown message. The special thing about
    that message is that in the current system the Controller can just
    send out Shutdown messages to all actors without needing to know
    anything about the internals of the actors. This can be achieved
    through a
    watch channel.
    The Controller has the sender and services (subscribiers) have
    clones of the receiver. There is a second message handle loop inside
    the Actors that receives the Shutdown message and forwards it to the
    primary message handle loop. This will be revised in a follow-up PR.
  • For now, the actix SystemRegistry remains in place, however for that
    to work with the Tokio runtime a copy of the System needs to be
    available in each Tokio runtime thread. This is achieved by using the
    on_thread_start,
    and can be removed once actix is gone.
  • To store the address to the Healthcheck service we currently use a
    lazy_static, a minimal viable Registry of sorts.

Future Steps

  • With the switch from Futures 0.1 to Futures 0.3 on the horizon, it
    might be nice to refactor the system by changing: futures -->
    futures01 futures03 --> futures
  • The current design of the Healthcheck service only allows for
    limited parallelization. As such, it might be interesting to use
    channels internally to protect the internal resource and allow for
    greater parallelization.
  • A good choice for the next upgrade is the Upstream actor. This will
    require us to circle back and remove the hardcoded bool in the
    struct Message<T>.

This is a resubmit of #1349

@tobias-wilfert tobias-wilfert requested a review from a team August 2, 2022 09:43
@jan-auer
Copy link
Member

jan-auer commented Aug 2, 2022

Note for reviewers: The actual fix is in 52a7459.

@tobias-wilfert tobias-wilfert merged commit 5427d3a into master Aug 2, 2022
@tobias-wilfert tobias-wilfert deleted the tobias-wilfert/future-proofing-relay branch August 2, 2022 11:20
tobias-wilfert added a commit that referenced this pull request Aug 3, 2022
As part of the effort to future-proof Relay as outlined here #1374
the goal is to use the standard Futures instead of the older version
of the crate. With this effort currently under way this PR refactors
the system by changing `futures` to `futures01` and `futures03` to 
`futures`. As such it builds further on the changes made here #1374.
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.

None yet

2 participants