165 dev stale gathering detection

[akbulutdora]

No description provided.

[TylerBloom]

I missed this during the peer programming. We might not want a regular sender here. Instead, we might want a downgraded sender.

Imagine we drop all clients to a given actor such that the only way to get a new client is from the scheduler. This means that the actor can't (currently) receive any messages from its primary channel and could be considered dead. However, the channel is not considered closed because the scheduler holds a sender. This could impede otherwise desired cleanup.

I don't think there needs to be a change from this PR, but I wanted to note this for future efforts.

[akbulutdora]

I will downgrade this along with the tests I write on sunday.

[TylerBloom]

This is very minor, but did you consider using a non-unit, zero-sized type? Basically, a type that is functionally identical to a unit but conveys meaning in its name, say pub struct Completed.

[akbulutdora]

No I didn't. I am not very familiar with the benefits of that. If there is a resource you can share I'd love to look into it.

[TylerBloom]

The only benefit is that it's easier to understand. At runtime, there is no difference between a struct Completed; and a (). However, seeing OneshotSender<Completed> makes it easier to remember why you need a sender at all. I bring this up only because, while reviewing this, it took me a little while to remember the reasoning for why we were sending a unit back.

[TylerBloom]

Why was std::ops::Add imported?

[akbulutdora]

My ide recommended importing Add, because calculating the next check time requires adding a Duration to an Instant.

fn get_last_message_checked_at(&self) -> Instant {
    self.last_message_processed_at
        .add(Duration::from_secs(24 * 60 * 60))
}

[TylerBloom]

Ah, gotcha. You should be able to avoid that in the future by just using the + operator.

fn get_last_message_checked_at(&self) -> Instant {
    self.last_message_processed_at + Duration::from_secs(24 * 60 * 60)
}