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-9869] Globally Manage Lifecycle of Shared Connection to App #4761

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

bashar-515
Copy link
Member

After unifying the connections to App used by RDK's net appender and restart checker, we realized that the lifecycle of the connection they now share should not be managed by either of them. The net appender already does not manage it (i.e., it never tries to close it), but the restart checker does try to close the connection. This PR stops the restart checker from closing it and instead closes the connection when the server exits - once we know it [the connection] isn't getting used anymore. This oversight became apparent through the logs below. The net appender was incorrectly logging messages in its queue because the restart checker was closing the connection required for the logging to succeed.

2025-01-30T15:03:54.717Z INFO rdk.networking.netlogger logging/net_appender.go:303 error logging to network: not connected 2025-01-30T15:03:56.356Z WARN rdk.networking.netlogger logging/net_appender.go:181 NetAppender.Close() did not progress in 1.5s, closing with 22 still in queue

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jan 30, 2025
@bashar-515 bashar-515 requested a review from cheukt January 30, 2025 17:04
@bashar-515 bashar-515 marked this pull request as ready for review January 30, 2025 17:04
@bashar-515 bashar-515 merged commit 56719cc into viamrobotics:main Jan 30, 2025
16 checks passed
@bashar-515 bashar-515 deleted the RSDK-9869 branch January 30, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants