Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: recreate cio_stream if storage type is different(#8259) #8290

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

nokute78
Copy link
Collaborator

@nokute78 nokute78 commented Dec 16, 2023

Fixes #8259

cio_load creates storage stream if directory exists before creating input chunks.
It prevents to create memory stream by input plugins.
This patch is to recreate stream if the stream type is different.


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.

Configuration

[SERVICE]
    storage.path data

[INPUT]
    Name dummy
    Tag dummy

[FILTER]
    Name rewrite_tag
    Match dummy
    Rule $message dummy rewritten false
    Emitter_Name rewriter
    Emitter_Storage.type memory

[OUTPUT]
    Name stdout
    Match rewritten

Debug/Valgrind output

  1. mkdir -p "$(pwd)/data/emitter.2"
  2. fluent-bit -c a.conf
  3. No files are created under data/emitter.2 while running fluent-bit.
$ valgrind --leak-check=full bin/fluent-bit -c 8259.conf 
==78668== Memcheck, a memory error detector
==78668== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==78668== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==78668== Command: bin/fluent-bit -c 8259.conf
==78668== 
Fluent Bit v2.2.1
* Copyright (C) 2015-2023 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2023/12/16 12:55:00] [ info] [fluent bit] version=2.2.1, commit=2389f44ee4, pid=78668
[2023/12/16 12:55:00] [ info] [storage] ver=1.2.0, type=memory+filesystem, sync=normal, checksum=off, max_chunks_up=128
[2023/12/16 12:55:00] [ info] [storage] backlog input plugin: storage_backlog.1
[2023/12/16 12:55:00] [ info] [cmetrics] version=0.6.5
[2023/12/16 12:55:00] [ info] [ctraces ] version=0.3.1
[2023/12/16 12:55:00] [ info] [input:dummy:dummy.0] initializing
[2023/12/16 12:55:00] [ info] [input:dummy:dummy.0] storage_strategy='memory' (memory only)
[2023/12/16 12:55:00] [ info] [input:storage_backlog:storage_backlog.1] initializing
[2023/12/16 12:55:00] [ info] [input:storage_backlog:storage_backlog.1] storage_strategy='memory' (memory only)
[2023/12/16 12:55:00] [ info] [input:storage_backlog:storage_backlog.1] queue memory limit: 95.4M
[2023/12/16 12:55:00] [ info] [input:emitter:rewriter] initializing
[2023/12/16 12:55:00] [ info] [input:emitter:rewriter] storage_strategy='memory' (memory only)
[2023/12/16 12:55:00] [ info] [storage] storage type mismatch. input type='memory' (memory only)
[2023/12/16 12:55:00] [ info] [storage] re-create stream
[2023/12/16 12:55:01] [ info] [sp] stream processor started
[2023/12/16 12:55:01] [ info] [output:stdout:stdout.0] worker #0 started
[0] rewritten: [[1702698901.293772520, {}], {"message"=>"dummy"}]
[0] rewritten: [[1702698902.298262899, {}], {"message"=>"dummy"}]
^C[2023/12/16 12:55:03] [engine] caught signal (SIGINT)
[0] rewritten: [[1702698903.241552179, {}], {"message"=>"dummy"}]
[2023/12/16 12:55:03] [ warn] [engine] service will shutdown in max 5 seconds
[2023/12/16 12:55:03] [ info] [input] pausing dummy.0
[2023/12/16 12:55:03] [ info] [input] pausing storage_backlog.1
[2023/12/16 12:55:04] [ info] [engine] service has stopped (0 pending tasks)
[2023/12/16 12:55:04] [ info] [input] pausing dummy.0
[2023/12/16 12:55:04] [ info] [input] pausing storage_backlog.1
[2023/12/16 12:55:04] [ info] [output:stdout:stdout.0] thread worker #0 stopping...
[2023/12/16 12:55:04] [ info] [output:stdout:stdout.0] thread worker #0 stopped
==78668== 
==78668== HEAP SUMMARY:
==78668==     in use at exit: 0 bytes in 0 blocks
==78668==   total heap usage: 2,365 allocs, 2,365 frees, 2,524,208 bytes allocated
==78668== 
==78668== All heap blocks were freed -- no leaks are possible
==78668== 
==78668== For lists of detected and suppressed errors, rerun with: -s
==78668== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)


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.

cio_load creates storage stream if directory exists.
It prevents to create memory stream by input plugins.
This patch is to recreate stream if the type is different.

Signed-off-by: Takahiro Yamashita <[email protected]>
@@ -532,6 +532,18 @@ int flb_storage_input_create(struct cio_ctx *cio,
return -1;
}
}
else if (stream->type != cio_storage_type) {
flb_info("[storage] storage type mismatch. input type=%s",
Copy link
Member

Choose a reason for hiding this comment

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

I will put this as a debug instead of info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case user environment is need to be fixed.

  • Remove storage
  • Modify storage property

I think it is better the level is info or warn to notice it.
What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

hmm, I would not remove the storage content for the user unless the directory is empty (no chunks), but I agree with better messaging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modified log message. Could you check it ?
34c00a1

Signed-off-by: Takahiro Yamashita <[email protected]>
@edsiper edsiper added this to the Fluent Bit v2.2.2 milestone Jan 10, 2024
@edsiper edsiper merged commit 309d961 into fluent:master Jan 10, 2024
38 of 40 checks passed
@edsiper
Copy link
Member

edsiper commented Jan 10, 2024

thank you

@nokute78 nokute78 deleted the fix_8259 branch January 13, 2024 00:13
shaerpour pushed a commit to shaerpour/fluent-bit that referenced this pull request Jan 16, 2024
fluent#8290)

* storage: recreate cio_stream if storage type is different(fluent#8259)

cio_load creates storage stream if directory exists.
It prevents to create memory stream by input plugins.
This patch is to recreate stream if the type is different.

Signed-off-by: Takahiro Yamashita <[email protected]>

* storage: modify log message

Signed-off-by: Takahiro Yamashita <[email protected]>

---------

Signed-off-by: Takahiro Yamashita <[email protected]>
shaerpour pushed a commit to shaerpour/fluent-bit that referenced this pull request Jan 16, 2024
fluent#8290)

* storage: recreate cio_stream if storage type is different(fluent#8259)

cio_load creates storage stream if directory exists.
It prevents to create memory stream by input plugins.
This patch is to recreate stream if the type is different.

Signed-off-by: Takahiro Yamashita <[email protected]>

* storage: modify log message

Signed-off-by: Takahiro Yamashita <[email protected]>

---------

Signed-off-by: Takahiro Yamashita <[email protected]>
Signed-off-by: ahspw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants