dora icon indicating copy to clipboard operation
dora copied to clipboard

Add a new event type to notify nodes about dropped inputs

Open phil-opp opened this issue 1 year ago • 6 comments

This gives nodes a way to check whether some inputs have been dropped by dora. The event also includes a drop reason, which is currently only the queue size.

This might make it easier to debug issues such as https://github.com/dora-rs/dora/issues/515

phil-opp avatar Jun 05 '24 13:06 phil-opp

I feel like this is a bit of a weird features, as if a node start dropping input is probably because of being under heavy load.

I would probably track dropped input at a daemon level but not send additional input to a node that is already under load.

haixuanTao avatar Jun 06 '24 09:06 haixuanTao

Could we before adding additional features such as additional event, focus on our current codebase to push stability first?

haixuanTao avatar Jun 06 '24 09:06 haixuanTao

My motivation is that it makes it easier to debug issues such as #515. There is currently no way for nodes to know whether inputs were dropped intentionally or because of a bug in dora. So I think that it is related to stability given that users no longer need to look through our GitHub issue tracker to find out why dora dropped some inputs.

phil-opp avatar Jun 06 '24 10:06 phil-opp

Alternative idea: Instead of adding a new event type, we could add a dropped: usize field to Input events. This way, we don't have the additional overhead of an extra event and it's easier to ignore the information. What do you think about this idea, @haixuanTao?

phil-opp avatar Jun 06 '24 11:06 phil-opp

I understand why you would want to add this for debuggability, but I would want to move maybe on other things before handling this error.

Can we maybe post pone this PR?

Thanks a lot!

haixuanTao avatar Jun 06 '24 12:06 haixuanTao

Instead of adding a new event type, we could add a dropped: usize field to Input events.

I pushed some new commits that implement this approach. It's a much smaller change this way.

Can we maybe post pone this PR?

Sure, this PR is not urgent. I just think that something like this would make dora's behavior clearer for users.

phil-opp avatar Jun 06 '24 13:06 phil-opp