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

RSDK-7986 - stop logging noisy errors #121

Merged

Conversation

stuqdog
Copy link
Member

@stuqdog stuqdog commented Jun 24, 2024

We were generating noisy errors in rust-utils occasionally when trying to connect. This seemed to be the result of parallel dialing, but it happened non-deterministically. While the errors were basically harmless (one branch always succeeds and so a connection is always still formed), they were noisy and scary-looking (it could be easy to miss that a connection was in fact formed successfully and that any calls were made successfully).

Some digging revealed that the cause of this problem was a failure to receive updates from the signaling service, meaning we never set a UUID and so sent an empty string as our UUID. Thus, we can avoid the noisy errors by checking if the UUID is empty before sending an update request and just not sending if so. The exact cause of the problem is unknown to me at this time; I'd love to dig into it more but given that this issue doesn't seem to be actually affecting performance or functionality in any way, it's probably not worth the effort at this time. Very happy to revisit when that changes (or if anyone disagrees)!

cc @benjirewis, who helpfully noticed and flagged this issue to begin with.

@stuqdog stuqdog requested a review from a team as a code owner June 24, 2024 14:51
@stuqdog stuqdog requested review from purplenicole730, lia-viam and njooma and removed request for purplenicole730 June 24, 2024 14:51
@benjirewis
Copy link
Member

one branch always succeeds and so a connection is always still formed

Is it possible for both connections to succeed and us to "leak" one of them? We had that issue in go.

@stuqdog
Copy link
Member Author

stuqdog commented Jun 24, 2024

one branch always succeeds and so a connection is always still formed

Is it possible for both connections to succeed and us to "leak" one of them? We had that issue in go.

It shouldn't be, at least as I understand it. Rust drops the unused connection, which should be closing the data channel. Some rudimentary testing seems to support this, but I confess I'm not 100% confident here because I'm not sure what exactly I should be looking for as signs of a leak. How did y'all identify the leak?

@benjirewis
Copy link
Member

Rust drops the unused connection, which should be closing the data channel.

Cool; I think that should be fine. IIRC the mechanics of goroutines were kind of limiting us and we had a difficult time ensuring the second goroutine to establish a connection both halted and closed its connection.

@stuqdog stuqdog merged commit 4ce1135 into viamrobotics:main Jun 26, 2024
4 checks passed
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.

3 participants