in_tail: fix data loss with buffered data on shutdown and gzip files#11269
in_tail: fix data loss with buffered data on shutdown and gzip files#11269jinyongchoi wants to merge 4 commits intofluent:masterfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds gzip-aware resume state and DB-backed bookkeeping: new per-file fields (anchor_offset, skip_bytes, exclude_bytes, skipping_mode), DB schema migration and SQL updates for Changes
Sequence Diagram(s)sequenceDiagram
participant Disk as Disk File
participant Reader as in_tail Reader
participant Decompress as Gzip Decompressor
participant Buffer as In-memory Buffer
participant DB as SQLite DB
Disk->>Reader: read compressed/raw bytes (advance raw offset)
Reader->>Buffer: append raw bytes
alt decompression_context (gzip)
Buffer->>Decompress: feed compressed bytes
Decompress-->>Buffer: decompressed data (may span members)
Buffer->>Buffer: apply exclude_bytes / skipping_mode (drop initial decompressed bytes)
alt gzip member boundary reached
Decompress->>Reader: notify member end
Reader->>Reader: set anchor_offset (member start), reset skip_bytes
end
Reader->>DB: persist raw offset, skip, anchor
else no decompression
Buffer->>DB: persist raw offset
end
alt Shutdown with buffered unprocessed data and no decompression
Reader->>DB: rewind offset (offset -= buf_len) and persist
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-10-23T07:43:16.216ZApplied to files:
📚 Learning: 2025-09-22T15:59:55.794ZApplied to files:
📚 Learning: 2025-08-29T06:25:27.250ZApplied to files:
📚 Learning: 2025-08-29T06:24:26.170ZApplied to files:
📚 Learning: 2025-08-29T06:25:27.250ZApplied to files:
📚 Learning: 2025-08-29T06:25:02.561ZApplied to files:
📚 Learning: 2025-08-29T06:24:55.855ZApplied to files:
📚 Learning: 2025-08-29T06:24:44.797ZApplied to files:
📚 Learning: 2025-09-08T11:21:33.975ZApplied to files:
📚 Learning: 2025-08-31T12:46:11.940ZApplied to files:
📚 Learning: 2025-11-21T06:23:29.770ZApplied to files:
🧬 Code graph analysis (2)plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
🔇 Additional comments (16)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/in_tail/tail_file.c(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (1)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(290-321)
🪛 Cppcheck (2.18.0)
plugins/in_tail/tail_file.c
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (32)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: Agent
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-centos-7
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jinyongchoi
force-pushed
the
fix/11265-in-tail-data-loss
branch
from
71208f6 to
f63f1f4
Compare
jinyongchoi
force-pushed
the
fix/11265-in-tail-data-loss
branch
from
f63f1f4 to
76f3c05
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugins/in_tail/tail_file.c (1)
1471-1471: Remove trailing whitespace.There are trailing spaces after
#endifon this line.Apply this diff:
-#endif +#endif
🧹 Nitpick comments (1)
plugins/in_tail/tail_file.c (1)
1451-1472: Consider adding a warning for compressed files with buffered data.While the fix correctly excludes compressed files (where rewinding isn't possible due to gzip decompression state requirements), users monitoring
.gzfiles may be unaware that buffered data can still be lost on shutdown. Consider adding a debug or warning message similar to:if (file->buf_len > 0) { if (file->decompression_context != NULL) { flb_plg_debug(ctx->ins, "inode=%"PRIu64" compressed file %s has %lu bytes buffered; " "rewind not supported for compressed files", file->inode, file->name, (unsigned long)file->buf_len); } else { /* existing rewind logic */ ... } }This would help users understand the limitation without affecting correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/in_tail/tail_file.c(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (1)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(290-321)
🪛 Cppcheck (2.18.0)
plugins/in_tail/tail_file.c
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-without-cxx (3.31.6)
🔇 Additional comments (1)
plugins/in_tail/tail_file.c (1)
1451-1472: LGTM! Rewind logic correctly prevents data loss for regular files.The implementation properly addresses the data loss issue when Fluent Bit shuts down with unprocessed buffered data:
- The condition
file->buf_len > 0 && file->decompression_context == NULLcorrectly identifies regular files with unprocessed data while excluding compressed files (where offset and buf_len have incompatible units).- The rewind arithmetic
offset -= buf_len(clamped to 0) correctly repositions the file offset to the start of unprocessed data.- The DB persistence call ensures the rewound offset survives restarts.
This addresses the core issue described in #11265 where partial lines and buffered data were lost on restart.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/in_tail/tail_file.c (2)
1451-1472: Core rewind logic looks correct; consider adding warning for compressed files.The check for
file->decompression_context == NULLproperly prevents the compressed-file arithmetic bug flagged in earlier reviews. For regular files, the offset and buffer length are in the same units, so the subtraction and DB update are valid.However, when compressed files have buffered decompressed data at shutdown, that data will be silently lost (since rewinding is skipped). Consider logging a warning in that case so users are aware:
if (file->buf_len > 0 && file->decompression_context == NULL) { /* * If there is data in the buffer, it means it was not processed. * We must rewind the offset to ensure this data is re-read on restart. */ off_t old_offset = file->offset; if (file->offset > file->buf_len) { file->offset -= file->buf_len; } else { file->offset = 0; } flb_plg_debug(ctx->ins, "inode=%"PRIu64" rewind offset for %s: old=%"PRId64" new=%"PRId64" (buf_len=%lu)", file->inode, file->name, old_offset, file->offset, (unsigned long)file->buf_len); #ifdef FLB_HAVE_SQLDB if (ctx->db) { flb_tail_db_file_offset(file, ctx); } #endif +} +else if (file->buf_len > 0 && file->decompression_context != NULL) { + flb_plg_warn(ctx->ins, "inode=%"PRIu64" cannot rewind compressed file %s; " + "%lu decompressed bytes in buffer may be lost on restart", + file->inode, file->name, (unsigned long)file->buf_len); }
1471-1471: Remove trailing whitespace.Minor formatting issue: extra spaces after
#endif.-#endif +#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/in_tail/tail_file.c(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (1)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(290-321)
🪛 Cppcheck (2.18.0)
plugins/in_tail/tail_file.c
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/runtime/in_tail.c (1)
2886-3086: Tighten DB rewind test to fail fast if DB offset never advancesThe overall flow of
flb_test_db_offset_rewind_on_shutdown()matches the bug you’re targeting (DB offset advanced while an incomplete line stays buffered, then verifying replay after restart). One corner case: ifget_db_offset()never observes an offset > 0 or the second poll never seescurrent_offset > start_offset, the test logs a warning viaTEST_CHECK/TEST_MSGbut continues. That can let the test pass without actually exercising the “advanced DB offset + buffered data” scenario.Consider treating a non‑advancing DB offset as a hard failure so the test can’t silently turn into a no‑op if timing changes. For example:
- if (!TEST_CHECK(current_offset > start_offset)) { - TEST_MSG("DB offset did not advance. start=%ld current=%ld", start_offset, current_offset); - } + if (!TEST_CHECK(current_offset > start_offset)) { + TEST_MSG("DB offset did not advance. start=%ld current=%ld", + start_offset, current_offset); + test_tail_ctx_destroy(ctx); + unlink(file[0]); + unlink(db); + exit(EXIT_FAILURE); + }This keeps the test robust against future timing changes in the tailer/DB path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/in_tail/tail.h(1 hunks)plugins/in_tail/tail_db.c(11 hunks)plugins/in_tail/tail_file.c(9 hunks)plugins/in_tail/tail_file_internal.h(1 hunks)plugins/in_tail/tail_fs_inotify.c(1 hunks)plugins/in_tail/tail_fs_stat.c(1 hunks)plugins/in_tail/tail_sql.h(1 hunks)tests/runtime/in_tail.c(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/in_tail/tail_sql.h
- plugins/in_tail/tail_fs_inotify.c
- plugins/in_tail/tail.h
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_file_internal.htests/runtime/in_tail.cplugins/in_tail/tail_file.cplugins/in_tail/tail_db.cplugins/in_tail/tail_fs_stat.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-09-22T15:59:55.794Z
Learnt from: nicknezis
Repo: fluent/fluent-bit PR: 10882
File: plugins/out_http/http.c:112-116
Timestamp: 2025-09-22T15:59:55.794Z
Learning: When users consider bug fixes out of scope for their focused PRs, it's appropriate to create separate GitHub issues to track those concerns rather than expanding the current PR scope.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (3)
tests/runtime/in_tail.c (3)
src/flb_gzip.c (1)
flb_gzip_compress(157-252)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_lib.c (5)
flb_service_set(652-678)flb_input_set(305-335)flb_output_set(520-551)flb_input(266-276)flb_output(279-289)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(366-399)
plugins/in_tail/tail_db.c (1)
src/flb_sqldb.c (2)
flb_sqldb_query(151-166)flb_sqldb_close(129-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (9)
plugins/in_tail/tail_fs_stat.c (1)
128-143: Truncation path correctly resets new skip/anchor stateResetting
anchor_offset,skip_bytes,exclude_bytes, andskipping_modealongsideoffset/buf_lenon truncation keeps gzip skip state consistent withadjust_counters()and avoids stale skip on truncated files. Looks good.plugins/in_tail/tail_file_internal.h (1)
44-50: Struct extensions and type choices align with DB and gzip logicSwitching
offsettooff_tand introducinganchor_offset,skip_bytes/exclude_bytes(uint64_t), andskipping_modeis consistent with how offsets and skip counters are handled intail_db.candtail_file.c, including 64‑bit DB storage. No issues from a layout or semantics standpoint.plugins/in_tail/tail_db.c (3)
28-35: Column migration viapragma_table_infois robust and fail‑fastUsing
PRAGMA table_info('in_tail_files')withcb_column_existscleanly distinguishes “column missing” from real SQL errors, and aborts if the pragma itself fails. Addingskip/anchoronly whencolumn_found == 0avoids repeatedALTER TABLEattempts and the earlier misdetection problem.Also applies to: 42-44, 61-108
182-221: Skip/anchor plumbing between DB and in‑memory state is consistent
db_file_exists()now returnsskipasuint64_tandanchorasoff_t, reading them from columns 6 and 7; this matches the SQL layout and avoids 32‑bit truncation on large gzip members.db_file_insert()bindsfile->skip_bytesandfile->anchor_offset, ensuring new rows are created with correct initial values.flb_tail_db_file_set()applies DB state back into the file:offset,skip_bytes,anchor_offset, and initializesexclude_bytes/skipping_modeonly whenskip_bytes > 0, which lines up with the transient skip semantics used intail_file.c.flb_tail_db_file_offset()updates offset, skip, and anchor together, keeping DB and in‑memory state aligned.This all looks coherent with the gzip resume design.
Also applies to: 248-280, 318-360, 365-399
424-445: Resettingdb_idon delete correctly prevents row resurrectionAfter a successful delete, setting
file->db_id = FLB_TAIL_DB_ID_NONEensures later calls that gate ondb_id > FLB_TAIL_DB_ID_NONE(e.g., inflb_tail_file_remove()) will not re‑insert a row for a file that was deliberately removed. This addresses the stale‑entry resurrection concern without changing existing update paths.tests/runtime/in_tail.c (2)
37-40: Helper and guard additions for gzip/DB tests look correct
write_raw()nicely models incomplete vs completed lines without duplicating logic fromwrite_msg().create_gzip_file()/append_gzip_file()now checkfwrite()return values and clean up on short write, avoiding silently corrupted gzip test artifacts.struct test_ctx+cb_check_gzip_resume()correctly guard msgpack string lengths and usememcmp, eliminating the prior OOB risk.get_db_offset()and the sqlite3 include are cleanly wrapped in#ifdef FLB_HAVE_SQLDB, so non‑SQLite builds won’t see sqlite symbols.wait_for_count_with_timeout()follows the same time‑diff pattern as existing wait helpers.No functional concerns here.
Also applies to: 274-303, 309-338, 340-367, 369-415, 417-459, 501-530
3088-3411: Gzip+DB runtime tests comprehensively cover the new resume semanticsThe four gzip DB tests (
db_gzip_resume_loss,db_gzip_inotify_append,db_gzip_rotation,db_gzip_multi_resume) exercise:
- replaying an incomplete last line across restarts,
- inotify/append behavior while running,
- rotation with old/new gzip files both receiving appends within
rotate_wait,- multiple resume cycles using
skip_bytes/anchor_offset.All are correctly guarded by
#ifdef FLB_HAVE_SQLDB, and the new TEST_LIST entries are under the same guard, so non‑SQLite builds won’t see DB‑dependent tests. This is solid coverage for the new anchor/skip design.Also applies to: 3441-3448
plugins/in_tail/tail_file.c (2)
47-48: Gzip resume: seek/anchor/skip behavior matches the intended design
- Including
<fluent-bit/flb_compression.h>here is appropriate given the direct use of decompressor state.- In
set_file_position():
- DB‑enabled paths now treat
file->offset > 0as the primary persisted position for both plain and gzip files, so migrated DBs that only haveoffsetare still respected.- For gzip files with established state (
anchor_offset > 0 || skip_bytes > 0), you overrideseek_postoanchor_offset, ensuring we always start decompressing from the beginning of the tracked member and then apply skipping viaskip_bytes/exclude_bytes.- When there is no DB position and
read_from_headis off, seeking to EOF and settinganchor_offsetaccordingly preserves the “tail from end” semantics while initializing anchor for future DB updates.- Post‑seek init for gzip without DB (
anchor_offset = file->offset; exclude_bytes = file->skip_bytes) cleanly seeds the transient skip countdown for the next run ifskip_bytesis ever non‑zero.- In
flb_tail_file_chunk():
- The
skipping_mode+exclude_bytesblock correctly discards already‑processed decompressed bytes before handing any remainder toprocess_content(), usingmemmovesafely over overlapping buffer regions.skip_bytes += processed_bytestracks logical decompressed progress, and the “gzip member completed” check updatesanchor_offsetto the current rawoffsetand resetsskip_bytesto 0 only when we’re at a member boundary with no buffered decompressed data. This matches the intended “anchor + relative skip” model per member.This end‑to‑end behavior looks coherent for both upgraded and newly created gzip DB entries.
Also applies to: 1027-1115, 1839-1859, 1901-1921
1304-1305: Offset rewind and DB‑ID handling correctly avoid stale or resurrected DB rows
- Initializing
file->db_idtoFLB_TAIL_DB_ID_NONEinflb_tail_file_append()ensures we never try to update offsets for files that haven’t been inserted into the DB yet.- In
flb_tail_file_remove():
- For non‑compressed files, rewinding
file->offsetbyfile->buf_len(capped at 0) when the buffer is non‑empty ensures any unprocessed bytes are re‑read on restart, directly addressing the original data‑loss bug.- The DB update is gated on
ctx->db && file->db_id > FLB_TAIL_DB_ID_NONE, so entries that were explicitly deleted viaflb_tail_db_file_delete()(which now resetsdb_idto the sentinel) are not silently re‑created.- For gzip files, you intentionally avoid rewinding the raw offset, emit a clear debug message about buffered data, and still persist the current DB state when applicable; this matches the stated “no rewind for compressed” constraint.
adjust_counters()now resetsanchor_offset,skip_bytes,exclude_bytes, andskipping_modewhen a truncation is detected, aligning the truncation path with the fs backends and preventing stale skip state after a compressed file is truncated.These pieces together give the right rewind semantics for plain files and safe DB behavior around deletion/truncation.
Also applies to: 1478-1516, 1632-1647
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/in_tail/tail_file.c (1)
1054-1078: Missingexclude_bytesandskipping_modeinitialization for gzip DB resume.When resuming a gzip file from DB with
skip_bytes > 0:
skip_bytesis loaded from DB- Code seeks to
anchor_offsetand returns at line 1078exclude_bytesandskipping_modeare never initializedWithout
skipping_mode = FLB_TRUE, the skip logic at line 1839 won't trigger, causing already-processed decompressed data to be reprocessed—resulting in duplicate logs.Add initialization after seeking for gzip files in the DB path:
file->offset = ret; + + /* Initialize skip state for gzip resume */ + if (file->decompression_context != NULL && file->skip_bytes > 0) { + file->exclude_bytes = file->skip_bytes; + file->skipping_mode = FLB_TRUE; + } } else if (ctx->read_from_head == FLB_FALSE) {
🧹 Nitpick comments (2)
plugins/in_tail/tail_db.c (1)
216-220: Column indices are correct; consider explicit SELECT for robustness.The column access indices at lines 216-220 correctly reference the schema:
- Column 6 →
skip✓- Column 7 →
anchor✓However, since
SQL_GET_FILEusesSELECT *, the hardcoded indices are brittle against future schema changes. Consider refactoring to explicitly name columns:SELECT id, name, offset, inode, created, rotated, skip, anchor FROM in_tail_files ...to ensure column positions remain correct.plugins/in_tail/tail_file.c (1)
1110-1113:skipping_modenot set in non-DB decompression path.If
skip_bytes > 0here,skipping_modeshould also be set toFLB_TRUEfor the skip logic to trigger. Whileskip_bytesis likely 0 in this non-DB path, the initialization should be consistent.else { file->anchor_offset = file->offset; file->exclude_bytes = file->skip_bytes; /* Init transient skip countdown */ + if (file->skip_bytes > 0) { + file->skipping_mode = FLB_TRUE; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/in_tail/tail.h(1 hunks)plugins/in_tail/tail_db.c(11 hunks)plugins/in_tail/tail_file.c(9 hunks)plugins/in_tail/tail_file_internal.h(1 hunks)plugins/in_tail/tail_fs_inotify.c(1 hunks)plugins/in_tail/tail_fs_stat.c(1 hunks)plugins/in_tail/tail_sql.h(1 hunks)tests/runtime/in_tail.c(5 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_fs_stat.cplugins/in_tail/tail_file_internal.hplugins/in_tail/tail_db.ctests/runtime/in_tail.cplugins/in_tail/tail_file.cplugins/in_tail/tail_fs_inotify.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
tests/runtime/in_tail.cplugins/in_tail/tail_file.c
📚 Learning: 2025-09-22T15:59:55.794Z
Learnt from: nicknezis
Repo: fluent/fluent-bit PR: 10882
File: plugins/out_http/http.c:112-116
Timestamp: 2025-09-22T15:59:55.794Z
Learning: When users consider bug fixes out of scope for their focused PRs, it's appropriate to create separate GitHub issues to track those concerns rather than expanding the current PR scope.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (3)
plugins/in_tail/tail_db.c (1)
src/flb_sqldb.c (2)
flb_sqldb_query(151-166)flb_sqldb_close(129-149)
tests/runtime/in_tail.c (3)
src/flb_gzip.c (1)
flb_gzip_compress(157-252)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_lib.c (6)
flb_input_set(305-335)flb_output_set(520-551)flb_start(983-994)flb_stop(1011-1055)flb_input(266-276)flb_output(279-289)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(366-399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (26)
plugins/in_tail/tail.h (1)
36-38: LGTM!The
FLB_TAIL_DB_ID_NONEmacro is a clean addition for representing the "not in database" state. Using 0 is appropriate since SQLite rowids start at 1, so this serves as a safe sentinel value to prevent resurrecting deleted DB entries.plugins/in_tail/tail_file_internal.h (1)
44-49: LGTM!The new per-file state fields are well-structured:
skip_bytesandexclude_bytescorrectly useuint64_tto avoid 32-bit truncation (addressing the past review concern)anchor_offsetusesoff_tconsistent with the file offset type- Clear comments documenting each field's purpose
plugins/in_tail/tail_fs_inotify.c (1)
270-273: LGTM!Proper initialization of the new per-file state fields during file truncation. Resetting
anchor_offset,skip_bytes,exclude_bytes, andskipping_modewhen a file is truncated ensures the skip/anchor tracking starts fresh, which is correct behavior for handling truncated files.plugins/in_tail/tail_fs_stat.c (1)
139-142: LGTM!Consistent with the inotify backend changes. Both filesystem backends now properly reset the skip/anchor state fields during file truncation, ensuring uniform behavior regardless of which backend is in use.
plugins/in_tail/tail_sql.h (1)
30-53: LGTM!The SQL schema and statement updates are well-structured:
- New
skipandanchorcolumns withDEFAULT 0ensure backward compatibilitySQL_INSERT_FILEandSQL_UPDATE_OFFSETcorrectly include the new fields- The migration logic in
tail_db.chandles upgrades from older databasesplugins/in_tail/tail_db.c (4)
28-34: LGTM!Clean implementation of the
cb_column_existscallback for detecting column presence during migration. This addresses the past review concern about usingSELECTqueries which could misfire on non-schema errors.
61-107: LGTM!The migration logic correctly uses
PRAGMA table_infoto detect missing columns, addressing the past review feedback. The pattern properly handles both theskipandanchorcolumns with appropriate error handling and cleanup.
348-359: LGTM!Correct initialization of
skipping_modeandexclude_bytesbased on the persistedskip_bytesvalue. When resuming from a DB with a non-zero skip value, the file will enter skipping mode to skip already-processed decompressed bytes.
444-444: LGTM!Resetting
db_idtoFLB_TAIL_DB_ID_NONEupon deletion prevents accidentally resurrecting deleted DB entries whenflb_tail_db_file_offsetis later called. This directly addresses the PR objective.tests/runtime/in_tail.c (11)
37-40: LGTM!The
sqlite3.hinclude is now properly guarded withFLB_HAVE_SQLDB, addressing the past review concern about builds without SQLite headers.
274-303: LGTM!The
write_rawhelper is well-designed for testing incomplete line scenarios. It correctly handles:
- Optional data write with length validation
- Optional newline append controlled by
add_newlineflag- Proper fsync to ensure data is flushed
This enables testing the exact scenario where buffered data without a trailing newline remains in the tail buffer.
309-367: LGTM!Both gzip helpers now properly check
fwrite()return values and handle errors correctly, addressing the past review concern about silent truncation. The error handling cleans up resources (closes file, frees memory) before returning -1.
397-406: LGTM!The msgpack string comparisons now correctly use
memcmpwith explicit size checks, addressing the past review concern about OOB reads with non-NUL-terminated msgpack strings:
- Key comparison:
key.via.str.size == 3 && memcmp(..., "log", 3) == 0- Value comparison:
v.via.str.size >= 5 && memcmp(..., "line2", 5) == 0
417-459: LGTM!The
get_db_offsethelper is now properly guarded with#ifdef FLB_HAVE_SQLDB, addressing the past review concern about compilation failures on builds without SQLite support.
2870-3091: Comprehensive test for offset rewind on shutdown.This test properly validates the core fix by:
- Writing data with a complete line, verifying it's processed
- Writing data without newline (stays in buffer as incomplete line)
- Polling DB until offset advances (data read into buffer)
- Stopping Fluent Bit abruptly
- Restarting and verifying buffered data is re-read
The test methodology is sound for verifying the data loss prevention mechanism.
3093-3175: LGTM!The gzip resume loss test correctly validates:
- First run processes only complete lines (
line1)- Incomplete line (
line2without newline) stays in buffer- After restart with appended data,
line2andline3are both processedfound_line2flag confirmsline2wasn't lostThe
t_ctx.found_line2 = 0reset before restart addresses the past review concern about false positives.
3178-3239: LGTM!Test properly validates gzip file append detection via inotify/refresh while Fluent Bit is running. The test confirms that appended gzip members are correctly detected and processed.
3242-3320: LGTM!Comprehensive rotation test that validates:
- Initial file processing
- File rotation (rename to .gz.1)
- New file with same name created
- Appends to both old (rotated) and new files within rotate_wait window
- Both files continue to be monitored and processed correctly
3323-3416: LGTM!Multi-resume test validates that gzip skip/anchor tracking works correctly across multiple restart cycles:
- Run 1: Process line1, stop
- Run 2: Skip line1, process only line2, stop
- Run 3: Skip line1+line2, process only line3
This confirms the skip_bytes persistence correctly prevents duplicate processing.
3449-3453: LGTM!All new DB/gzip tests are correctly registered within the
#ifdef FLB_HAVE_SQLDBguard, ensuring they're only compiled when SQLite support is available.plugins/in_tail/tail_file.c (6)
47-47: LGTM!Required header for the decompression context types and functions used in the gzip handling logic.
1304-1304: LGTM!Using the explicit
FLB_TAIL_DB_ID_NONEconstant instead of an implicit 0 improves clarity and aligns with the resurrection prevention logic at line 1499.
1478-1516: LGTM!The rewind logic correctly:
- Rewinds uncompressed files by
buf_lento prevent data loss- Avoids rewinding gzip files (infeasible with streaming decompression) and logs a warning
- Checks
db_id > FLB_TAIL_DB_ID_NONEto prevent resurrecting deleted DB entries- Uses proper format specifiers (
%jdwithintmax_tcast,%zuforsize_t)
1636-1639: LGTM!All gzip-related state (
anchor_offset,skip_bytes,exclude_bytes,skipping_mode) is properly reset on file truncation. This addresses the previous review concern about inconsistentexclude_bytesreset.
1839-1859: LGTM!The skip logic correctly handles gzip resume by discarding already-processed decompressed bytes:
- Tracks remaining bytes to skip with
exclude_bytes- Uses
memmoveto shift usable data when skip completes- Clears
skipping_modewhen done- Uses proper format specifiers
1900-1921: LGTM!The gzip member boundary detection is well-designed:
- Updates
anchor_offsetonly when a member is fully complete (state awaiting next header, no buffered compressed or decompressed data)- Resets
skip_bytesto 0 at member boundaries since no skipping is needed- Enables safe resume from the next gzip member on restart
This addresses the fundamental challenge of gzip offset tracking with streaming decompression.
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/in_tail/tail_file_internal.h (1)
44-49: Clarifyskipping_mode’s semanticsThe new offset/anchor/skip fields look appropriate, but the
skipping_modecomment (/* skipping mode ? */) is vague. Suggest clarifying what the flag actually represents (e.g., “actively discarding decompressed bytes up to exclude_bytes for gzip resume”) to aid future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/in_tail/tail_db.c(11 hunks)plugins/in_tail/tail_file.c(9 hunks)plugins/in_tail/tail_file_internal.h(1 hunks)plugins/in_tail/tail_fs_inotify.c(1 hunks)plugins/in_tail/tail_fs_stat.c(1 hunks)plugins/in_tail/tail_sql.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/in_tail/tail_fs_stat.c
- plugins/in_tail/tail_fs_inotify.c
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_file_internal.hplugins/in_tail/tail_file.cplugins/in_tail/tail_db.c
📚 Learning: 2025-09-22T15:59:55.794Z
Learnt from: nicknezis
Repo: fluent/fluent-bit PR: 10882
File: plugins/out_http/http.c:112-116
Timestamp: 2025-09-22T15:59:55.794Z
Learning: When users consider bug fixes out of scope for their focused PRs, it's appropriate to create separate GitHub issues to track those concerns rather than expanding the current PR scope.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (2)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(366-399)
plugins/in_tail/tail_db.c (1)
src/flb_sqldb.c (2)
flb_sqldb_query(151-166)flb_sqldb_close(129-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
🔇 Additional comments (8)
plugins/in_tail/tail_sql.h (1)
31-40: Schema & SQL constants line up with new skip/anchor handlingThe extended
in_tail_filesschema (skip,anchor) and the updatedINSERT/UPDATEstatements are consistent with the column indices used indb_file_exists()andflb_tail_db_file_offset()(columns 6/7 for skip/anchor). This keeps DB reads/writes coherent across migrations and new installs.Also applies to: 45-47, 52-53
plugins/in_tail/tail_db.c (3)
28-35: Migration viapragma_table_infois robust and fails fast on real DB errorsUsing
SELECT 1 FROM pragma_table_info('in_tail_files') WHERE name='skip'/'anchor'withcb_column_existscleanly distinguishes “no such column” (no rows) from generic SQL failures. On error you now log, close the DB, and abort instead of mis-treating lock/corruption/permission issues as a missing column, which is safer.Also applies to: 61-83, 85-107
182-221: DB ↔ runtime skip/anchor wiring looks correct for gzip resume
db_file_exists()now pullsskip(col 6) andanchor(col 7), matching the schema.flb_tail_db_file_set()copies those intofile->skip_bytes/file->anchor_offsetand initializesexclude_bytes/skipping_modebased onskip_bytes.This ensures a restarted gzip tailer can seek to the stored anchor, discard
skip_bytesworth of decompressed data, and continue without duplicates, while keeping raw offsets infile->offset.Also applies to: 318-360
365-376: Avoiding resurrection of deleted DB rows viadb_idsentinel is soundBinding
skip_bytesandanchor_offsetintostmt_offsetkeeps the DB in sync with the new gzip state, and resettingfile->db_idtoFLB_TAIL_DB_ID_NONEinflb_tail_db_file_delete()ensures later calls toflb_tail_db_file_offset()fromflb_tail_file_remove()are gated and don’t recreate rows that were intentionally deleted.Also applies to: 425-445
plugins/in_tail/tail_file.c (4)
1310-1311: UsingFLB_TAIL_DB_ID_NONEsentinel on initialization is appropriateInitializing
file->db_idtoFLB_TAIL_DB_ID_NONEinflb_tail_file_append()matches the new semantics intail_db.c(where this sentinel guards offset updates and is reset on delete), and avoids accidental DB updates before the file is actually registered.
1484-1522: Rewind-on-buffered-data for plain files and warning‑only path for gzip are well‑scoped
- For uncompressed files, rewinding
file->offsetbybuf_len(clamped at 0) before persisting avoids losing unprocessed buffered bytes across restarts.- The DB update is correctly guarded with
file->db_id > FLB_TAIL_DB_ID_NONE, so rows explicitly deleted earlier won’t be recreated.- For gzip inputs, you only log a warning and persist the current DB state without raw rewind, which is consistent with the “anchor + skip” design and avoids incorrect seeks when decompressed vs compressed lengths diverge.
This addresses the original data‑loss issue while avoiding the previous “resurrect deleted DB entry” problem.
1631-1652: Truncation now clears gzip skip state consistentlyIn
adjust_counters(), whensize_delta < 0you now reset:
anchor_offsetto the new offset,skip_bytesandexclude_bytesto 0,skipping_modetoFLB_FALSE,and persist the new DB offset if enabled. This prevents stale gzip skipping state from being reused after a truncate, aligning with the resets already done in the inotify/stat truncation paths.
1845-1865: Gzipexclude_bytesand member‑boundary anchor updates look logically sound
- The
skipping_modeblock drops up toexclude_bytesworth of freshly decompressed data, either by consuming whole chunks or bymemmove‑ing the remaining tail down, and only increasesbuf_lenby the post‑skip length. Buffer invariants stay intact.- After processing,
skip_bytes += processed_bytestracks the total decompressed bytes consumed since the current anchor.- When a gzip member is fully consumed (
EXPECTING_HEADER,input_buffer_length == 0,buf_len == 0), updatinganchor_offsetto the current rawoffsetand resettingskip_bytesto 0 correctly advances the anchor to the next member boundary and keeps DB state minimal.Overall this matches the intended “anchor + skip per member” model and should allow correct resume without duplicates for multi‑member gzip files.
Also applies to: 1906-1927
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
In addition to the existing implementation, the following changes were added:
Please let me know if you have any feedback or see any issues with this approach. Thanks! |
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
I found a quite minor style issue.
Otherwise, the patch looks pretty great.
We're mostly all set. 👍
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/in_tail/tail_db.c (1)
182-220: Minor type inconsistency:skipmixes uint64_t and int64_t.The
skipfield is declared asuint64_tin C (lines 184, 324) but is read/written usingsqlite3_column_int64andsqlite3_bind_int64which operate onint64_t. While unlikely to cause issues in practice (skip_bytes won't exceedINT64_MAX), this inconsistency could theoretically cause sign-extension or overflow issues with very large values.For consistency and correctness, consider using
int64_tforskipthroughout, or explicitly cast to/fromuint64_twith overflow checks.Also applies to: 262-263, 323-324, 372-375
plugins/in_tail/tail_file.c (1)
1040-1089: Critical: stream_offset not initialized for compressed files in DB path.Lines 1085-1087 only initialize
stream_offsetfor non-decompression contexts. For compressed files restored from DB withskip_bytes > 0,stream_offsetremains at 0 instead of being set toskip_bytes. This breaksoffset_keymonotonicity:
- On restart,
stream_offsetstays 0stream_offsetaccumulates from 0 at line 1912offset_key(calculated asstream_offset + processed_bytes) restarts from 0 instead of continuing from the saved positionAfter the
lseek()succeeds (line 1061), add:if (file->decompression_context == NULL) { file->stream_offset = file->offset; } else { file->stream_offset = file->skip_bytes; }This ensures
stream_offsetreflects the decompressed stream position for gzip files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/in_tail/tail_db.c(11 hunks)plugins/in_tail/tail_file.c(9 hunks)plugins/in_tail/tail_file_internal.h(1 hunks)plugins/in_tail/tail_fs_inotify.c(1 hunks)plugins/in_tail/tail_fs_stat.c(1 hunks)plugins/in_tail/tail_sql.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/in_tail/tail_fs_stat.c
- plugins/in_tail/tail_fs_inotify.c
- plugins/in_tail/tail_file_internal.h
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_db.cplugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-22T15:59:55.794Z
Learnt from: nicknezis
Repo: fluent/fluent-bit PR: 10882
File: plugins/out_http/http.c:112-116
Timestamp: 2025-09-22T15:59:55.794Z
Learning: When users consider bug fixes out of scope for their focused PRs, it's appropriate to create separate GitHub issues to track those concerns rather than expanding the current PR scope.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (2)
plugins/in_tail/tail_db.c (1)
src/flb_sqldb.c (2)
flb_sqldb_query(151-166)flb_sqldb_close(129-149)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(366-399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
🔇 Additional comments (11)
plugins/in_tail/tail_sql.h (2)
37-40: LGTM: Schema extension for gzip resume state.The addition of
skipandanchorcolumns with appropriate defaults enables the anchor+skip mechanism for gzip file resume. The trailing comma afterrotatedand the column definitions are syntactically correct.
45-47: LGTM: SQL statements updated for new columns.The INSERT and UPDATE statements correctly include the new
skipandanchorfields with proper parameter binding.Also applies to: 52-53
plugins/in_tail/tail_db.c (4)
28-34: LGTM: Column existence detection callback.The callback correctly sets the found flag when any row is returned by the PRAGMA query, enabling reliable migration detection.
61-108: LGTM: Migration logic addresses past concerns.The migration now uses
PRAGMA table_infoto reliably detect column existence instead of attempting a SELECT that could misfire on DB errors. The error handling properly aborts if the PRAGMA query itself fails, and column addition proceeds only when the column is confirmed absent.
348-359: LGTM: Skipping mode initialization from DB state.The initialization logic correctly sets
exclude_bytesandskipping_modewhen resuming from a saved gzip position withskip_bytes > 0. This properly restores the runtime state needed to skip already-processed decompressed data on restart.
444-444: LGTM: Prevent resurrecting deleted DB entries.Resetting
db_idtoFLB_TAIL_DB_ID_NONEafter deletion ensures that subsequent offset updates won't inadvertently recreate the deleted entry. This addresses the concern raised in past review comments.plugins/in_tail/tail_file.c (5)
1315-1324: LGTM: New gzip resume fields initialized.All new fields (
anchor_offset,skip_bytes,exclude_bytes,skipping_mode) are properly initialized for newly tracked files. The use ofFLB_TAIL_DB_ID_NONEis consistent with the deletion handling.
1495-1534: LGTM: Rewind logic correctly handles buffered data.The rewind implementation properly addresses the stated objectives:
- Uncompressed files: Rewinds
offsetbybuf_len(clamped to 0) so unprocessed bytes are re-read on restart- Compressed files: Logs a warning that buffered decompressed data cannot be safely rewound due to streaming decompression, accepting the known limitation
- DB protection: Checks
db_id > FLB_TAIL_DB_ID_NONEto prevent updating deleted entries- Format specifiers: Uses proper
%jdwith(intmax_t)casts foroff_tvalues
1654-1657: LGTM: Truncation resets gzip resume state.All gzip-related fields (
anchor_offset,skip_bytes,exclude_bytes,skipping_mode) are properly reset when the file is truncated, ensuring stale skip state doesn't persist after truncation. This addresses the concern raised in past review comments.
1857-1877: LGTM: Skip mechanism correctly excludes already-processed data.The skip logic properly decrements
exclude_bytesas decompressed data is produced, usingmemmoveto discard the skipped portion when a partial skip occurs, and clearingskipping_modewhen the skip completes. This correctly implements the runtime skip countdown for gzip resume.
1918-1939: LGTM: Gzip member boundary detection updates anchor points.Line 1919 increments
skip_bytesbyprocessed_bytesafter each chunk is processed, tracking decompressed progress within the current gzip member. When a member completes (decompressor state isEXPECTING_HEADER, buffers are empty), the code updatesanchor_offsetto the current compressed position and resetsskip_bytesto 0, establishing a new resume point at the member boundary. This correctly implements the anchor+skip strategy for gzip files.
|
@cosmo0920
Please let me know if there are any other concerns. |
|
I found a potential issue but we could be skipped in this PR: Interaction with multiline + gzip (MEDIUM risk, but acceptable)There’s a subtle corner: Scenario:
Risk: If shutdown occurs between
and
then DB may lag by one chunk. This results in small duplication, not loss. This scenario could be still a main risk to behind the right offset of tailing gzipped files but this just causes duplicated contents of gzipped files. This could be technically difficult to solve this. So, we need to describe such corner case if we accept this behavior. Other risks are still existing in this patch but others are relatively too small than this. Plus, it's just a logging issue but should we need to show altering database when starting to do this operation? This is just my curiosity. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/in_tail/tail_db.c (1)
258-263: Verify SQL_INSERT_FILE parameter order and address skip_bytes type mismatch.The bindings at lines 258-263 correctly match SQL_INSERT_FILE's parameter order (name, offset, inode, created, skip, anchor). However,
skip_bytesis defined asuint64_tbut bound viasqlite3_bind_int64, which expects a signedsqlite3_int64. While values exceedingINT64_MAXare unlikely for a byte offset, consider either casting toint64_texplicitly or usingsqlite3_bind_int64with a comment documenting the assumption thatskip_bytesremains within signed int64 range.
🧹 Nitpick comments (1)
plugins/in_tail/tail_file.c (1)
1027-1141: Document multi-member gzip stream_offset limitation more prominently.The comment at lines 1090-1095 acknowledges that
stream_offset = skip_bytesis only correct for single-member gzip files. For multi-member gzip,skip_bytesresets at member boundaries, sostream_offsetwon't reflect total decompressed bytes from all prior members. This could affectoffset_keyaccuracy when users concatenate multiple gzip streams.Consider either:
- Adding a more prominent warning in the code or documentation that
offset_keyfor multi-member gzip reflects position within the current member, not total decompressed bytes across all members.- Tracking total decompressed bytes separately if multi-member gzip is a common use case.
Otherwise, the seek logic correctly handles DB-backed resume for both gzip and plain files, with proper initialization of
stream_offset,exclude_bytes, andskipping_mode.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/in_tail/tail_db.c(11 hunks)plugins/in_tail/tail_file.c(9 hunks)plugins/in_tail/tail_file_internal.h(1 hunks)plugins/in_tail/tail_fs_inotify.c(1 hunks)plugins/in_tail/tail_fs_stat.c(1 hunks)plugins/in_tail/tail_sql.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/in_tail/tail_file_internal.h
- plugins/in_tail/tail_fs_stat.c
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
📚 Learning: 2025-10-23T07:43:16.216Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.
Applied to files:
plugins/in_tail/tail_db.cplugins/in_tail/tail_file.cplugins/in_tail/tail_fs_inotify.c
📚 Learning: 2025-09-22T15:59:55.794Z
Learnt from: nicknezis
Repo: fluent/fluent-bit PR: 10882
File: plugins/out_http/http.c:112-116
Timestamp: 2025-09-22T15:59:55.794Z
Learning: When users consider bug fixes out of scope for their focused PRs, it's appropriate to create separate GitHub issues to track those concerns rather than expanding the current PR scope.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
plugins/in_tail/tail_file.c
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
plugins/in_tail/tail_file.c
🧬 Code graph analysis (2)
plugins/in_tail/tail_db.c (2)
src/flb_sqldb.c (2)
flb_sqldb_query(151-166)flb_sqldb_close(129-149)plugins/in_tail/tail_file.h (1)
flb_tail_target_file_name_cmp(64-116)
plugins/in_tail/tail_file.c (1)
plugins/in_tail/tail_db.c (1)
flb_tail_db_file_offset(366-399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
🔇 Additional comments (16)
plugins/in_tail/tail_db.c (6)
28-34: LGTM!The column-existence callback is correctly implemented. It properly sets the flag when a row is returned from the pragma query, enabling reliable column detection for schema migration.
61-108: LGTM!The migration logic correctly uses
pragma_table_infoto detect column existence, distinguishing real errors from missing columns. The pattern is consistent for bothskipandanchorcolumns, with proper error handling and logging.
318-363: LGTM!The function correctly restores
skip_bytesandanchor_offsetfrom the database and initializes the runtime-onlyexclude_bytesandskipping_modefields based on the persisted skip state. The logic properly handles both cases (skip > 0 and skip == 0).
424-446: LGTM!Resetting
db_idtoFLB_TAIL_DB_ID_NONEafter deletion prevents accidental resurrection of deleted DB entries whenflb_tail_db_file_offsetis called later. This is an important safeguard for data integrity.
365-398: The parameter binding order inflb_tail_db_file_offsetcorrectly matches theSQL_UPDATE_OFFSETstatement: offset (parameter 1), skip (parameter 2), anchor (parameter 3), and id (parameter 4).
182-221: Column indices for skip and anchor are correct.The code correctly reads
skipfrom column index 6 andanchorfrom column index 7, matching the table schema defined in SQL_CREATE_FILES. WhenSELECT *is executed, SQLite returns columns in the order they appear in the table definition: id (0), name (1), offset (2), inode (3), created (4), rotated (5), skip (6), and anchor (7).plugins/in_tail/tail_fs_inotify.c (1)
259-281: LGTM!The truncation handler correctly resets all gzip resume state fields (
anchor_offset,skip_bytes,exclude_bytes,skipping_mode) alongside the file offset and buffer, ensuring a clean state after truncation. This initialization is consistent with similar handling intail_fs_stat.candadjust_countersintail_file.c.plugins/in_tail/tail_sql.h (3)
30-40: LGTM!The table schema correctly adds
skipandanchorcolumns withINTEGER DEFAULT 0, ensuring backward compatibility with existing databases. Column indices (skip=6, anchor=7) align with the reads indb_file_exists.
45-47: LGTM!The
SQL_INSERT_FILEstatement correctly includesskipandanchorin both the column list and VALUES clause. The parameter order matches the binding sequence indb_file_insert(tail_db.c lines 258-263).
52-53: LGTM!The
SQL_UPDATE_OFFSETstatement correctly updates all three position-tracking fields (offset,skip,anchor) atomically. The parameter order matches the binding sequence inflb_tail_db_file_offset(tail_db.c lines 372-375).plugins/in_tail/tail_file.c (6)
47-47: LGTM!Including
flb_compression.his appropriate for the gzip decompression functionality added in this PR.
1328-1338: LGTM!All gzip resume fields (
anchor_offset,skip_bytes,exclude_bytes,skipping_mode) are correctly initialized to zero/false when a new file is appended, ensuring clean initial state.
1509-1548: LGTM!The offset rewind logic correctly handles buffered data on shutdown:
- For non-compressed files: rewinds the offset by
buf_len(with bounds checking) so unprocessed data is re-read on restart.- For compressed files: logs a warning explaining that accurate rewinding is infeasible with streaming decompression.
- DB updates are properly guarded by
db_id > FLB_TAIL_DB_ID_NONEto prevent resurrecting deleted entries.This addresses the data loss issue described in the PR objectives.
1657-1679: LGTM!The truncation handler in
adjust_counterscorrectly resets all gzip resume state fields (anchor_offset,skip_bytes,exclude_bytes,skipping_mode) when a file is truncated, consistent with the inotify and stat-based truncation handlers.
1871-1891: LGTM!The skip logic during decompression correctly handles resuming from a mid-stream position:
- When
exclude_bytes >= decompressed_data_length, all newly decompressed data is skipped andexclude_bytesis decremented.- When
exclude_bytes < decompressed_data_length, the remaining bytes are shifted to the buffer start usingmemmove, andskipping_modeis cleared.This enables accurate gzip resume from the persisted
skip_bytesposition.
1932-1953: LGTM!The gzip member boundary handling correctly tracks position using the anchor/skip pattern:
skip_bytesis incremented byprocessed_bytesto track position within the current member.- When a member completes (decompressor transitions to
EXPECTING_HEADERstate and all buffers are empty),anchor_offsetadvances to the current raw file position andskip_bytesresets to 0.This enables resume at member boundaries for multi-member gzip files, addressing the data loss issue for compressed inputs mentioned in the PR objectives.
|
Thanks for the detailed analysis. I fully agree with your opinion. Although there is a risk of duplication in case of abrupt shutdown, it is technically difficult to solve completely, and duplication is definitely better than data loss. Also, regarding your question about the logs, I have changed the log level of the database migration messages to debug. This ensures they are not too noisy during normal operation while still being available for troubleshooting if needed. Finally, should I add a note about the limitation (potential duplication on crash) to the documentation? I think adding a warning/note to the 'Database file' section would be helpful for users. Let me know what you think! |
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
I found a small nitpick issue but your PR is not following our coding style.
So, we need to follow the style of defining variables.
I suppose that we need to Note annotations to depicts the possibility for database corruptions in the official documentation which should be corresponding PR for documentation. This could be corner cases but it's technically hard to solve cleanly. |
Got it! I'll create a separate PR for the documentation. |
|
we need conflict to be resolved |
Hi @edsiper, I've resolved the conflicts. |
|
Rebased onto latest master to resolve conflicts with #11394 (in_tail: harden restart restore). Changes in this rebase:
|
Code reviewFound 1 issue:
Generated with Claude Code - If this code review was useful, please react with a thumbs up. Otherwise, react with a thumbs down. |
|
The issue from the previous code review has been addressed in the latest force push:
|
- Added test for offset rewind on shutdown (prevent DB resurrection) - Added gzip tests: resume loss, inotify append, rotation, multi resume Signed-off-by: jinyong.choi <inimax801@gmail.com>
Unprocessed buffered data is lost on shutdown because the file offset is saved ahead of processing. This patch rewinds the offset by the remaining buffer length on exit, ensuring data is re-read on restart. Also, to prevent resurrecting deleted files in the DB, db_id is reset to 0 upon deletion. The offset update logic now checks db_id > 0 to ensure only active files are updated. Signed-off-by: jinyong.choi <inimax801@gmail.com>
Add skip and anchor columns to the tail DB schema to track gzip decompression state across restarts. On resume, the anchor_offset determines the raw file seek position while skip_bytes controls how many decompressed bytes to discard before emitting records. Includes DB schema migration for older databases, integration with the offset_marker validation from upstream, and proper reset of skip/anchor state when stale DB positions are detected. Signed-off-by: jinyong.choi <inimax801@gmail.com>
Adapt the db_offset_rewind_on_shutdown test to work with the upstream flb_tail_file_db_offset() auto-rewind behavior. The DB offset now automatically accounts for buffered incomplete lines, so the test no longer polls for offset advancement and instead verifies data is re-read on restart. Signed-off-by: jinyong.choi <inimax801@gmail.com>
|
Additional cleanup in the latest force push:
This addresses the inconsistency flagged earlier -- both migration groups now share the same pattern as the upstream `offset_marker` migration. |
Unprocessed data in the internal buffer is discarded when Fluent Bit stops, causing data loss because the DB offset is already advanced.
This patch fixes the issue by rewinding the file offset by the remaining buffer length on exit, ensuring data is re-read on restart.
For compressed gzip files, a separate issue caused data duplication after restart because skip_bytes was incorrectly decremented during runtime. A new field 'exclude_bytes' is introduced as a runtime-only counter, preserving skip_bytes for correct DB persistence.
Additionally, this patch prevents resurrecting deleted file entries in the DB by resetting db_id to 0 upon deletion and checking it before updating the offset.
The SQLite schema is updated to include 'anchor_offset' and 'skip_bytes' columns. On upgrade from older versions, these columns are automatically added via ALTER TABLE if they do not exist.
Closes #11265
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.