in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677
Conversation
cosmo0920
requested review from
edsiper,
leonardo-albertovich,
fujimotos and
koleini
as code owners
cosmo0920
force-pushed
the
cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog
branch
from
9666fe5 to
0930d59
Compare
cosmo0920
changed the title
cosmo0920
force-pushed
the
cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog
branch
from
0930d59 to
4dbeeb7
Compare
…per a cycle Previously, I had chosen to restrict with the threshold to prevent unlimited subscribing per a cycle. This should cause flood of memory comsumptions but it depends on the amound of the channels of Windows EventLog. Nowadays, this plugin is getting widely used. Also, in the fluent community slack, there is a request to make to be able to configure for this threshold per a cycle. To prevent slowing down for the subscribe the channels, it won't de able to set up below 512KiB with the newly introduced parameter. Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
force-pushed
the
cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog
branch
from
4dbeeb7 to
f6a4d46
Compare
|
Can someone prioritize the review and merge this PR? @edsiper @leonardo-albertovich @fujimotos @koleini It'd resolve one part of performance issues (Ref: #8673) for winevtlog plugin and hence would be of great help! |
Signed-off-by: Hiroshi Hatake <[email protected]>
plugins/in_winevtlog/in_winevtlog.c
Outdated
| #define DEFAULT_THRESHOLD_SIZE 0x7ffff /* Default reading buffer size */ | ||
| /* (512kib = 524287bytes) */ | ||
| #define MINIMUM_THRESHOLD_SIZE 0x0400 /* 1024 bytes */ | ||
| #define MAXIMUM_THRESHOLD_SIZE 0x1ccccd /* 1887437 bytes (about 1.8 MiB) */ |
There was a problem hiding this comment.
Set MAXIMUM_THRESHOLD_SIZE to (FLB_INPUT_CHUNK_FS_MAX_SIZE - (1024 * 200)) so it scales in case someone changes the default maximum chunk size.
I guess in the future when we make the chunk size configurable we'll have to do that in runtime.
There was a problem hiding this comment.
Sorry about not requesting it this way the first time around, it must've slipped from my mind.
There was a problem hiding this comment.
No worries. Your suggestion is also helpful. And it has more effective than just specified the fixed value for the current adequate value.
…pecification of chunk size Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
plugins/in_winevtlog/in_winevtlog.c
Outdated
| flb_utils_bytes_to_human_readable_size((size_t) MINIMUM_THRESHOLD_SIZE, | ||
| human_readable_size, | ||
| sizeof(human_readable_size) - 1); | ||
| flb_plg_error(ctx->ins, |
There was a problem hiding this comment.
You probably meant to use flb_plg_warn here
There was a problem hiding this comment.
Oh, good catch. I'll fix it shortly.
Signed-off-by: Hiroshi Hatake <[email protected]>
edsiper
deleted the
cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog
branch
|
thanks! |
Previously, I had chosen to restrict with the threshold to prevent unlimited subscribing per a cycle.
This should cause flood of memory comsumptions but it depends on the amound of the channels of Windows EventLog.
Nowadays, this plugin is getting widely used.
Also, in the fluent community slack, there is a request to make to be able to configure for this threshold per a cycle.
To prevent slowing down for the subscribe the channels, it won't de able to set up below 512KiB with the newly introduced parameter.
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:
After introduced
threshold_sizeparameter, the number of collected logs per a cycle is increased around 2000. This could increase events per second.If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
fluent/fluent-bit-docs#1350
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.