Bug#778303: chef: Memory leak

[Piotr Pańczyk]

Hello again,

I think that is a serious regression. If you manage to make it work
correctly, please do get in touch and we can prepare a stable update for
the memory leak.

I think I've done it, looking at the source of chef 11.12.8-2 (from
Jessie). I'm not a programmer and I know Ruby only a little, so it would
probably be possible to do it better, but it's been working fine on
about 150 nodes for over 5 months. The stacktrace is still visable in
the output (and also in logs), but the chef-stacktrace.out file is also
produced properly, including the forked client stacktrace. I attach the
patch and chef-stacktrace.out files to compare - from modified Wheezy
package and from Jessie.
Is the result satisfying?

adding new features in a stable update is generally not a good idea,
no matter what.

OK, I understand.  However, I found another bug in the package, related
to FileEdit. Both the issue and the solution is here:
https://github.com/chef/chef/pull/754
Please let me know if it is possible to apply it together with the
forked runs (technically it is), or should I open another issue.

--
Greetings,
Piotr Pańczyk




Asseco Business Solutions S.A.
ul. Konrada Wallenroda 4c
20-607 Lublin
tel.: +48 81 535 30 00
fax: +48 81 535 30 05

Sąd Rejonowy Lublin-Wschód w Lublinie z siedzibą w Świdniku
VI Wydział Gospodarczy Krajowego Rejestru Sądowego
KRS 028257
NIP 522-26-12-717
kapitał zakładowy 167 090 965,00 zł (w całości opłacony)
www.assecobs.pl



Powyższa korespondencja przeznaczona jest wyłącznie dla osoby lub podmiotu, do 
którego jest adresowana i może zawierać informacje o charakterze poufnym lub 
zastrzeżonym. Nieuprawnione wykorzystanie informacji zawartych w wiadomości 
e-mail przez osobę lub podmiot nie będący jej adresatem jest zabronione 
odpowiednimi przepisami prawa. Odbiorca korespondencji, który otrzymał ją 
omyłkowo, proszony jest o niezwłoczne zawiadomienie nadawcy drogą elektroniczną 
lub telefonicznie i usunięcie tej treści z poczty elektronicznej. Dziękujemy. 
Asseco Business Solutions S.A.



Weź pod uwagę ochronę środowiska, zanim wydrukujesz ten e-mail.



This information is intended only for the person or entity to which it is 
addressed and may contain confidential and/or privileged material. Unauthorized 
use of this information by person or entity other than the intended recipient 
is prohibited by law. If you received this by mistake, please immediately 
contact the sender by e-mail or by telephone and delete this information from 
any computer. Thank you. Asseco Business Solutions S.A.



Please consider your environmental responsibility before printing this e-mail.
--- chef-10.12.0.orig/lib/chef/config.rb
+++ chef-10.12.0/lib/chef/config.rb
@@ -184,6 +184,7 @@ class Chef
 run_command_stdout_timeout 120
 solo  false
 splay nil
+client_fork true
 
 # Set these to enable SSL authentication / mutual-authentication
 # with the server
--- chef-10.12.0.orig/lib/chef/daemon.rb
+++ chef-10.12.0/lib/chef/daemon.rb
@@ -24,6 +24,7 @@ class Chef
   class Daemon
 class << self
   attr_accessor :name
+  attr_accessor :forked_child
 
   # Daemonize the current process, managing pidfiles and process uid/gid
   #
@@ -46,7 +47,7 @@ class Chef
 $stdout.reopen("/dev/null", "a")
 $stderr.reopen($stdout)
 save_pid_file
-at_exit { remove_pid_file }
+at_exit { remove_pid_file unless forked_child }
   rescue NotImplementedError => e
 Chef::Application.fatal!("There is no fork: #{e.message}")
   end
--- chef-10.12.0.orig/lib/chef/client.rb
+++ chef-10.12.0/lib/chef/client.rb
@@ -36,6 +36,7 @@ require 'chef/cookbook/file_vendor'
 require 'chef/cookbook/file_system_file_vendor'
 require 'chef/cookbook/remote_file_vendor'
 require 'chef/version'
+require 'chef/daemon'
 require 'ohai'
 require 'rbconfig'
 
@@ -135,50 +136,52 @@ class Chef
 end
 
 # Do a full run for this Chef::Client.  Calls:
+# * do_run
 #
-#  * run_ohai - Collect information about the system
-#  * build_node - Get the last known state, merge with local changes
-#  * register - If not in solo mode, make sure the server knows about this client
-#  * sync_cookbooks - If not in solo mode, populate the local cache with the node's cookbooks
-#  * converge - Bring this system up to date
-#
+# This provides a wrapper around #do_run allowing the 
+# run to be optionally forked.
 # === Returns
-# true:: Always returns true.
+# boolean:: Return value from #do_run. Should always returns true.
 def run
-  run_context = nil
-
-  Chef::Log.info("*** Chef #{Chef::VERSION} ***")
-  enforce_path_sanity
-  run_ohai
-  register unless Chef::Config[:solo]
-  build_node
-
-  begin
-

Bug#778303: chef: Memory leak

[Antonio Terceiro]

On Mon, Jun 15, 2015 at 02:58:39PM +0200, Piotr Pańczyk wrote:
 Hello again,
 I did manage to apply those two commits manually:
 https://github.com/chef/chef/commit/d99e497874e7d08f017376717aa38a4c8d7fecd5
 https://github.com/chef/chef/commit/6c10604f1e8e9b557b69449b484df3aae47ee468
 omitting a few lines related to some intermediate versions.
 Forking is working correctly, the result is visualized in attached munin
 grapph. I also attach the diff result.
 
 Such modified client has been used on about 80 servers for over 2
 months. The only problem I noticed is that if client run fails, the
 stacktrace.out file only contains stacktrace from the parent process.
 Child stacktrace is printed out to stderr (it's not included in logs,
 visible only while executing the manual, single run). I've made some
 custom modifications comparing files with chef 11.12, but I think the
 result is not yet what we really expect. If you want to look at it,
 please let me know.

I think that is a serious regression. If you manage to make it work
correctly, please do get in touch and we can prepare a stable update for
the memory leak.

 If you decide to backport the package, I would suggest to add one small
 feature (from newer versions) by chance - I really miss the file_edited?
 method in Chef::Util::FileEdit. It's just one getter, the diff is also
 attached.

adding new features in a stable update is generally not a good idea, no
matter what.

-- 
Antonio Terceiro terce...@debian.org


signature.asc
Description: Digital signature

Bug#778303: chef: Memory leak

[Piotr Pańczyk]

Hello again,
I did manage to apply those two commits manually:
https://github.com/chef/chef/commit/d99e497874e7d08f017376717aa38a4c8d7fecd5
https://github.com/chef/chef/commit/6c10604f1e8e9b557b69449b484df3aae47ee468
omitting a few lines related to some intermediate versions.
Forking is working correctly, the result is visualized in attached munin
grapph. I also attach the diff result.

Such modified client has been used on about 80 servers for over 2
months. The only problem I noticed is that if client run fails, the
stacktrace.out file only contains stacktrace from the parent process.
Child stacktrace is printed out to stderr (it's not included in logs,
visible only while executing the manual, single run). I've made some
custom modifications comparing files with chef 11.12, but I think the
result is not yet what we really expect. If you want to look at it,
please let me know.

If you decide to backport the package, I would suggest to add one small
feature (from newer versions) by chance - I really miss the file_edited?
method in Chef::Util::FileEdit. It's just one getter, the diff is also
attached.

--
Greetings,
Piotr Pańczyk




Asseco Business Solutions S.A.
ul. Konrada Wallenroda 4c
20-607 Lublin
tel.: +48 81 535 30 00
fax: +48 81 535 30 05

Sąd Rejonowy Lublin-Wschód w Lublinie z siedzibą w Świdniku
VI Wydział Gospodarczy Krajowego Rejestru Sądowego
KRS 028257
NIP 522-26-12-717
kapitał zakładowy 167 090 965,00 zł (w całości opłacony)
www.assecobs.pl



Powyższa korespondencja przeznaczona jest wyłącznie dla osoby lub podmiotu, do 
którego jest adresowana i może zawierać informacje o charakterze poufnym lub 
zastrzeżonym. Nieuprawnione wykorzystanie informacji zawartych w wiadomości 
e-mail przez osobę lub podmiot nie będący jej adresatem jest zabronione 
odpowiednimi przepisami prawa. Odbiorca korespondencji, który otrzymał ją 
omyłkowo, proszony jest o niezwłoczne zawiadomienie nadawcy drogą elektroniczną 
lub telefonicznie i usunięcie tej treści z poczty elektronicznej. Dziękujemy. 
Asseco Business Solutions S.A.



Weź pod uwagę ochronę środowiska, zanim wydrukujesz ten e-mail.



This information is intended only for the person or entity to which it is 
addressed and may contain confidential and/or privileged material. Unauthorized 
use of this information by person or entity other than the intended recipient 
is prohibited by law. If you received this by mistake, please immediately 
contact the sender by e-mail or by telephone and delete this information from 
any computer. Thank you. Asseco Business Solutions S.A.



Please consider your environmental responsibility before printing this e-mail.
--- chef-10.12.0.orig/lib/chef/config.rb
+++ chef-10.12.0/lib/chef/config.rb
@@ -184,6 +184,7 @@ class Chef
 run_command_stdout_timeout 120
 solo  false
 splay nil
+client_fork true
 
 # Set these to enable SSL authentication / mutual-authentication
 # with the server
--- chef-10.12.0.orig/lib/chef/daemon.rb
+++ chef-10.12.0/lib/chef/daemon.rb
@@ -24,6 +24,7 @@ class Chef
   class Daemon
 class  self
   attr_accessor :name
+  attr_accessor :forked_child
 
   # Daemonize the current process, managing pidfiles and process uid/gid
   #
@@ -46,7 +47,7 @@ class Chef
 $stdout.reopen(/dev/null, a)
 $stderr.reopen($stdout)
 save_pid_file
-at_exit { remove_pid_file }
+at_exit { remove_pid_file unless forked_child }
   rescue NotImplementedError = e
 Chef::Application.fatal!(There is no fork: #{e.message})
   end
--- chef-10.12.0.orig/lib/chef/client.rb
+++ chef-10.12.0/lib/chef/client.rb
@@ -135,48 +135,29 @@ class Chef
 end
 
 # Do a full run for this Chef::Client.  Calls:
+# * do_run
 #
-#  * run_ohai - Collect information about the system
-#  * build_node - Get the last known state, merge with local changes
-#  * register - If not in solo mode, make sure the server knows about this client
-#  * sync_cookbooks - If not in solo mode, populate the local cache with the node's cookbooks
-#  * converge - Bring this system up to date
-#
+# This provides a wrapper around #do_run allowing the 
+# run to be optionally forked.
 # === Returns
-# true:: Always returns true.
+# boolean:: Return value from #do_run. Should always returns true.
 def run
-  run_context = nil
-
-  Chef::Log.info(*** Chef #{Chef::VERSION} ***)
-  enforce_path_sanity
-  run_ohai
-  register unless Chef::Config[:solo]
-  build_node
-
-  begin
-
-run_status.start_clock
-Chef::Log.info(Starting Chef Run for #{node.name})
-run_started
-
-run_context = setup_run_context
-converge(run_context)
-save_updated_node
-
-

Bug#778303: chef: Memory leak

[Piotr Pańczyk]

Hello Antonio,
I think the simple, ad ad-hoc solution can be changing the default logrotate 
period from a week to a day, which would slightly reduce the amount of wasted 
memory, as logrotate restarts the daemon.

The problem is quite serious for us, especially on servers running a lot of 
virtual machines. If you manage to apply the commits related to the forking 
behaviour, I can test the package for you.

On 22.02.2015 03:16, Antonio Terceiro wrote:

Hello Piotr,

On Fri, Feb 13, 2015 at 03:56:38PM +0100, Piotr Pańczyk wrote:


Hello Antonio,

On 13.02.2015 14:29, Antonio Terceiro wrote:


Would you by any chance be able to identify the upstream commit that
introduced the fix? If so, I can try backporting that and releasing a
stable update.


I think this is it:
https://github.com/chef/chef/pull/291



I was looking at this, and there a few issues:

- that commit does not apply clean on the wheezy chef tree.

- there are are a few other commits in the upstream history related to
 the forking behaviour, so I think there were a few issues with this
 it. I have the the impression that it is too risky to pull this into a
 stable branch, and I won't have to spare cycles to handle the breakage
 that will probably come.

- I only use chef-solo, so testing this would a little difficult for me.



--
Greetings,
Piotr Pańczyk



Asseco Business Solutions S.A.
ul. Konrada Wallenroda 4c
20-607 Lublin
tel.: +48 81 535 30 00
fax: +48 81 535 30 05

Sąd Rejonowy Lublin-Wschód w Lublinie z siedzibą w Świdniku
VI Wydział Gospodarczy Krajowego Rejestru Sądowego
KRS 028257
NIP 522-26-12-717
kapitał zakładowy 167 090 965,00 zł (w całości opłacony)
www.assecobs.pl



Powyższa korespondencja przeznaczona jest wyłącznie dla osoby lub podmiotu, do 
którego jest adresowana i może zawierać informacje o charakterze poufnym lub 
zastrzeżonym. Nieuprawnione wykorzystanie informacji zawartych w wiadomości 
e-mail przez osobę lub podmiot nie będący jej adresatem jest zabronione 
odpowiednimi przepisami prawa. Odbiorca korespondencji, który otrzymał ją 
omyłkowo, proszony jest o niezwłoczne zawiadomienie nadawcy drogą elektroniczną 
lub telefonicznie i usunięcie tej treści z poczty elektronicznej. Dziękujemy. 
Asseco Business Solutions S.A.



Weź pod uwagę ochronę środowiska, zanim wydrukujesz ten e-mail.



This information is intended only for the person or entity to which it is 
addressed and may contain confidential and/or privileged material. Unauthorized 
use of this information by person or entity other than the intended recipient 
is prohibited by law. If you received this by mistake, please immediately 
contact the sender by e-mail or by telephone and delete this information from 
any computer. Thank you. Asseco Business Solutions S.A.



Please consider your environmental responsibility before printing this e-mail.

Bug#778303: chef: Memory leak

[Antonio Terceiro]

Hello Piotr,

On Fri, Feb 13, 2015 at 03:56:38PM +0100, Piotr Pańczyk wrote:
 Hello Antonio,
 
 On 13.02.2015 14:29, Antonio Terceiro wrote:
 Would you by any chance be able to identify the upstream commit that
 introduced the fix? If so, I can try backporting that and releasing a
 stable update.
 I think this is it:
 https://github.com/chef/chef/pull/291

I was looking at this, and there a few issues:

- that commit does not apply clean on the wheezy chef tree.

- there are are a few other commits in the upstream history related to
  the forking behaviour, so I think there were a few issues with this
  it. I have the the impression that it is too risky to pull this into a
  stable branch, and I won't have to spare cycles to handle the breakage
  that will probably come.

- I only use chef-solo, so testing this would a little difficult for me.

-- 
Antonio Terceiro terce...@debian.org


signature.asc
Description: Digital signature

Bug#778303: chef: Memory leak

[Piotr Panczyk]

Package: chef
Version: 10.12.0-3
Severity: important

Dear Maintainer,
Chef Client causes memory leak when run as a daemon. Just after start,
the process uses about 30 MB of RAM. Then this amount rises with each
client run. After a week (before logrotate restarts the daemon), the
usage is at about 500 MB. The exact value depends on cookbooks/recipies
used on the node.

As I know, the problem was fixed in Chef Client version 10.14 by
introducing forking for each client run.

Greetings,
Piotr

-- System Information:
Debian Release: 7.8
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.32.bsd54h0 (SMP w/24 CPU cores)
Locale: LANG=pl_PL.UTF-8, LC_CTYPE=pl_PL.UTF-8 (charmap=locale: Cannot set 
LC_CTYPE to default locale: No such file or directory
locale: Cannot set LC_MESSAGES to default locale: No such file or directory
locale: Cannot set LC_ALL to default locale: No such file or directory
ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/dash

Versions of packages chef depends on:
ii  debconf [debconf-2.0] 1.5.49
ii  ohai  6.14.0-2
ii  ruby  1:1.9.3
ii  ruby-bunny0.7.8-1
ii  ruby-erubis [erubis]  2.7.0-2
ii  ruby-highline 1.6.13-2
ii  ruby-json 1.7.3-3
ii  ruby-mixlib-authentication1.1.4-2
ii  ruby-mixlib-cli   1.2.2-2
ii  ruby-mixlib-config1.1.2-3
ii  ruby-mixlib-log   1.4.1-1
ii  ruby-mixlib-shellout  1.0.0-2
ii  ruby-moneta   0.6.0-4
ii  ruby-net-ssh  1:2.5.2-2
ii  ruby-net-ssh-multi1.1-2
ii  ruby-rest-client  1.6.7-3
ii  ruby-treetop  1.4.10-5
ii  ruby-uuidtools2.1.2-2
ii  ruby-yajl 1.1.0-2
ii  ruby1.9.1 [ruby-interpreter]  1.9.3.194-8.1+deb7u3
ii  ucf   3.0025+nmu3

chef recommends no packages.

chef suggests no packages.

-- Configuration Files:
/etc/default/chef-client changed:
LOGFILE=/var/log/chef/client.log
CONFIG=/etc/chef/client.rb
INTERVAL=1800
SPLAY=300


-- debconf information excluded


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#778303: chef: Memory leak

[Antonio Terceiro]

Hello Piotr, thanks for reporting.

On Fri, Feb 13, 2015 at 12:37:19PM +0100, Piotr Panczyk wrote:
 Package: chef
 Version: 10.12.0-3
 Severity: important
 
 Dear Maintainer,
 Chef Client causes memory leak when run as a daemon. Just after start,
 the process uses about 30 MB of RAM. Then this amount rises with each
 client run. After a week (before logrotate restarts the daemon), the
 usage is at about 500 MB. The exact value depends on cookbooks/recipies
 used on the node.
 
 As I know, the problem was fixed in Chef Client version 10.14 by
 introducing forking for each client run.

Would you by any chance be able to identify the upstream commit that
introduced the fix? If so, I can try backporting that and releasing a
stable update.

-- 
Antonio Terceiro terce...@debian.org


signature.asc
Description: Digital signature

Bug#778303: chef: Memory leak

[Piotr Pańczyk]

Hello Antonio,

On 13.02.2015 14:29, Antonio Terceiro wrote:

Would you by any chance be able to identify the upstream commit that
introduced the fix? If so, I can try backporting that and releasing a
stable update.

I think this is it:
https://github.com/chef/chef/pull/291

--
Greetings,
Piotr Pańczyk




Asseco Business Solutions S.A.
ul. Konrada Wallenroda 4c
20-607 Lublin
tel.: +48 81 535 30 00
fax: +48 81 535 30 05

Sąd Rejonowy Lublin-Wschód w Lublinie z siedzibą w Świdniku
VI Wydział Gospodarczy Krajowego Rejestru Sądowego
KRS 028257
NIP 522-26-12-717
kapitał zakładowy 167 090 965,00 zł (w całości opłacony)
www.assecobs.pl



Powyższa korespondencja przeznaczona jest wyłącznie dla osoby lub podmiotu, do 
którego jest adresowana i może zawierać informacje o charakterze poufnym lub 
zastrzeżonym. Nieuprawnione wykorzystanie informacji zawartych w wiadomości 
e-mail przez osobę lub podmiot nie będący jej adresatem jest zabronione 
odpowiednimi przepisami prawa. Odbiorca korespondencji, który otrzymał ją 
omyłkowo, proszony jest o niezwłoczne zawiadomienie nadawcy drogą elektroniczną 
lub telefonicznie i usunięcie tej treści z poczty elektronicznej. Dziękujemy. 
Asseco Business Solutions S.A.



Weź pod uwagę ochronę środowiska, zanim wydrukujesz ten e-mail.



This information is intended only for the person or entity to which it is 
addressed and may contain confidential and/or privileged material. Unauthorized 
use of this information by person or entity other than the intended recipient 
is prohibited by law. If you received this by mistake, please immediately 
contact the sender by e-mail or by telephone and delete this information from 
any computer. Thank you. Asseco Business Solutions S.A.



Please consider your environmental responsibility before printing this e-mail.


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org