Skip to content

Add host memory introspection utilities - #756

Open
dsaini2-sc wants to merge 1 commit into
mainfrom
dsaini2/add-host-memory-utilities
Open

Add host memory introspection utilities#756
dsaini2-sc wants to merge 1 commit into
mainfrom
dsaini2/add-host-memory-utilities

Conversation

@dsaini2-sc

Copy link
Copy Markdown
Collaborator

Adds gigl/utils/host_memory.py and its tests. Two new files; no existing code path changes.

Why

psutil.virtual_memory().available reads /proc/meminfo, which inside a container reports the host's memory rather than the container's limit. A memory budget check that trusts it alone can pass moments before the container is OOM-killed. This module reads the cgroup limit alongside meminfo and believes the smaller.

What it handles

  • Resolves the process's own cgroup rather than the hierarchy root, translated through the mount root, trying v2 then v1.
  • Picks the cgroup with the least headroom. Every ancestor constrains the process, so the first limit found walking up is not necessarily the binding one.
  • cgroup_memory_breakdown separates anonymous memory from reclaimable page cache, which is what decides whether sitting near the limit is survivable.
  • log_stage_memory logs that split at pipeline phase boundaries.

Testing

45 tests pass; ty and ruff clean. The tests build real cgroup hierarchies in temporary directories and read them through the real code path, mocking only cgroup discovery and psutil.

psutil.virtual_memory().available reads /proc/meminfo, which inside a container reports the HOST's memory rather than the container's limit, so a memory budget check that trusts it alone can pass immediately before the container is killed. This module reads the cgroup limit alongside meminfo and believes the smaller.

It resolves the process's own cgroup path rather than assuming the hierarchy root, translates that path through each mount's root, tries v2 then v1, and selects the cgroup with the least headroom, since every ancestor constrains the process and allocations compete with every other descendant of it.

cgroup_memory_breakdown separates anonymous memory from reclaimable page cache, which is what decides whether sitting near the limit is survivable; for v1 it reads the hierarchical total_* fields so the breakdown describes the same population as the usage figure beside it. log_stage_memory logs that split at pipeline phase boundaries.

Adds a module and its tests only; no existing code path changes.
self.assertLessEqual(available, host_memory.psutil.virtual_memory().total)


class MemoryBreakdownTest(CgroupResolutionTest):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: inheriting from a concrete test class will inherits its test methods. consider refactor it to move test methods out of the base class

Comment thread gigl/utils/host_memory.py
limit, current = limits
parts.append(
f"cgroup {current / 2**30:.1f}/{limit / 2**30:.1f} GiB "
f"({100.0 * current / limit:.0f}%) headroom {(limit - current) / 2**30:.1f} GiB"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: guard against division by 0?

Comment thread gigl/utils/host_memory.py
Comment on lines +1 to +6
"""How much host memory this process may still allocate.

``psutil.virtual_memory().available`` reads ``/proc/meminfo``, which inside a container reports the
HOST's memory rather than the container's limit, so a budget check trusting it alone can pass
immediately before the container is killed. The cgroup knows the real limit; read both and believe
the smaller.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. Can we update this docstring for recommended usage of the module? The public functions are all at the bottom which can make it hard for human readers to discover

Comment thread gigl/utils/host_memory.py
Comment on lines +51 to +53
fields = line.strip().split(":", 2)
if len(fields) != 3:
continue

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit, can we leave the expected output here?

Comment thread gigl/utils/host_memory.py
parts = [part for part in path.split("/") if part]
while True:
candidate = "/" + "/".join(parts)
if candidate not in expanded:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

BTW do we call this often? Do we suspect that using a set here and t hen converting to list later (if needed) would be faster?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On the perf note (and if if this isn't expensive it's ok to ignore), can we do the following:

  1. have `parts: tuple[str, ...] = tuple(part for part in path.split("/") if part)
  2. for i in range(len(parts)
  3. if parts[:len(parts-i) in ...
  4. Add the sub-tuple to expanded (as a set)
  5. Covert all to str at the end.

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.

3 participants