Skip to content

Conversation

@daipom
Copy link
Contributor

@daipom daipom commented Apr 23, 2025

Which issue(s) this PR fixes:
Backport #4877
Fixes #4873

What this PR does / why we need it:
Before invoke @num_errors_metrics.inc, the does retries correctly when #try_write fails test will be failed if thread will be switched at

def update_retry_state(chunk_id, using_secondary, error = nil)
@retry_mutex.synchronize do
@num_errors_metrics.inc

It is easy to reproduce by adding sleep just before @num_errors_metrics.inc, like:

diff --git a/lib/fluent/plugin/output.rb b/lib/fluent/plugin/output.rb
index a987937b..d97d8ec6 100644
--- a/lib/fluent/plugin/output.rb
+++ b/lib/fluent/plugin/output.rb
@@ -1312,6 +1312,7 @@ def check_slow_flush(start)

       def update_retry_state(chunk_id, using_secondary, error = nil)
         @retry_mutex.synchronize do
+          sleep 0.1
           @num_errors_metrics.inc
           chunk_id_hex = dump_unique_id_hex(chunk_id)

This patch will wait until @num_errors_metrics.inc is invoked.

Docs Changes:

Release Note:

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:

Docs Changes:

Release Note:

**Which issue(s) this PR fixes**:
Fixes #4873

**What this PR does / why we need it**:
Before invoke `@num_errors_metrics.inc`, the `does retries correctly
when #try_write fails` test will be failed if thread will be switched at
https://github.com/fluent/fluentd/blob/f34a2531fcfc4e8958f665301d48558de200aa3a/lib/fluent/plugin/output.rb#L1313-L1315

It is easy to reproduce by adding `sleep` just before
`@num_errors_metrics.inc`, like:

```diff
diff --git a/lib/fluent/plugin/output.rb b/lib/fluent/plugin/output.rb
index a987937..d97d8ec6 100644
--- a/lib/fluent/plugin/output.rb
+++ b/lib/fluent/plugin/output.rb
@@ -1312,6 +1312,7 @@ def check_slow_flush(start)

       def update_retry_state(chunk_id, using_secondary, error = nil)
         @retry_mutex.synchronize do
+          sleep 0.1
           @num_errors_metrics.inc
           chunk_id_hex = dump_unique_id_hex(chunk_id)

```

This patch will wait until `@num_errors_metrics.inc` is invoked.

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <[email protected]>
Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom daipom added this to the v1.16.8 milestone Apr 23, 2025
Copy link
Contributor

@Watson1978 Watson1978 left a comment

Choose a reason for hiding this comment

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

👍🏻

@daipom daipom merged commit 854213b into v1.16 Apr 23, 2025
19 checks passed
@daipom daipom deleted the backport-pr4877 branch April 23, 2025 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants