Skip to content

Backport pr4686 - #4792

Closed
kenhys wants to merge 45 commits into
masterfrom
backport-pr4686
Closed

Backport pr4686#4792
kenhys wants to merge 45 commits into
masterfrom
backport-pr4686

Conversation

@kenhys

@kenhys kenhys commented Jan 29, 2025

Copy link
Copy Markdown
Contributor

Which issue(s) this PR fixes:

Backport #4686

What this PR does / why we need it:

Fixes an issue where an inappropriate protocol was specified in the unused_port method.
See #4674 (comment)

Docs Changes:

Release Note:

mrudrego and others added 30 commits October 31, 2023 13:44
Fix for rotate_age where Fluentd passes as Symbol while Ruby Logger expects String

---------

Signed-off-by: mrudrego <maheshmr.gowda@gmail.com>
Signed-off-by: mrudrego <mahesh.rudregowda@nokia.com>
Fix #3614

Although known stall issues of in_tail on `follow_inode` case are fixed
in v1.16.2, it has still a similar problem on `!follow_inode` case.

In this case, a tail watcher is possible to mark the position entry as
`unwatched` if it's tansitioned to `rotate_wait` state by
`refresh_watcher` even if another newer tail watcher is managing it.
It's hard to occur in usual because `stat_watcher` will be called
immediately after the file is changed  while `refresh_wather` is called
every 60 seconds by default. However, there is a rare possibility that
this order might be swapped especillay if in_tail is busy on processing
large amount of logs. Because in_tail is single threadied, event queues
such as timers or inotify will be stucked in this case.

There is no such problem on `follow_inode` case because position entries
are always marked as `unwatched` before entering `rotate_wait` state.

---------

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Co-authored-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Alex Doolittle <alex_doolittle@uhaul.com>
For `refresh_watcher`, if an exisiting TailWatcher already
follows a target path with the different inode, it means that
the TailWatcher following the rotated file still exists.

In this case, `refresh_watcher` can't start the new TailWatcher
for the new current file.
So, we should output a warning log in order to prevent silent
collection stops, such as #4327.

The similar warning may work for follow_inodes too.
Just limiting the case to suppress the impact to existing logic.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
…v1.16) (#4363)

It fixes following error when many `chunk bytes limit exceeds` errors
are occurred:
```
2020-07-28 14:59:26 +0000 [warn]: #0 emit transaction failed: error_class=IOError error="closed stream" location="/fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer/file_chunk.rb:82:in `pos'" tag="cafiscode-eks-cluster.default"
  2020-07-28 14:59:26 +0000 [warn]: #0 /fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer/file_chunk.rb:82:in `pos'
  2020-07-28 14:59:26 +0000 [warn]: #0 /fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer/file_chunk.rb:82:in `rollback'
  2020-07-28 14:59:26 +0000 [warn]: #0 /fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer.rb:339:in `rescue in block in write'
  2020-07-28 14:59:26 +0000 [warn]: #0 /fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer.rb:332:in `block in write'
  2020-07-28 14:59:26 +0000 [warn]: #0 /fluentd/vendor/bundle/ruby/2.6.0/gems/fluentd-1.11.1/lib/fluent/plugin/buffer.rb:331:in `each'
  ...
```

Fix #3089

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Signed-off-by: Christian Norbert Menges <christian.norbert.menges@sap.com>
Backport (v1.16): buffer: Avoid calling dump_unique_id_hex if log level is not trace
After a tail watcher transitions to `rotate_wait` state, the
`rotate_wait` timer is no longer managed by in_tail, it might cause
unexpected behaviour. e.g.)

* It's never unwatched when shutdown occurs before `rotate_wait` passed.
* Needless `rotate_wait` timers are executed when it detects more
  rotations.

This patch fixes such unexpected behaviour.

Note: The comment about `detach_watcher` was added in 76f246a.
At that time, closing was done by event-loop.
Now, the situation is completely different, so it should be removed.

---------

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Co-authored-by: Daijiro Fukuda <fukuda@clear-code.com>
Backport (v1.16): in_tail: Manage tail watchers that are `rorate_wait` state too (#4334)
Refine CI settings:

* Unify CI settings for each platforms to one file
* Don't run CI for Ruby head on each push/pull_request event
    * Because Ruby head is continually changed and not released
      yet, running CI one shot isn't so meaningful.
    * Instead run it regularly on every Sunday.
* Add TESTOPT=-v to all platfroms
    * It's useful to investigate when a test is stalled.
* Remove a hack for Ruby 3.0 on Windows
    * It seems no longer needed.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
because Ruby head is never supported on v1.16, and it seems wrong
to use schedule trigger outside of the default branch (master).

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Takuro Ashie <ashie@clear-code.com>
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
…ables

Make easy to understand what these tests do.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Behaviour of `IO#set_encoding` has been changed as of Ruby 3.3.
We don't yet determine how to solve this issue, it might be better to
address in Ruby.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Backported form 2ea28c9

---

It is not harmful because GitHub allows both of file extensions (.yaml
and .yml), but it may be better to unify it for consistency.

.yml is used in https://docs.github.com/en/actions/learn-github-actions/understanding-github-actions

Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Backport (v1.16):  Fix or suppress failed tests on Ruby 3.3 (#4375)
Backported from 5d18f35

* Avoid calling `instance_start` in duplicate.
* Avoid not calling `instance_shutdown`.

To fix the following error, which sometimes occur.
I'm not sure this actually fixes it, but, at least, we should fix the
points above.

    Error: test: Create new connection per send_data(ForwardOutputTest): ArgumentError: expected loop to be an instance of Coolio::Loop, not nil
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/io.rb:35:in `attach'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/io.rb:35:in `attach'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/socket.rb:39:in `attach'
    (eval):7:in `attach'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/server.rb:40:in `on_connection'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/listener.rb:65:in `on_readable'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/loop.rb:88:in `run_once'
    C:/hostedtoolcache/windows/Ruby/3.2.3/x64/lib/ruby/gems/3.2.0/gems/cool.io-1.8.0/lib/cool.io/loop.rb:88:in `run'
    D:/a/fluentd/fluentd/lib/fluent/plugin_helper/event_loop.rb:93:in `block in start'
    D:/a/fluentd/fluentd/lib/fluent/plugin_helper/thread.rb:78:in `block in thread_create'

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Takuro Ashie <ashie@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Kentaro Hayashi <hayashi@clear-code.com>
Backported from 13a5199.

After 95438b2 (#4342), there is a
section where chunks do not have a lock in `write_step_by_step()`.

`write_step_by_step()` must ensure their locks until passing them
to the block.
Otherwise, race condition can occur and it can cause emit error
by IOError.
Example of warning messages of emit error:

    [warn]: #0 emit transaction failed: error_class=IOError error="closed stream" location=...
    [warn]: #0 send an error event stream to @error: error_class=IOError error="closed stream" location=...

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
TESTOPTS=-v shows running test and the result.
If something weird happen, it show error immediately.

As rake task executes many tests, so it may be better to
delay showing test details later.

NOTE: --progress-style=fault-only helps you to focus on
failure, but it is inconvenient when test case has stalled.

Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Co-authored-by: Kentaro Hayashi <hayashi@clear-code.com>
console gem v1.24 and v1.25 has some specification changes that
influence Fluentd.

Since they have nothing to do with vulnerability, we should keep
the version on v1.16 stable branch.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Backported from 74b2e3d. (#4502) 

Signed-off-by: Shingo Nakayama <nkymsng@akane.waseda.jp>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Shingo Nakayama <152375941+Shingo-Nakayama@users.noreply.github.com>
Backported from 57f821b.

---

timecop 0.9.9 supports `Process.clock_gettime`.
This breaks specifications of `process_extenstion` of Fluentd and
`Fluent::Clock`.

`Fluent::Clock` uses `CLOCK_MONOTONIC_RAW` if possible and it does not
be affected.
However, `CLOCK_MONOTONIC_RAW` is not available on Windows, so the
impact on tests on Windows is very significant.

For now, we should avoid this effect by fixing the version.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Backported from 236d87d.

---

This is because `@log_level` is invalid for YAML.
Instead, we should interpret $log_level as `@log_level` on YAML parser.

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Backport(v1.16): config: yaml_parser: Handle $log_level element for special case (#4482)
Backported from 3b27984.

---

`Tempfile#binmode` returns `File` object, not own `Tempfile`
object.
So, GC will cause its finalizer and the file can be closed during
the test.
This is the cause why these tests sometimes fail by
`IOError: closed stream`.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
daipom and others added 15 commits August 16, 2024 14:57
Backported from 891ce71.

---

If Oj is not installed, LoadError with the empty message is raised.
So, the current condition `/\boj\z/.match?(ex.message)` does not work
and the following meaningless warning is displayed.

    {datetime} [warn]: #x {id} LoadError

After this fix, the log message will be:

    {datetime} [info]: #x {id} Oj is not installed, and failing back to
    Yajl for json parser

Refactor "rescue" logic because this falling back feature is
currently only for "oj" (LoadError can not occur for "json" and
"yajl").

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Co-authored-by: Takuro Ashie <ashie@clear-code.com>
Backported from fe5843f

---

This option is explained as "add plugin directory".
However, since v1.16.0, the behavior has changed to overwrite the
default value unintentionally.
(PR: #4064, commit: 41678bf).

We should revert it to the original behavior.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Because it does not fix the bug itself.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
For updating documentation or release task, running GitHub Actions
is waste of resource.

https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore

Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Backported from a2b935a (#4671).

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Co-authored-by: Watson <fujita@clear-code.com>
**Which issue(s) this PR fixes**: 

Fixes #4674

**What this PR does / why we need it**: 

It obtains unused port number for TCP by unused_port method and the
number has been used in UDP.
And that number may be already used by UDP sockets.

This patch will obtain and use unused ports appropriately for each
protocol.

Backported from #4675

**Docs Changes**:

**Release Note**:

Signed-off-by: Watson <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Co-authored-by: Watson <fujita@clear-code.com>
#4676) (#4789)

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

**What this PR does / why we need it**: 

On Windows, `message_length_limit` test always take 300s for execution.

```
UdpInputTest:
  test:  configure w/o parse section:			.: (0.002351)
  test: configure[ipv4]:				.: (0.002723)
  test: configure[ipv6]:				.: (0.002602)
  test: message size with format[none]:			.: (1.029781)
  test: message size with format[json]:			.: (1.113799)
  test: message size with format[regexp]:		.: (1.110006)
  test: message_length_limit:				.: (300.538596)
```

The 300 sec comes from
https://github.com/fluent/fluentd/blob/a2b935ae2bc4b4d43e5adddbec01092ea4228b9e/lib/fluent/test/driver/base.rb#L36,
and it always times out in Windows.

This patch set a short timeout to reduce test execution time on Windows.

Backported from #4676

**Docs Changes**:

**Release Note**:

Signed-off-by: Watson <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Co-authored-by: Watson <fujita@clear-code.com>
…limit test (#4682) (#4790)

**Which issue(s) this PR fixes**: 

Backport #4682 

**What this PR does / why we need it**: 

Although it's improved in #4676, it still takes 5 seconds on windows. It
can be reduced more by sending data that doesn't exceed the limit.

```
  test: message_length_limit:                           .: (1.104855)
```

**Docs Changes**:

**Release Note**:

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Co-authored-by: Takuro Ashie <ashie@clear-code.com>
…eout setting (#4685) (#4791)

**Which issue(s) this PR fixes**: 

Backport #4685 

**What this PR does / why we need it**: 

Seems that timeout setting is short in ack_response_timeout.
Seems that It may take some time to receive a ACK response
so the process in ack handler has expired and the node is disabled.

This PR will remove unnecessary ack_response_timeout settings for the
test

**Docs Changes**:

**Release Note**:

Signed-off-by: Watson <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Co-authored-by: Watson <fujita@clear-code.com>
Signed-off-by: Watson <fujita@clear-code.com>
Co-authored-by: Daijiro Fukuda <fukuda@clear-code.com>
@kenhys kenhys closed this Jan 29, 2025
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.

8 participants