-
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 system config using double memory #496
Conversation
end | ||
end | ||
|
||
# TODO: this method should be moved to SystemConfig class method | ||
def apply_system_config(opt) | ||
# Create NULL file to avoid $log uninitialized problem before call @log.init |
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.
Removed this trick. Now, read_config
and apply_system_config
uses the default (or commad line option arguments) log_level on starting up. After the system_config is applied, the log_level is changed.
96bfef4
to
a3b5797
Compare
18b4c20
to
bb6c0bd
Compare
Verified this code works well by running on my production environment. |
opt.merge!(SystemConfig.new(systems.first).to_opt) | ||
ensure | ||
file.close | ||
SystemConfig.new(systems.first).apply(self) |
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.
Why this line is in ensure?
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.
Look. Not in ensure
.
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.
Ouch!
#497 provides new plugin instantiation approach. |
It seems ok. @tagomoris Do you see any concern? |
No problem. |
Fix system config using double memory
Fix system config using double memory
Changes Unknown when pulling bb6c0bd on fix_system_config into * on master*. |
since fluent#496 Signed-off-by: Yuta Iwama <[email protected]>
The current code calls
read_config
twice on the parent (supervisor) process and the child (main) process. Because the member variable@config_data
is overwritten in the child process, the memory usage for@config_data
becomes double. This causes a problem on an environment whose conf file is large like 10,000 lines.Commenting out this line
fluentd/lib/fluent/supervisor.rb
Line 108 in 50a90ac
This patch fixes the problem without removing the system config feature.