Bug 1171289 - background pkill command in OOMPlugin

node-util/conf/watchman/plugins.d/oom_plugin.rb

@@ -22,8 +22,6 @@
 require 'openshift-origin-node/utils/cgroups/libcgroup'
 require 'openshift-origin-node/model/application_container'
 
-OP_TIMEOUT=360
-
 # Provide Watchman with monitoring of CGroups resource killing of gears
 class OomPlugin < OpenShift::Runtime::WatchmanPlugin
   PLUGIN_NAME    = 'OOM Plugin'
@@ -69,6 +67,13 @@ def try_cgfetch(cg, attr, retries=CG_RETRIES)
     return nil
   end
 
+  def safe_pkill(uuid)
+    # We need to background and detach this pkill command, because it
+    # will usually hang until the memsw limit is bumped.
+    pid = Kernel.spawn("pkill -9 -u #{uuid}")
+    Process.detach(pid)
+  end
+
   # Search for gears under_oom
   # @param [OpenShift::Runtime::WatchmanPluginTemplate::Iteration] iteration timestamps of given events
   # @return void
@@ -107,19 +112,19 @@ def apply(iteration)
         # Verify that we are ready to reset to the old limit
         current = try_cgfetch(cgroup, MEMSW_USAGE)[MEMSW_USAGE].to_i
         increased = orig_memsw_limit
-        app = OpenShift::Runtime::ApplicationContainer.from_uuid(uuid)
-        while (current >= restore_memsw_limit or current == 0) && retries > 0
+        while (current >= restore_memsw_limit or current == 0 or oom_control['under_oom'] == '1') && retries > 0
           # Increase limit by 10% in order to clean up processes. Trying to
           # restart a gear already at its memory limit is treacherous.
           increased = (increased * @memsw_multiplier).round(0)
           Syslog.info %Q(#{PLUGIN_NAME}: Increasing memory for gear #{uuid} to #{increased} and killing processes)
-          app.kill_procs()
+          safe_pkill(uuid)
           if not try_cgstore(cgroup, MEMSW_LIMIT, increased)
             Syslog.warning %Q(#{PLUGIN_NAME}: Failed to increase memsw limit for gear #{uuid})
           end
           sleep @stop_wait_seconds
           retries -= 1
           current = try_cgfetch(cgroup, MEMSW_USAGE)[MEMSW_USAGE].to_i
+          oom_control = try_cgfetch(cgroup, 'memory.oom_control')['memory.oom_control']
         end
       rescue Exception => e
         Syslog.warning %Q(#{PLUGIN_NAME}: exception in OOM handling: #{e})

node/test/unit/oom_plugin_test.rb

@@ -40,9 +40,6 @@ def setup
     @libcgroup_mock = mock('OpenShift::Runtime::Utils::Cgroups::Libcgroup')
     @libcgroup_mock.stubs(:parameters).returns(parameters)
 
-    @appcontainer_mock = mock('OpenShift::Runtime::ApplicationContainer')
-    @appcontainer_mock.stubs(:kill_procs).returns(nil)
-
   end
 
   def test_no_oom_control
@@ -60,26 +57,51 @@ def test_no_oom_control
   end
 
   def test_oom_control
+    # This tests over a set of four gears where only one is OOM
+    # That gear is tested under three conditions:
+    #   1) under_oom = 1, mem usage == mem limit
+    #   2) under_oom = 1, mem usage < mem limit
+    #   3) under_oom = 0, mem usage < mem limit
+    # Then it is restarted.
     @libcgroup_mock.expects(:fetch).
         with('memory.oom_control').
         returns({'memory.oom_control' =>
+                     {'under_oom'        => '0',
+                      'oom_kill_disable' => '0'}},
+                {'memory.oom_control' =>
                      {'under_oom'        => '1',
+                      'oom_kill_disable' => '0'}},
+                {'memory.oom_control' =>
+                     {'under_oom'        => '1',
+                      'oom_kill_disable' => '0'}},
+                {'memory.oom_control' =>
+                     {'under_oom'        => '0',
                       'oom_kill_disable' => '0'}}).
-        times(@uuids.length)
-    @libcgroup_mock.expects(:fetch).with(OomPlugin::MEMSW_LIMIT).returns({OomPlugin::MEMSW_LIMIT => 1024}).times(@uuids.length)
-    @libcgroup_mock.expects(:fetch).with(OomPlugin::MEMSW_USAGE).returns({OomPlugin::MEMSW_USAGE => 1024}).times(@uuids.length * (OomPlugin::BUMP_RETRIES + 1))
-    @libcgroup_mock.expects(:store).with(OomPlugin::MEMSW_LIMIT, kind_of(Fixnum)).times(@uuids.length * (OomPlugin::BUMP_RETRIES + 1))
-
-    OpenShift::Runtime::ApplicationContainer.stubs(:from_uuid).
-        with(any_of(*@uuids)).
-        returns(@appcontainer_mock)
+        times(@uuids.length + 3)
+    @libcgroup_mock.expects(:fetch).
+        with(OomPlugin::MEMSW_LIMIT).
+        returns({OomPlugin::MEMSW_LIMIT => 1024}).
+        times(1)
+    @libcgroup_mock.expects(:fetch).
+        with(OomPlugin::MEMSW_USAGE).
+        returns({OomPlugin::MEMSW_USAGE => 1024},
+                {OomPlugin::MEMSW_USAGE => 1023}).
+        times(4)
+    @libcgroup_mock.expects(:store).
+        with(OomPlugin::MEMSW_LIMIT, kind_of(Fixnum)).
+        times(4)
 
     OpenShift::Runtime::Utils::Cgroups::Libcgroup.stubs(:new).
         with(any_of(*@uuids)).
         returns(@libcgroup_mock)
 
-    @operation.expects(:call).with(:restart, any_of(*@uuids)).times(@uuids.length)
+    @operation.expects(:call).with(:restart, @uuids[1]).times(1)
 
-    OomPlugin.new(nil, nil, @gears, @operation, 0).apply(nil)
+    oom_plugin = OomPlugin.new(nil, nil, @gears, @operation, 0)
+    # stubbing the pkill call.  Is there a better way to mock this?
+    def oom_plugin.safe_pkill uuid
+      return nil
+    end
+    oom_plugin.apply(nil)
   end
 end