Skip to content

RDKEMW-16955: Log firebolt state of the app in OOMCrash plugin#436

Draft
ks734 wants to merge 1 commit into
developfrom
topic/RDKEMW-16955
Draft

RDKEMW-16955: Log firebolt state of the app in OOMCrash plugin#436
ks734 wants to merge 1 commit into
developfrom
topic/RDKEMW-16955

Conversation

@ks734
Copy link
Copy Markdown
Contributor

@ks734 ks734 commented May 5, 2026

Description

  • OOMCrash plugin is now automatically enabled in DobbySpecConfig for every container, even if not explicitly configured in the app spec.
  • The path field in the plugin schema is no longer required. When omitted, the plugin still detects and logs OOM events but skips crash-file creation.
  • Replaced the memory.failcnt-based check with a multi-tier detection strategy using memory.oom_control:
    1. oom_kill > 0 — definitive OOM kill counter (kernel >= 4.13)
    2. under_oom > 0 — transient OOM flag fallback (kernel < 4.13)
    3. max_usage_in_bytes >= limit_in_bytes — persistent high-water-mark fallback (all kernels, catches cases where under_oom has already cleared)

Test Procedure

  • Verify on kernel >= 4.13 (oom_kill counter present)
  • Verify on kernel < 4.13 (falls back to under_oom / max_usage)
  • Containers with no oomcrash config in their spec still get OOM detection and logging

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other (doesn't fit into the above categories - e.g. documentation updates)

Requires Bitbake Recipe changes?

  • The base Bitbake recipe (meta-rdk-ext/recipes-containers/dobby/dobby.bb) must be modified to support the changes in this PR (beyond updating SRC_REV)

Copilot AI review requested due to automatic review settings May 5, 2026 10:45
Copy link
Copy Markdown
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 extends the OOMCrash plugin so it can run without an explicit path, auto-enables it during spec generation, and changes OOM detection to use memory.oom_control plus usage-based fallbacks while logging Firebolt state.

Changes:

  • Made OOMCrash path optional and skipped mount/crash-file setup when it is absent.
  • Added automatic oomcrash plugin injection in DobbySpecConfig.
  • Reworked OOM detection to parse memory.oom_control, add max-usage fallback checks, and log Firebolt annotations.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
rdkPlugins/OOMCrash/source/OOMCrashPlugin.h Declares the new memory-limit helper used by OOM detection.
rdkPlugins/OOMCrash/source/OOMCrashPlugin.cpp Implements optional-path handling, new OOM detection logic, and Firebolt state logging.
bundle/runtime-schemas/defs-plugins.json Relaxes the OOMCrash schema so path is no longer required.
bundle/lib/source/DobbySpecConfig.cpp Auto-injects the oomcrash RDK plugin into generated specs.

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

bool oomDetected = false;
if (mUtils->exitStatus != 0)
oomDetected = checkForOOM();
bool oomDetected = checkForOOM();
Comment on lines +321 to +324
// Priority 3: on kernel < 4.13 under_oom may have cleared — check max_usage
else if (isMemoryAtLimit())
{
AI_LOG_WARN("oom_control did not confirm OOM but max memory usage reached limit "
Comment on lines +342 to +347
auto prevIt = annotations.find(FIREBOLT_STATE_PREV);
if (prevIt != annotations.end())
{
fireboltState = prevIt->second;
AI_LOG_INFO("Using previous fireboltState '%s' (current may have been "
"set after OOM kill)", fireboltState.c_str());
@ks734 ks734 marked this pull request as draft May 5, 2026 13:28
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