Merge pull request #54 from glyn/master

oom diagnostic logging

.gitignore

@@ -12,3 +12,4 @@ ci-working-dir
 *.swp
 .rvmrc
 *.pid
+*.iml

warden/lib/warden/container/features/mem_limit.rb

@@ -22,7 +22,7 @@ def initialize(container)
             @container = container
 
             oom_notifier_path = Warden::Util.path("src/oom/oom")
-            @child = DeferredChild.new(oom_notifier_path, container.cgroup_path(:memory))
+            @child = DeferredChild.new(oom_notifier_path, container.memory_cgroup_path)
             @child.logger = logger
             @child.run
             @child_exited = false
@@ -65,14 +65,49 @@ def restore
         end
 
         def oomed
-          logger.warn("OOM happened for #{handle}")
+          memory = memory_cgroup_file_contents('memory.usage_in_bytes')
+          memory_limit = memory_cgroup_file_contents('memory.limit_in_bytes')
+          swap = memory_cgroup_file_contents('memory.memsw.usage_in_bytes')
+          swap_limit = memory_cgroup_file_contents('memory.memsw.limit_in_bytes')
+          stats = format_memory_stats(memory_cgroup_file_contents('memory.stat'))
+          logger.warn("OOM happened for container with handle '#{handle}', memory usage: #{memory}, memory limit: #{memory_limit}, memory + swap usage: #{swap}, memory + swap limit: #{swap_limit}, #{stats}")
 
           events << "out of memory"
+
+          oom_killer true
+
           if state == State::Active
             dispatch(Protocol::StopRequest.new)
           end
         end
 
+        def format_memory_stats(memory_stats)
+          memory_stats.gsub(' ', ': ').gsub("\n", ', ')
+        end
+
+        private :format_memory_stats
+
+        def oom_killer(enable)
+          File.open(File.join(memory_cgroup_path, "memory.oom_control"), 'w') do |f|
+            f.write(enable ? '0' : '1')
+          end
+        end
+
+        private :oom_killer
+
+        def memory_cgroup_file_contents(filename)
+          File.read(File.join(memory_cgroup_path, filename)).chomp
+        rescue
+          # memory.memsw.* files cannot be read when swapping is off
+          '-'
+        end
+
+        private :memory_cgroup_file_contents
+
+        def memory_cgroup_path
+          cgroup_path(:memory)
+        end
+
         def start_oom_notifier_if_needed
           unless @oom_notifier
             @oom_notifier = OomNotifier.new(self)
@@ -89,6 +124,9 @@ def start_oom_notifier_if_needed
         private :start_oom_notifier_if_needed
 
         def limit_memory(limit_in_bytes)
+          # Disable the oom killer before setting up the oom notifier.
+          oom_killer false
+
           # Need to set up the oom notifier before we set the memory
           # limit to avoid a race between when the limit is set and
           # when the oom notifier is registered.
@@ -106,7 +144,7 @@ def limit_memory(limit_in_bytes)
           # successfully. To mitigate this, both limits are written twice.
           2.times do
             ["memory.limit_in_bytes", "memory.memsw.limit_in_bytes"].each do |path|
-              File.open(File.join(cgroup_path(:memory), path), 'w') do |f|
+              File.open(File.join(memory_cgroup_path, path), 'w') do |f|
                 f.write(limit_in_bytes.to_s)
               end
             end
@@ -126,7 +164,7 @@ def do_limit_memory(request, response)
             end
           end
 
-          limit_in_bytes = File.read(File.join(cgroup_path(:memory), "memory.limit_in_bytes"))
+          limit_in_bytes = File.read(File.join(memory_cgroup_path, "memory.limit_in_bytes"))
           response.limit_in_bytes = limit_in_bytes.to_i
 
           nil

warden/spec/container/linux_spec.rb

@@ -204,7 +204,11 @@ def trigger_oom
 
     describe "setting limits" do
       def integer_from_memory_cgroup(file)
-        File.read(File.join("/tmp/warden/cgroup/memory", "instance-#{@handle}", file)).to_i
+        memory_cgroup_file_contents(file).to_i
+      end
+
+      def memory_cgroup_file_contents(file)
+        File.read(File.join("/tmp/warden/cgroup/memory", "instance-#{@handle}", file))
       end
 
       let(:hundred_mb) { 100 * 1024 * 1024 }
@@ -214,6 +218,10 @@ def integer_from_memory_cgroup(file)
         response.limit_in_bytes.should == hundred_mb
       end
 
+      it 'disables oom killer' do
+        memory_cgroup_file_contents("memory.oom_control").should match(/oom_kill_disable\s*1/)
+      end
+
       it "sets `memory.limit_in_bytes`" do
         integer_from_memory_cgroup("memory.limit_in_bytes").should == hundred_mb
       end