Skip to content

engine_dispatch: re-schedule retry when chunk content cannot be read - #12260

Open
ku524 wants to merge 2 commits into
fluent:cosmo0920-plug-stale-failed-tasks-on-dispatchfrom
ku524:fix/dispatch-retry-reschedule-on-read-failure
Open

engine_dispatch: re-schedule retry when chunk content cannot be read#12260
ku524 wants to merge 2 commits into
fluent:cosmo0920-plug-stale-failed-tasks-on-dispatchfrom
ku524:fix/dispatch-retry-reschedule-on-read-failure

Conversation

@ku524

@ku524 ku524 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Stacked on #12254, so the base branch is cosmo0920-plug-stale-failed-tasks-on-dispatch
and the diff here is only my two commits.

#12254 fixes the task leak reported in #12252, and I verified that it does. This PR keeps
that fix and changes one thing: when the chunk content cannot be read, spend a delivery
attempt on it before giving up, instead of dropping on the first failure. With the default
retry_limit of 1 the behaviour is identical to #12254, so the change only takes effect
when the user has explicitly raised their retry budget.

Builds on #12254, which closes #12252.

Problem

Releasing the task on the first read failure deletes the chunk file. The call chain is
flb_task_users_release() -> flb_task_destroy(task, FLB_TRUE) ->
flb_input_chunk_destroy(ic, FLB_TRUE) -> cio_chunk_close(ch, CIO_TRUE) ->
cio_file_native_delete().

flb_input_chunk_flush() returns NULL when cio_chunk_up() or cio_chunk_get_content()
fails. Those failures are typically transient under memory or descriptor pressure rather
than permanent corruption, so the records would very likely have been delivered by a later
attempt. The leak being fixed here is bad, but it does at least keep the records on disk
for a restart to flush, which is the behaviour I measured while investigating #12252.

Summary

The give-up path is unchanged from #12254, including its drop accounting. What is new is
that a read failure first consumes a delivery attempt and re-schedules through
flb_task_retry_reschedule(), which is what the failure case a few lines above already
does for the same reason. On both paths the task is never left with no users and no
retries, so the leak stays fixed.

Reusing Retry_Limit keeps the decision where the user already expressed it, so no new
configuration option or constant is introduced.

Retry_Limit Behaviour on a read failure
1 (default) Drops immediately, same as #12254
no_retries (0) Drops immediately
5 Re-schedules up to 4 times, then drops with accounting
no_limits Re-schedules until the chunk becomes readable

Verification

The three tests added by #12254 pass unchanged, so this does not alter the cases they
cover. flb-it-task_map passes as well. I added one test for the re-scheduled path that
asserts the task keeps its task-map slot, its chunk and a pending retry, and that no drop
accounting is recorded.

End to end on a build with 8 dummy inputs and 8 forward outputs, 20 read failures
injected with gdb produced 20 re-schedules and 0 drops, and the busy chunk gauge settled
back to 0 over three hours. The same injection against unpatched v5.1.0 pinned 20 chunks
permanently. Throughput was 3998 records per second against 3971 for the unpatched build,
and no crash, assertion or signal appeared in roughly three hours of continuous operation.

Trade-offs

A read failure now consumes one delivery attempt, so retry->attempts covers both delivery
and read failures. It never drops earlier than #12254 does.

A permanently unreadable chunk under no_limits stays on disk until
storage.total_limit_size evicts it.


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.

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

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

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

ku524 added 2 commits August 11, 2026 15:06
flb_input_chunk_flush() returning NULL is usually transient, so dropping
the retry discards records a later attempt could still deliver. Spend a
delivery attempt on it instead and only give up once the configured
retry limit is reached, where the existing accounting already applies.

Behaviour is unchanged with the default retry_limit of 1: the first read
failure still drops immediately.

Signed-off-by: ku524 <yeonjuyeong@gmail.com>
Asserts that with retry budget left the task keeps its task-map slot,
its chunk and a pending retry, and that no drop accounting is recorded.

Signed-off-by: ku524 <yeonjuyeong@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9a9c1631-be67-4112-8893-1cb145c80fdb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

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