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

[Mellanox] do not use system clock to avoid issues caused by system time change #238

Closed
wants to merge 1 commit into from

Conversation

Junchao-Mellanox
Copy link
Owner

@Junchao-Mellanox Junchao-Mellanox commented Jan 15, 2025

Why I did it

Mellanox platform API uses standard python time function time.time() in many places. time.time() gets time from system clock which could be changed by NTP or user. Adjusting system clock will affect the code logical and causes bugs. For example, in platform/mellanox/mlnx-platform-api/sonic_platform/utils.py there is a Timer class, the timer will trigger event with unexpected interval if user/NTP changes the system clock. This PR changes time.time() to time.monotonic to avoid such issue.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Use time.monotonic() instead of time.time .

How to verify it

Manual test.
Unit test.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@Junchao-Mellanox Junchao-Mellanox changed the base branch from master to master-log-eeprom January 15, 2025 03:06
@Junchao-Mellanox Junchao-Mellanox changed the base branch from master-log-eeprom to master January 15, 2025 03:06
@Junchao-Mellanox
Copy link
Owner Author

ci passed 314

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