Support for Memory Statisticsd Process and Configurations by Arham-Nasir · Pull Request #20355 · sonic-net/sonic-buildimage

Arham-Nasir · 2024-09-26T11:35:02Z

The Memory_Statisticsd Process was added to enhance system monitoring by collecting memory usage data at configurable intervals. This will allow the system to store historical data, making it easier to analyze memory trends and debug memory-related issues.

…sage data Signed-off-by: Arham-Nasir <[email protected]>

Arham-Nasir · 2024-10-02T12:36:57Z

Hi @qiluo-msft , @prgeor , and @FengPan-Frank ,

Could you please help review the HLD and the linked code PRs for our feature?

Thank you!

FengPan-Frank · 2024-10-11T13:46:09Z

files/image_config/memory_statistics/memory_statistics_handler.py

+                memory_statistics = self.collect_memory_statistics()
+                self.store_memory_statistics(memory_statistics)
+            except Exception as e:
+                self.logger.log(f"Error collecting or storing memory statistics: {e}", logging.ERROR)


will this be repeated in dead loop and generate bunch of logs?

Thank you for your feedback. The memory statistics daemon is designed with robust loop control to prevent dead loops. It collects data at a configurable interval, logs it in compressed format for historical data, and shuts down gracefully when triggered. The loop control mechanism ensures it waits according to the sampling interval and only continues running until a shutdown signal is received. Potential issues could arise from misconfigurations (e.g., zero or negative sampling intervals) but the daemon is designed to handle such cases effectively.

…-process

Signed-off-by: Arham-Nasir <[email protected]>

…:Arham-Nasir/sonic-buildimage into feature/memory-statistics-daemon-process Merge branch 'feature/memory-statistics-daemon-process' - Integrated changes from the latest upstream. - Added SyslogLogger class for syslog message logging.

…handling, time delta, and memory size formatting. Signed-off-by: Arham-Nasir <[email protected]>

…d retention calculations Signed-off-by: Arham-Nasir <[email protected]>

…stics reports Signed-off-by: Arham-Nasir <[email protected]>

Signed-off-by: Arham-Nasir <[email protected]>

…or and MemoryStatisticsCollector class Signed-off-by: Arham-Nasir <[email protected]>

ridahanif96 · 2024-11-06T08:49:34Z

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

Signed-off-by: Arham-Nasir <[email protected]>

Arham-Nasir · 2024-11-21T13:40:19Z

Hi @FengPan-Frank and @qiluo-msft ,
When you have a moment, could you please review this PR? I would appreciate your feedback and suggestions. Thank you for your time and guidance

FengPan-Frank · 2024-11-22T08:42:41Z

Hi @FengPan-Frank and @qiluo-msft , When you have a moment, could you please review this PR? I would appreciate your feedback and suggestions. Thank you for your time and guidance

Thanks for the work, could you add test case to cover collector service code?

Arham-Nasir · 2024-11-23T16:47:47Z

Hi @FengPan-Frank and @qiluo-msft , When you have a moment, could you please review this PR? I would appreciate your feedback and suggestions. Thank you for your time and guidance

Thanks for the work, could you add test case to cover collector service code?

Thank you for the review and suggestion, I’ll add test cases for the collector service code. Please let me know if there are any specific scenarios you'd like me to cover.

FengPan-Frank · 2024-11-25T11:52:08Z

Hi @FengPan-Frank and @qiluo-msft , When you have a moment, could you please review this PR? I would appreciate your feedback and suggestions. Thank you for your time and guidance

Thanks for the work, could you add test case to cover collector service code?

Thank you for the review and suggestion, I’ll add test cases for the collector service code. Please let me know if there are any specific scenarios you'd like me to cover.

Just common sanity check to cover the daemon functions.

…ully Signed-off-by: Arham-Nasir <[email protected]>

xincunli-sonic · 2024-12-02T18:54:30Z

files/image_config/memory_statistics/memory_statistics.conf

@@ -0,0 +1,10 @@
+# Memory Statistics Daemon Configuration


Would it better if these configurations can put into config db instead of a static conf file? #Closed

We have stored configurations in both config_db and a static conf file for flexibility and reliability.

At startup, the daemon reads default settings from the conf file to ensure a consistent state after a restart. During runtime, the daemon dynamically updates its configuration from config_db without needing a restart. This approach combines the stability of predefined defaults with the convenience of live updates, ensuring predictable behavior and seamless adaptability.

xincunli-sonic · 2024-12-02T18:55:12Z

files/image_config/memory_statistics/memory_statistics_handler.py

@@ -0,0 +1,1877 @@
+#!/usr/bin/env python3
+import psutil


Can you sort import as nit purpose? #Closed

Thank you for the review. I have organized the imports following the standard Python import grouping practice: standard library imports, third-party imports, and local application imports, sorted alphabetically within each group for improved readability and maintainability.

xincunli-sonic · 2024-12-02T18:59:53Z

files/image_config/memory_statistics/memory_statistics_handler.py

+        """Logs a message with the 'DEBUG' level."""
+        self.log(syslog.LOG_DEBUG, message)
+
+    def close_logger(self):


close_logger

There is no caller for this close method? #Closed

I have addressed it by adding context manager methods (enter, exit) to manage the syslog connection lifecycle, ensuring it opens and closes cleanly. Additionally, the close method now ensures proper resource management when used explicitly.

xincunli-sonic · 2024-12-02T19:02:24Z

files/image_config/memory_statistics/memory_statistics_handler.py

+                        logger.log_info(f"Removing outdated entry: {old_entry}")
+                        del total_dict['system_memory'][memory_type][old_entry]
+
+        total_dict['system_memory']['count'] = len(total_dict['system_memory'].get('system', {}))


count

A constant? #Closed

xincunli-sonic · 2024-12-02T19:04:31Z

files/image_config/memory_statistics/memory_statistics_handler.py

+                logger.log_info("Accepted new connection")
+                self.handle_connection(connection)
+            except socket.timeout:
+                continue  


continue

Add log in case timeout happen, but lost debuggability. #Closed

Thanks for the feedback! I've added a logger.log_debug for socket.timeout to improve debuggability.

xincunli-sonic · 2024-12-02T19:05:22Z

files/image_config/memory_statistics/memory_statistics_handler.py

+        """
+        self.pid_file = pid_file
+
+    def daemonize(self):


daemonize

Add validation to confirm the daemonization process succeeded and log an appropriate error if it did not.

Hi @xincunli-sonic ,

The enhanced Daemonizer class is designed to ensure reliable daemon creation, with robust validation for process isolation, session management, and file descriptor redirection.

Thank you.

xincunli-sonic · 2024-12-02T19:06:42Z

files/image_config/memory_statistics/memory_statistics_handler.py

+            if slot < num_columns:
+                self.add_entry_to_time_group_list(time_entry_summary, slot, memory_entry)
+
+    def aggregate_data(self, request_data, time_entry_summary, num_columns):


aggregate_data

In aggregate_data, the logic for adding entries to the time_group_list could be inefficient for large datasets due to multiple nested calls and list manipulations.
Suggestion: Use batch processing or caching mechanisms to reduce redundant operations and improve performance. #Closed

Thank you for your valuable feedback and suggestion to optimize the aggregate_data function. I have updated the code by implementing batch processing with dynamic sizing to reduce redundant operations, minimizing nested calls for better performance, and adding robust error handling with detailed logging. Let me know if further adjustments are needed.

xincunli-sonic · 2024-12-02T19:08:15Z

files/image_config/memory_statistics/memory_statistics_handler.py

+        logger.log_info(f"Service initialized with name: {self.name}")
+
+        self.config_file_path = config_file_path
+        self.memory_statistics_lock = threading.Lock()


memory_statistics_lock

Please check all code for shared resource access and ensure that locks are applied consistently, especially when modifying global state. #Closed

Thank you for pointing this out. I have carefully reviewed the code and ensured that locks are applied consistently for all shared resource access.

Added ThreadSafeConfig class for safe configuration access and consistently used memory_statistics_lock during memory statistics collection to prevent race conditions.

Let me know if further adjustments are needed

qiluo-msft · 2025-04-23T01:46:43Z

files/image_config/memory_statistics/memory_statistics_service.py

+            if os.path.exists(pid_file):
+                os.unlink(pid_file)
+                logger.log_info(f"Removed PID file: {pid_file}")
+        except Exception as e:


except Exception as

The same

Replaced except Exception with specific exceptions for improved clarity and control.

qiluo-msft · 2025-04-23T01:48:13Z

files/image_config/memory_statistics/memory_statistics_service.py

+        try:
+            with gzip.open(memory_statistics_config['TOTAL_MEMORY_STATISTICS_LOG_FILENAME'], 'wt', encoding='utf-8') as jfile:
+                json.dump(total_dict, jfile)
+        except Exception as e:


except Exception

still too general Exception

Thank you for the feedback.
I’ve replaced all instances of the generic except Exception with more specific exceptions to ensure better error handling and clarity. Please let me know if you have any further suggestions.

mssonicbld · 2025-05-05T07:53:23Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-05T07:53:35Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Arham-Nasir <[email protected]>

mssonicbld · 2025-05-06T07:49:27Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-06T07:49:39Z

Azure Pipelines successfully started running 1 pipeline(s).

lguohan · 2025-05-06T15:41:01Z

need to rethink the approach, this looks like reinvent the wheel of time series database on the sonic itself.

Signed-off-by: Arham-Nasir <[email protected]>

mssonicbld · 2025-05-12T21:03:28Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-12T21:03:40Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Arham-Nasir <[email protected]>

mssonicbld · 2025-05-13T10:05:46Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-13T10:06:00Z

Azure Pipelines successfully started running 1 pipeline(s).

… feature/memory-statistics-daemon-process

mssonicbld · 2025-05-14T19:17:18Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-14T19:17:31Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Arham-Nasir <[email protected]>

mssonicbld · 2025-05-15T08:08:58Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-15T08:09:10Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Arham-Nasir <[email protected]>

mssonicbld · 2025-05-18T18:45:12Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-18T18:45:24Z

Azure Pipelines successfully started running 1 pipeline(s).

Arham-Nasir · 2025-05-20T08:39:18Z

need to rethink the approach, this looks like reinvent the wheel of time series database on the sonic itself.

We want to clarify that the purpose of memorystatsd is not to build a time-series database (TSDB) inside the SONiC Redis DB. Instead, memorystatsd is designed to be a simple and lightweight solution that fits well in SONiC environments.

To keep the system efficient and reduce complexity, memorystatsd avoids using Redis for storing data.

Instead, it saves compressed JSON snapshots to /var/log/memorystats/memory-stats.json.gz.

Each snapshot is very compact approximately 225 bytes resulting in a total of around 63 KB per day. Depending on the configured retention period, the storage footprint remains minimal.

Old data is automatically deleted based on the set retention period, helping to meet platform limits and keep resource usage low.

wajahatrazi · 2025-05-20T08:41:41Z

Dear @qiluo-msft

This is the only PR left to be merged as per the scope of the HLD, hoping for your support.

qiluo-msft · 2025-05-23T21:16:24Z

I left a comment in Add HLD for Memory Statistics Enhancement with New Metrics, Leak Detection, and gNMI Access by Arham-Nasir · Pull Request #1962 · sonic-net/SONiC
Comment raised in community review meeting: why develop a new daemon MemoryStatsd for monitor memory data, which is overlapping with existing procdockerd? Could this new feature design reusing existing daemon or combined them into one daemon?
I see some code PRs are already merged, but this comment is still applicable.

Please resolve this issue at high priority

Added Memory Statistics Daemon to collect, store, and manage memory u…

4b20cb2

…sage data Signed-off-by: Arham-Nasir <[email protected]>

Arham-Nasir requested a review from lguohan as a code owner September 26, 2024 11:35

Arham-Nasir mentioned this pull request Sep 26, 2024

HLD for Memory_Statistics sonic-net/SONiC#1760

Merged

FengPan-Frank reviewed Oct 11, 2024

View reviewed changes

Arham-Nasir and others added 10 commits October 14, 2024 18:32

Merge branch 'sonic-net:master' into feature/memory-statistics-daemon…

03d0650

…-process

Add SyslogLogger class for syslog message logging

27a31e1

Signed-off-by: Arham-Nasir <[email protected]>

Added Dict2Obj for dict-object conversion and Utility class for date …

4ef28ef

…handling, time delta, and memory size formatting. Signed-off-by: Arham-Nasir <[email protected]>

Added TimeProcessor class to handle time validation, date parsing, an…

08cddde

…d retention calculations Signed-off-by: Arham-Nasir <[email protected]>

Add MemoryReportGenerator class for generating formatted memory stati…

b886d0f

…stics reports Signed-off-by: Arham-Nasir <[email protected]>

Add MemoryEntryManager class for managing and formatting memory entries

565201e

Signed-off-by: Arham-Nasir <[email protected]>

Add SocketHandler class for UNIX socket communication

f068c78

Signed-off-by: Arham-Nasir <[email protected]>

Adjusting the recomemnded file permission for unix socket

2b01214

Signed-off-by: Arham-Nasir <[email protected]>

Added the MemoryStatisticsService, Daemonizer,MemoryStatisticsProcess…

d7f3fb9

…or and MemoryStatisticsCollector class Signed-off-by: Arham-Nasir <[email protected]>

update the memory_statistics handler file

4f84986

Signed-off-by: Arham-Nasir <[email protected]>

Improved the config_db functionality to handle validation more gracef…

e96bb24

…ully Signed-off-by: Arham-Nasir <[email protected]>

xincunli-sonic reviewed Dec 2, 2024

View reviewed changes

qiluo-msft reviewed Apr 23, 2025

View reviewed changes

Pulled Changes from the Remote Master BuildImage

935f6da

updated the mem-stats code

b9f31ea

Signed-off-by: Arham-Nasir <[email protected]>

remove the all the generic exceptions and update code

5e994d9

Signed-off-by: Arham-Nasir <[email protected]>

update the memstats file

3f9667e

Signed-off-by: Arham-Nasir <[email protected]>

Merge branch 'master' of github.com:Arham-Nasir/sonic-buildimage into…

cf7649e

… feature/memory-statistics-daemon-process

update mem-stats

56ed56e

Signed-off-by: Arham-Nasir <[email protected]>

update memory statistics service file

7a31207

Signed-off-by: Arham-Nasir <[email protected]>

Arham-Nasir dismissed stale reviews from xincunli-sonic, ridahanif96, and FengPan-Frank via 7a31207 March 10, 2026 10:09

Conversation

Arham-Nasir commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arham-Nasir commented Oct 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ridahanif96 commented Nov 6, 2024

Uh oh!

Arham-Nasir commented Nov 21, 2024

Uh oh!

FengPan-Frank commented Nov 22, 2024

Uh oh!

Arham-Nasir commented Nov 23, 2024

Uh oh!

FengPan-Frank commented Nov 25, 2024

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xincunli-sonic Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented May 5, 2025

Uh oh!

azure-pipelines bot commented May 5, 2025

Uh oh!

mssonicbld commented May 6, 2025

Uh oh!

azure-pipelines bot commented May 6, 2025

Uh oh!

Arham-Nasir commented Sep 26, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading

xincunli-sonic Dec 2, 2024 •

edited

Loading