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

Sensormond changes to support configurable time for logging threshold and better handling of fatal signals #600

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gregoryboudreau
Copy link
Contributor

@gregoryboudreau gregoryboudreau commented Mar 21, 2025

Description

Two quality of life improvements for sensormond:

  1. Allow for the user to configure timeout for printing time log warning (keep default at 30)
  2. Cascade stop events (from signal handler) into voltage and current updaters to allow for more graceful checking during the update process, current setup has 10 second timeout and given the updaters can take some time to run can result in SIGKILLs getting sent during operations.

Motivation and Context

With a lot of sensors, the logs can be filled with warnings about exceeding the time to read even if it is expected by the vendor. Additionally, with this longer time, the SIGTERM can not be checked in enough time before supervisorctl falls back to using a SIGKILL

How Has This Been Tested?

Tested on Cisco Smartswitch, time configuration now no longer results in logs being triggered every loop and tested killing w/ supervisorctl stop sensormond and no longer see any error logs from system that were being hit when SIGKILL interrupted reads from driver.

Additional Information (Optional)

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bmridul bmridul self-requested a review March 21, 2025 21:39
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.

2 participants