-
Notifications
You must be signed in to change notification settings - Fork 1.8k
filter_throttle: support retain messages if rate limit is exceeded #5357
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
base: master
Are you sure you want to change the base?
Conversation
…limit is exceeded Signed-off-by: wangzhuzhen <[email protected]>
837efb0 to
cf25d05
Compare
|
@koleini @fujimotos @leonardo-albertovich @edsiper Please review this PR and give some feedbacks, Thank you very much! |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
@patrick-stephens can you maybe review this PR? |
|
The docs-required tag can be removed @patrick-stephens as the docs PR fluent/fluent-bit-docs#796 is waiting. |
patrick-stephens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify the use case?
Why would you use a throttle plugin that retains data instead of dropping it? Just don't use it if you want to retain data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems redundant, all it does is return the value of ctx->retain_data
This patch is to support retain messages if rate limit is exceeded.
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L84-L87
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L108-L115
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L212-L213
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L240-L241
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L247-L252
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.c#L302-L306
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.h#L35
https://github.com/fluent/fluent-bit/blob/master/plugins/filter_throttle/throttle.h#L42
For now, if rate limit is exceeded, throttle will drop the messages.
For now, if before fluent-bit first running, there is a input with huge messages which exceeded the throttle's (window * rate * interval), then only the first (window * rate * interval) records will be save, the others before fluent-bit running will be dropped.
Fixes #5356
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:
Configuration
For file /home/wong/fluent-bit-test/input.log with 150,000 lines records with format:
"[$id] aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"we can check the result in output file /home/wong/fluent-bit-test/output.log by timestamp and account lines
Debug log
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
Documentation
fluent/fluent-bit-docs#796
Backporting
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.