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

Support for Memory Statistics Host-Services #167

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

Conversation

kanza-latif
Copy link

@kanza-latif kanza-latif commented Sep 26, 2024

This PR provides hostservice demon functionality. It handles the configuration updates for the MemoryStatisticsd process, and listens to the ConfigDB changes and applies them by restarting, shutting down, or reloading the MemoryStatisticsd process.

@kanza-latif kanza-latif changed the title Added memory statistics host services Support for Memory Statistics Host-Services Sep 26, 2024
@kanza-latif kanza-latif marked this pull request as ready for review September 26, 2024 10:40
@kanza-latif
Copy link
Author

@qiluo-msft @prgeor @FengPan-Frank. Please help review our feature's hld and linked code PRs.

scripts/hostcfgd Outdated Show resolved Hide resolved
"enabled": "true",
"retention_time": "15",
"sampling_interval": "5"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: indent

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FengPan-Frank , thank you for your feedback! I've adjusted the indentation for better readability. Please let me know if there are any other improvements needed.

scripts/hostcfgd Outdated
self.restart_memory_statistics() # Start or restart the daemon
else:
self.shutdown_memory_statistics() # Stop the daemon if disabled
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to validate key strictly, otherwise any input not equals to "enable" will trigger reload.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback, @FengPan-Frank . I've added stricter validation for configuration keys in the Memory_StatisticsCfg class to ensure only valid keys trigger updates. We'll also implement input checks in the command line utilities folder. Let me know if you have any further suggestions!

@kanza-latif
Copy link
Author

/azpw run

@mssonicbld
Copy link

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ridahanif96
Copy link

ridahanif96 commented Oct 25, 2024

@qiluo-msft @xincunli-sonic @zbud-msft @FengPan-Frank Please help review the feature.

@ridahanif96
Copy link

@qiluo-msft @xincunli-sonic pls help review this feature

@FengPan-Frank
Copy link
Contributor

/azpw run sonic-net.sonic-host-services

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.

5 participants