Skip to content

Improve file object store logging#849

Open
bukka wants to merge 1 commit intosofthsm:mainfrom
bukka:object_store_file_logging
Open

Improve file object store logging#849
bukka wants to merge 1 commit intosofthsm:mainfrom
bukka:object_store_file_logging

Conversation

@bukka
Copy link
Member

@bukka bukka commented Mar 14, 2026

Extend file object store logging for better tracking of potential of issues.

Summary by CodeRabbit

  • Chores
    • Improved diagnostic and error logging across the object store: clearer error reports for directory and file operations, more informative warnings when tokens/objects are invalidated or missing, and added debug metrics for directory/token refreshes to aid troubleshooting and monitoring.

@bukka bukka requested a review from a team as a code owner March 14, 2026 15:07
@coderabbitai
Copy link

coderabbitai bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 882b4d85-fe8c-4a4b-b694-f5c324c85292

📥 Commits

Reviewing files that changed from the base of the PR and between 2d6a82b and 342afcb.

📒 Files selected for processing (4)
  • src/lib/object_store/Directory.cpp
  • src/lib/object_store/Generation.cpp
  • src/lib/object_store/OSToken.cpp
  • src/lib/object_store/ObjectFile.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/object_store/Generation.cpp

📝 Walkthrough

Walkthrough

Adds diagnostic logging (ERROR_MSG, WARNING_MSG, DEBUG_MSG) across object store components (Directory, Generation, OSToken, ObjectFile) on error paths and state changes; no public APIs or control flow semantics were changed.

Changes

Cohort / File(s) Summary
Directory operations
src/lib/object_store/Directory.cpp
Replaced/added ERROR_MSG and WARNING_MSG on directory open, entry stat failures, rmdir/remove failures (include errno). Added DEBUG_MSG after refresh reporting file/subdirectory counts.
Generation logging
src/lib/object_store/Generation.cpp
Inserted WARNING_MSG/DEBUG_MSG around token and non-token file open/read failures, value-difference detection, and commit operations; no control-flow or API changes.
Token validation & metrics
src/lib/object_store/OSToken.cpp
Improved constructor and refresh logs (validity), added warnings when objects removed/invalidated, tightened format specifiers for object counts.
Object file operations
src/lib/object_store/ObjectFile.cpp
Added logging when invalidating objects, converted write/open failures to ERROR_MSG, and logged attribute write/commit failures before returns.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through logs at break of dawn,

Warning lights blinked on the lawn,
Errors cried out, then shrank away,
Debug notes danced through the day,
The object store hums — I munch a prawn.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Improve file object store logging' directly and concisely summarizes the main change: adding diagnostic logging across multiple file object store components without altering functionality.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/object_store/Directory.cpp`:
- Around line 225-226: The format string in the DEBUG_MSG call uses "%d" for
files.size() and subDirs.size(), which are size_t; change the format specifiers
to "%zu" (or cast the values to int if intentional) so the call becomes
DEBUG_MSG("Directory %s refreshed: %zu files, %zu subdirs", path.c_str(),
files.size(), subDirs.size()) to avoid mismatched-specifier warnings and
incorrect output; locate the DEBUG_MSG invocation in Directory.cpp (around the
Directory refresh logic) and update the format string accordingly.

In `@src/lib/object_store/OSToken.cpp`:
- Around line 673-676: The WARNING_MSG call in OSToken.cpp uses "%d" to print
removedFiles.size() (a size_t) which is a format mismatch; update the format
string in the WARNING_MSG invocation that references tokenPath.c_str() and
removedFiles.size()—replace "%d" with "%zu" so the call becomes
WARNING_MSG("Token %s: %zu object(s) no longer on disk", tokenPath.c_str(),
removedFiles.size()).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40f28a1c-f6ab-4892-9408-07f0baf2d310

📥 Commits

Reviewing files that changed from the base of the PR and between 62c20ed and 2d6a82b.

📒 Files selected for processing (4)
  • src/lib/object_store/Directory.cpp
  • src/lib/object_store/Generation.cpp
  • src/lib/object_store/OSToken.cpp
  • src/lib/object_store/ObjectFile.cpp

@bukka bukka force-pushed the object_store_file_logging branch from 2d6a82b to 342afcb Compare March 14, 2026 15:48
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.

1 participant