Skip to content

tests: synchronize S3 runtime chunk setup - #12442

Open
cosmo0920 wants to merge 1 commit into
masterfrom
cosmo0920-plug-failures-on-out_s3-testing
Open

cosmo0920 wants to merge 1 commit into
masterfrom
cosmo0920-plug-failures-on-out_s3-testing

Conversation

@cosmo0920

@cosmo0920 cosmo0920 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Patched tests/runtime/out_s3.c line 265.

All three logs show SIGSEGVs in two tests sharing an unsafe pattern: a disk file becomes visible before its in-memory S3 object is ready. The tests accessed that object without the worker’s mutex.

Both now wait for registration and access the object under the mutex. Timeout paths fail cleanly instead of dereferencing a missing object.

Verification passed:

  • Full build and ctest --test-dir build -R '^flb-rt-out_s3$' --output-on-failure.
  • Five repeated runs of both affected cases.
  • Both cases under Valgrind: zero errors or lost bytes.
  • S3 integration: 19 passed, 6 skipped.
  • Strict Valgrind retry integration: 7 passed.

Exact integration commands used:

PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python tests/integration/.venv/bin/python -m pytest tests/integration/scenarios/out_s3/tests/test_out_s3_001.py -q

VALGRIND=1 VALGRIND_STRICT=1 PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python tests/integration/.venv/bin/python -m pytest tests/integration/scenarios/out_s3/tests/test_out_s3_001.py -k retry -q

The six skips cover unsupported columnar formats and OTLP metrics/traces uploads.


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:

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

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

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

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

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

Summary by CodeRabbit

  • Tests
    • Improved coverage for S3 file creation timing and ordered upload retry scenarios.
    • Updated test setup and cleanup paths to provide more reliable validation of shared uploads and exhausted chunk conditions.
    • Preserved existing behavior across other S3 test cases while standardizing file lookup handling.

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 6ee5870d-a4cd-4eb9-adf6-29c9cba23c94

📥 Commits

Reviewing files that changed from the base of the PR and between a48daa6 and c383eed.

📒 Files selected for processing (1)
  • tests/runtime/out_s3.c

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The S3 runtime test helper now optionally copies the located file name into a caller buffer. Two ordered-upload tests use this helper and add cleanup paths for early failures.

Changes

S3 runtime test updates

Layer / File(s) Summary
Extend S3 file wait helper
tests/runtime/out_s3.c
wait_for_s3_file_create_time optionally copies the located file name. Existing callers pass no output buffer.
Migrate ordered upload tests
tests/runtime/out_s3.c
The ordered-upload tests use the helper for file and create-time setup. Early failures now jump to cleanup paths. The shared-upload test retains its distinct-chunk check.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: singholt

Merge Risk: ⚪ Minimal · up to c383e

The S3 runtime test synchronization update has no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: synchronizing S3 runtime chunk setup in the affected tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

❤️ Share

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

@cosmo0920
cosmo0920 marked this pull request as ready for review September 18, 2026 09:27
@cosmo0920
cosmo0920 requested a review from a team as a code owner September 18, 2026 09:27
@cosmo0920 cosmo0920 added this to the Fluent Bit v5.1.3 milestone Sep 18, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-18T09:28:42.177896Z c383eed Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant