Skip to content

in_blob: Add conditional check for database file before insert#11502

Open
zshuang0316 wants to merge 1 commit intofluent:masterfrom
zshuang0316:patch-5
Open

in_blob: Add conditional check for database file before insert#11502
zshuang0316 wants to merge 1 commit intofluent:masterfrom
zshuang0316:patch-5

Conversation

@zshuang0316
Copy link
Contributor

@zshuang0316 zshuang0316 commented Feb 26, 2026


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • [N/A] Debug log output from testing the change
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Bug Fixes

    • Blob handling now writes to persistent storage only when appropriate, preventing unintended database insert attempts and reducing related error paths.
  • Refactor

    • Internal control flow cleaned up for clearer logic and maintainability.

@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed5c79 and c169ba9.

📒 Files selected for processing (1)
  • plugins/in_blob/blob_file.c

📝 Walkthrough

Walkthrough

This change updates blob_file_append to perform database insertion only when ctx->database_file is true, moving the DB insert call and its failure handling inside that conditional. No other functional changes were made.

Changes

Cohort / File(s) Summary
Database insertion guard
plugins/in_blob/blob_file.c
Moves the DB insertion call and its error handling inside an if (ctx->database_file) check in blob_file_append, preventing DB writes and related error handling when the flag is false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • edsiper

Poem

🐰 I nibble bugs with gentle hop,
Only write when flags say 'stop',
No stray inserts, neat and bright,
Quiet DB through calm night,
Hop, code, hop — all's alright.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a conditional check before database insertion in the in_blob plugin.

✏️ 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

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.

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/in_blob/blob_file.c (1)

81-98: ⚠️ Potential issue | 🟠 Major

Add DB rollback when blob registration fails

Line 82 inserts into the database before registration, but the error path at lines 93-95 only frees memory without calling blob_db_file_delete(). If registration fails after a successful insert, stale database rows remain while the blob file is never registered, causing future blob_db_file_exists() checks to skip files. Add rollback in the error path when ctx->database_file is enabled and bfile->db_id is valid:

if (ret == -1) {
    if (ctx->database_file && bfile->db_id >= 0) {
        blob_db_file_delete(ctx, bfile->db_id, bfile->path);
    }
    cfl_sds_destroy(bfile->path);
    flb_free(bfile);
    return -1;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/in_blob/blob_file.c` around lines 81 - 98, When
flb_input_blob_file_register(const...) fails after a prior successful
blob_db_file_insert(ctx, path, size), the code frees memory but leaves a stale
DB row; update the error path after flb_input_blob_file_register (check ret ==
-1) to, when ctx->database_file is true and bfile->db_id >= 0, call
blob_db_file_delete(ctx, bfile->db_id, bfile->path) before
cfl_sds_destroy(bfile->path) and flb_free(bfile) so the DB insert is rolled back
on registration failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@plugins/in_blob/blob_file.c`:
- Around line 81-98: When flb_input_blob_file_register(const...) fails after a
prior successful blob_db_file_insert(ctx, path, size), the code frees memory but
leaves a stale DB row; update the error path after flb_input_blob_file_register
(check ret == -1) to, when ctx->database_file is true and bfile->db_id >= 0,
call blob_db_file_delete(ctx, bfile->db_id, bfile->path) before
cfl_sds_destroy(bfile->path) and flb_free(bfile) so the DB insert is rolled back
on registration failure.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc966ed and 9b6a240.

📒 Files selected for processing (1)
  • plugins/in_blob/blob_file.c

Signed-off-by: zshuang0316 <zshuang0316@163.com>
@cosmo0920 cosmo0920 self-requested a review February 26, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants