-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix monitor agent compatibility #1232
Conversation
* RootAgent now knows all output plugins including children of MultiOutput * So in_monitor_agent doesn't need to fetch children of MultiOutput plugins * Fluent::Plugin::MultiOutput is now a root plugin class out of Fluent::Plugin::Output
…n_monitor_agent
@repeatedly Please review this too. |
@@ -326,6 +326,7 @@ def start | |||
@buffering = prefer_buffered_processing | |||
if !@buffering && @buffer | |||
@buffer.terminate # it's not started, so terminate will be enough | |||
@buffer = nil |
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.
It seems error prune.
Using configured value is better for detecting output plugin is un-buffered or buffered.
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.
Non-buffered output plugin (without #write
method) doesn't have values on @buffer
.
And, output plugin instances which runs this line is determined as non-buffered plugin. So the @buffer
value is unassigned.
No bad/wrong things there are.
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.
I see. If so, adding comment is better like "This line is needed for detecting buffer or non-buffer, e.g. used by in_monitor_agent".
Only this line, it looks like for GC.
And we should move this :buffer
reader from for test
to above attr_reader
lines.
fluentd/lib/fluent/plugin/output.rb
Line 145 in 24e15f3
attr_reader :buffer, :retry, :secondary, :chunk_keys, :chunk_key_time, :chunk_key_tag |
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.
No. @buffering
shows this plugin is buffered or non-buffered. Unassigning @buffer
is just to avoid side-effect in in_monitor_agent
.
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.
And @buffer
is still only for tests (... and in_monitor_agent
).
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.
I understand the situation. Adding comment is better with "Avoid side-effect that in_monitor_agent shows non-buffered plugins as buffered."
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.
Done.
LGTM |
fix #1226.