Skip to content

NC | Ignore EINVAL in stat for list#9247

Merged
jackyalbo merged 1 commit into
noobaa:masterfrom
jackyalbo:jacky-nc-fixes
Oct 22, 2025
Merged

NC | Ignore EINVAL in stat for list#9247
jackyalbo merged 1 commit into
noobaa:masterfrom
jackyalbo:jacky-nc-fixes

Conversation

@jackyalbo

@jackyalbo jackyalbo commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Describe the Problem

Reading extended attributes on internal directories on GPFS causes EINVAL and fails to list them.
We will now ignore this error - hopefully until we have a better way in identifying such directories.

Explain the Changes

  1. Skipping EINVAL just on list - so we won't fail the whole command - we will skip and log the issue in the log like we do for EACCESS

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

  • Doc added/updated
  • Tests added

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced file system listing operations to gracefully handle and ignore additional error conditions, improving system resilience and stability during edge case scenarios.
  • Chores

    • Added configuration option to control error handling behavior during file system operations.

Signed-off-by: jackyalbo <jacky.albo@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 22, 2025

Copy link
Copy Markdown

Walkthrough

Adds a new configuration flag NSFS_LIST_IGNORE_ENTRY_ON_EINVAL to enable ignoring EINVAL errors during namespace filesystem listing operations. Updates the stat_if_exists function to respect this flag when determining which errors to ignore during stat operations.

Changes

Cohort / File(s) Change Summary
Configuration flag addition
config.js
Adds NSFS_LIST_IGNORE_ENTRY_ON_EINVAL boolean configuration flag set to true, enabling EINVAL error suppression during NSFS listing operations
Error handling logic update
src/util/native_fs_utils.js
Modifies stat_if_exists function to ignore EINVAL errors when config.NSFS_LIST_IGNORE_ENTRY_ON_EINVAL is true, alongside existing EACCES, ENOENT, and ENOTDIR error handling

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

size/S

Suggested reviewers

  • guymguym
  • nimrod-becker
  • tangledbytes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "NC | Ignore EINVAL in stat for list" directly describes the primary change in the changeset. The PR adds a new configuration flag (NSFS_LIST_IGNORE_ENTRY_ON_EINVAL) and modifies the stat_if_exists function to ignore EINVAL errors during list operations, which is exactly what the title conveys. The title is concise, specific, and clearly communicates that EINVAL errors are being ignored in the stat function for list operations, avoiding vague terminology and accurately summarizing the main objective of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62f0e8c and 68db715.

📒 Files selected for processing (2)
  • config.js (1 hunks)
  • src/util/native_fs_utils.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
config.js (6)
src/util/native_fs_utils.js (1)
  • config (10-10)
src/sdk/namespace_fs.js (1)
  • config (13-13)
src/manage_nsfs/manage_nsfs_glacier.js (1)
  • config (6-6)
src/manage_nsfs/nc_lifecycle.js (1)
  • config (10-10)
src/cmd/manage_nsfs.js (1)
  • config (18-18)
src/cmd/nsfs.js (1)
  • config (20-20)
src/util/native_fs_utils.js (1)
config.js (1)
  • config (7-7)
🔇 Additional comments (1)
src/util/native_fs_utils.js (1)

321-322: Implementation is correct; logging already supports monitoring as suggested.

The code correctly implements the EINVAL ignore logic with a config guard and logging that will capture all skipped entries for debugging. Since the logging statement at lines 324-325 already includes the error code (err.code), post-deployment log monitoring will naturally identify patterns of EINVAL occurrences without requiring additional changes.

Comment thread config.js
Comment on lines +1016 to +1017
// we will for now handle the same way also EINVAL error - for gpfs stat issues on list (.snapshots)
config.NSFS_LIST_IGNORE_ENTRY_ON_EINVAL = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider narrowing the scope of EINVAL error suppression.

The global flag unconditionally ignores all EINVAL errors during list operations. EINVAL is a generic error code that could indicate various legitimate issues beyond GPFS internal directories. The "for now" comment suggests this is a temporary workaround.

Consider these alternatives:

  • Add path-based filtering (e.g., only ignore EINVAL for paths matching .snapshots or other known GPFS internal directories)
  • Make the config controllable via environment variable for easier debugging
  • Add more detailed logging when EINVAL is encountered to track if it's masking unexpected issues

Would you like me to generate a more targeted implementation that checks path patterns before ignoring EINVAL errors?

🤖 Prompt for AI Agents
In config.js around lines 1016-1017 the current global flag
NSFS_LIST_IGNORE_ENTRY_ON_EINVAL unconditionally suppresses all EINVAL errors;
narrow this by replacing the global boolean with a scoped policy: implement
path-based filtering so EINVAL is ignored only for known GPFS internal paths
(e.g., entries matching `.snapshots` or configurable patterns), make the
behavior toggleable via an environment variable (e.g.,
NSFS_LIST_IGNORE_ENTRY_ON_EINVAL_PATTERNS or a boolean plus pattern list) and
ensure list operation code logs detailed context (path, error stack/message and
whether it was suppressed) when an EINVAL occurs so unexpected cases are visible
for debugging.

@jackyalbo jackyalbo merged commit 92b3061 into noobaa:master Oct 22, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants