Skip to content

Conversation

@daipom
Copy link
Contributor

@daipom daipom commented Mar 5, 2025

Which issue(s) this PR fixes:
None.

What this PR does / why we need it:
Fix occasional test failure: https://github.com/fluent/fluentd/actions/runs/13650069134/job/38156420830?pr=4846#step:6:4985

The max size of the buffer queue is 1 by default.
So, try_flush will be called with flush_thread_interval, not flush_thread_burst_interval.

The previous test code assumes that the interval is 0.1s, but it was actually 1s.
This would be the cause of the occasional failure.

test: #write is called per value combination of variables, per flush_interval & chunk sizes, and buffer chunk is purged(BufferedOutputTest::buffered output feature with variables):
        assert{ @i.buffer.stage.size == 0 }
                |  |      |     |    |
                |  |      |     |    false
                |  |      |     1
                |  |      {...}
                |  #<...>
                #<...>
/Users/runner/work/fluentd/fluentd/test/plugin/test_output_as_buffered.rb:1459:in `block (2 levels) in <class:BufferedOutputTest>'
     1456:       @i.enqueue_thread_wait
     1457:       @i.flush_thread_wakeup
     1458:
  => 1459:       assert{ @i.buffer.stage.size == 0 }
     1460:
     1461:       waiting(4) do
     1462:         Thread.pass until @i.write_count > 1

Docs Changes:
Not needed.

Release Note:
Not needed.

@daipom daipom added CI Test/CI issues backport to v1.16 We will backport this fix to the LTS branch labels Mar 5, 2025
@daipom daipom added this to the v1.19.0 milestone Mar 5, 2025
The max size of queue is `1` by default.
So, `try_flush` will be called with `flush_thread_interval`, not
`flush_thread_burst_interval`.

The previous test code assumes that the interval is 0.1s, but
it was actually 1s.
This would be the cause of the occasional failure.

    test: #write is called per value combination of variables, per flush_interval & chunk sizes, and buffer chunk is purged(BufferedOutputTest::buffered output feature with variables):
            assert{ @i.buffer.stage.size == 0 }
                    |  |      |     |    |
                    |  |      |     |    false
                    |  |      |     1
                    |  |      {...}
                    |  #<...>
                    #<...>
    /Users/runner/work/fluentd/fluentd/test/plugin/test_output_as_buffered.rb:1459:in `block (2 levels) in <class:BufferedOutputTest>'
         1456:       @i.enqueue_thread_wait
         1457:       @i.flush_thread_wakeup
         1458:
      => 1459:       assert{ @i.buffer.stage.size == 0 }
         1460:
         1461:       waiting(4) do
         1462:         Thread.pass until @i.write_count > 1

Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom daipom force-pushed the test-fix-unstable-buffer-test branch from 0cbe64a to f8bb618 Compare March 5, 2025 07:12
@daipom daipom requested a review from Watson1978 March 5, 2025 09:10
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 1fa4f94 into fluent:master Mar 6, 2025
10 checks passed
daipom added a commit that referenced this pull request Mar 6, 2025
**Which issue(s) this PR fixes**:
None.

**What this PR does / why we need it**:
Fix occasional test failure:
https://github.com/fluent/fluentd/actions/runs/13650069134/job/38156420830?pr=4846#step:6:4985

The max size of the buffer queue is `1` by default.
So, `try_flush` will be called with `flush_thread_interval`, not
`flush_thread_burst_interval`.

The previous test code assumes that the interval is 0.1s, but it was
actually 1s.
This would be the cause of the occasional failure.

test: #write is called per value combination of variables, per
flush_interval & chunk sizes, and buffer chunk is
purged(BufferedOutputTest::buffered output feature with variables):
            assert{ @i.buffer.stage.size == 0 }
                    |  |      |     |    |
                    |  |      |     |    false
                    |  |      |     1
                    |  |      {...}
                    |  #<...>
                    #<...>

/Users/runner/work/fluentd/fluentd/test/plugin/test_output_as_buffered.rb:1459:in
`block (2 levels) in <class:BufferedOutputTest>'
         1456:       @i.enqueue_thread_wait
         1457:       @i.flush_thread_wakeup
         1458:
      => 1459:       assert{ @i.buffer.stage.size == 0 }
         1460:
         1461:       waiting(4) do
         1462:         Thread.pass until @i.write_count > 1

**Docs Changes**:
Not needed.

**Release Note**:
Not needed.

Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom daipom deleted the test-fix-unstable-buffer-test branch March 6, 2025 02:57
daipom added a commit that referenced this pull request Mar 6, 2025
**Which issue(s) this PR fixes**:
* Backport #4849

**What this PR does / why we need it**:
Fix occasional test failure:

https://github.com/fluent/fluentd/actions/runs/13650069134/job/38156420830?pr=4846#step:6:4985

The max size of the buffer queue is `1` by default. So, `try_flush` will
be called with `flush_thread_interval`, not
`flush_thread_burst_interval`.

The previous test code assumes that the interval is 0.1s, but it was
actually 1s.
This would be the cause of the occasional failure.

test: #write is called per value combination of variables, per
flush_interval & chunk sizes, and buffer chunk is
    purged(BufferedOutputTest::buffered output feature with variables):
                assert{ @i.buffer.stage.size == 0 }
                        |  |      |     |    |
                        |  |      |     |    false
                        |  |      |     1
                        |  |      {...}
                        |  #<...>
                        #<...>
    

/Users/runner/work/fluentd/fluentd/test/plugin/test_output_as_buffered.rb:1459:in
`block (2 levels) in <class:BufferedOutputTest>'
             1456:       @i.enqueue_thread_wait
             1457:       @i.flush_thread_wakeup
             1458:
          => 1459:       assert{ @i.buffer.stage.size == 0 }
             1460:
             1461:       waiting(4) do
             1462:         Thread.pass until @i.write_count > 1

**Docs Changes**:
Not needed.

**Release Note**:
Not needed.

Signed-off-by: Daijiro Fukuda <[email protected]>
@kenhys kenhys added the backported "backport to LTS" is done label Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport to v1.16 We will backport this fix to the LTS branch backported "backport to LTS" is done CI Test/CI issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants