Skip to content

Improve storage collector by using dictionary delete callbacks#21500

Merged
stelfrag merged 2 commits intonetdata:masterfrom
stelfrag:w_storage_cleanup
Jan 8, 2026
Merged

Improve storage collector by using dictionary delete callbacks#21500
stelfrag merged 2 commits intonetdata:masterfrom
stelfrag:w_storage_cleanup

Conversation

@stelfrag
Copy link
Collaborator

@stelfrag stelfrag commented Dec 22, 2025

Summary
  • Ensure proper deallocation of disk-related strings and reset pointers to NULL.
  • Add metadata reset (collected_metadata = false) during cleanup to prevent stale data usage.

Summary by cubic

Switch the Windows storage collector to dictionary delete callbacks for logical and physical disks. This centralizes cleanup, fixes memory leaks, and prevents stale metadata.

  • Bug Fixes
    • Register delete callbacks for logicalDisks and physicalDisks and rely on them for cleanup.
    • Free disk-related strings and set pointers to NULL during cleanup to prevent leaks.
    • Reset collected_metadata to false to avoid stale metadata after deletions.

Written for commit 456f627. Summary will update on new commits.

@github-actions github-actions bot added area/collectors Everything related to data collection collectors/windows labels Dec 22, 2025
Ensure proper deallocation of disk-related strings and reset pointers to NULL.
Add metadata reset (`collected_metadata = false`) during cleanup to prevent stale data usage.
@stelfrag stelfrag marked this pull request as ready for review January 8, 2026 12:07
@stelfrag stelfrag requested a review from thiagoftsm as a code owner January 8, 2026 12:07
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

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

All disks on Windows are showing data as expected. LGTM!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves memory management in the Windows storage collector by switching from manual cleanup to dictionary delete callbacks. The changes centralize resource cleanup, prevent memory leaks by properly freeing strings and setting pointers to NULL, and reset metadata flags to prevent stale data usage.

Key Changes

  • Added dictionary delete callbacks (dict_logical_disk_delete_cb and dict_physical_disk_delete_cb) to automate cleanup when dictionary entries are removed
  • Enhanced cleanup functions to properly free STRING pointers and reset them to NULL
  • Added metadata flag reset (collected_metadata = false) to prevent stale data
  • Simplified cleanup loops by removing manual cleanup calls (now handled by delete callbacks)
Comments suppressed due to low confidence (1)

src/collectors/windows.plugin/perflib-storage.c:162

  • The RRDDIM pointer rd_split should be set to NULL after marking st_split obsolete and setting it to NULL. Additionally, the RRDDIM pointers within the nested ND_DISK_* structures should be set to NULL as well, following the comprehensive cleanup pattern used in other collectors like proc_net_dev.c. Each ND_DISK_* structure contains RRDDIM pointers that need to be invalidated when their parent RRDSET is marked obsolete.
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_io.st_io);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_ops.st_ops);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_util.st_util);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_busy.st_busy);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_iotime.st_iotime);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_qops.st_qops);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_await.st_await);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_svctm.st_svctm);
    rrdset_is_obsolete___safe_from_collector_thread(d->disk_avgsz.st_avgsz);
    rrdset_is_obsolete___safe_from_collector_thread(d->st_split);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stelfrag stelfrag merged commit 3bcd59a into netdata:master Jan 8, 2026
123 checks passed
@stelfrag stelfrag deleted the w_storage_cleanup branch January 8, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/collectors Everything related to data collection collectors/windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants