kinode-dao / kinode

Kinode OS runtime
https://kinode.org
Apache License 2.0
35 stars 13 forks source link

feature: `net` connection timeouts should update when getting a message #591

Closed nick1udwig closed 2 hours ago

nick1udwig commented 2 weeks ago

Currently, net connections timeout after some fixed period of time, e.g.:

https://github.com/kinode-dao/kinode/blob/main/kinode/src/net/tcp/utils.rs#L85

It would be better if they instead timed out after a period of time since last message.

We should also consider whether the current timeout of 30m is appropriate. Should it be shorter?

dr-frmr commented 2 weeks ago

Are you sure? My understanding of the select statement here is that the timeout is actually dropped if one of the other branches (send or receive) completes first

dr-frmr commented 2 weeks ago

Thinking closely about this actually makes me slightly concerned that sent/received messages could be dropped if they occur near-simultaneously here -- need to check.

nick1udwig commented 6 days ago

Sorry, missed the notif here.

Are you sure? My understanding of the select statement here is that the timeout is actually dropped if one of the other branches (send or receive) completes first

You're right, but because both read and write are infinite loops that only break on error, I think this leads to non-erroring connections having fixed lifetimes unrelated to their activity. This isn't terrible, but could be improved by keeping active connections alive.

dr-frmr commented 5 days ago

you're right -- i'll fix

dr-frmr commented 3 hours ago

https://github.com/Enigma-Dark/kinode-engagement/issues/18#issuecomment-2481725115

funny enough--looks like we're better off forcing reconnections (after 30 minutes!) as a way to rotate session keys

nick1udwig commented 2 hours ago

lol nice

If we ever want to run Kinode on a phone, we'll probably want to have much shorter-lived connections (to save on the "keep-alive" ping battery costs), but we can always revisit this if that becomes something we're actually considering implementing.