fix(sleep) use interruptible sleep function

Author: Tieske

README.md

@@ -96,7 +96,7 @@ The setting `max_use` controls the timer behaviour. The default value is `1000`,
 which means that after each `1000` invocations the timer context is destroyed
 and a new one is generated (this happens transparent to the user).
 
-Optimizing this setting:
+Optimizing this setting (very opinionated/arbitrary!):
 
  * if the timer interval is more than `60` seconds, then keeping the context
    around in idle state for that period is probably more expensive resource wise
@@ -128,6 +128,8 @@ Versioning is strictly based on [Semantic Versioning](https://semver.org/)
   * Feat: provide a stacktrace upon errors in the timer callback
   * Feat: add a `max_use` option. This ensures timer-contexts are recycled to
     prevent memory leaks.
+  * Feat: adds a new function `sleep` similar to `ngx.sleep` except that it is
+    interrupted on worker exit.
   * Fix: now accounts for execution time of the handler, when rescheduling.
 
 ### 1.1.0 (6-Nov-2020)

docs/index.html

@@ -68,6 +68,10 @@ <h2><a href="#Functions">Functions</a></h2>
 	<td class="name" nowrap><a href="#new">new (opts, ...)</a></td>
 	<td class="summary">Create a new timer.</td>
 	</tr>
+	<tr>
+	<td class="name" nowrap><a href="#sleep">sleep (delay)</a></td>
+	<td class="summary">A <a href="index.html#sleep">sleep</a> function that exits early on worker exit.</td>
+	</tr>
 </table>
 
 <br/>
@@ -121,11 +125,11 @@ <h3>Usage:</h3>
     <li><p><code>recurring</code> : (boolean) set to <code>true</code> to make it a recurring timer</p></li>
     <li><p><code>jitter</code> : (optional, number) variable interval to add to the first interval, default 0.
     If set to 1 second then the first interval will be set between <code>interval</code> and <code>interval + 1</code>.
-    This makes sure if large numbers of timers are used, their execution gets randomly
+    This makes sure if a large number of timers are used, their execution gets randomly
     distributed.</p></li>
     <li><p><code>immediate</code> : (boolean) will do the first run immediately (the initial
     interval will be set to 0 seconds). This option requires the <code>recurring</code> option.
-    The first run will not include the <code>jitter</code> interval, it will be added to second run.</p></li>
+    The first run will not include the <code>jitter</code> interval, it will be added to the second run.</p></li>
     <li><p><code>detached</code> : (boolean) if set to <code>true</code> the timer will keep running detached, if
     set to <code>false</code> the timer will be garbage collected unless anchored
     by the user.</p></li>
@@ -147,6 +151,8 @@ <h3>Usage:</h3>
     maximum delay could be <code>interval * 2</code> before another worker picks it up. With
     this option set, the maximum delay will be <code>interval + sub_interval</code>.
     This option requires the <code>immediate</code> and <code>key_name</code> options.</p></li>
+    <li><p><code>max_use</code> : (optional, number, default 1000) the maximum use count for a
+    timer context before recreating it.</p></li>
 </ul>
 
 
@@ -211,6 +217,44 @@ <h3>Usage:</h3>
 <span class="keyword">end</span></pre>
     </ul>
 
+</dd>
+    <dt>
+    <a name = "sleep"></a>
+    <strong>sleep (delay)</strong>
+    </dt>
+    <dd>
+    A <a href="index.html#sleep">sleep</a> function that exits early on worker exit.  The same as <code>ngx.sleep</code>
+ except that it will be interrupted when the current worker starts exiting.
+ Calling this function after the worker started exiting will immediately
+ return (after briefly yielding with a <code>ngx.sleep(0)</code>).
+
+
+    <h3>Parameters:</h3>
+    <ul>
+        <li><span class="parameter">delay</span>
+         same as <code>ngx.sleep()</code>; delay in seconds.
+        </li>
+    </ul>
+
+    <h3>Returns:</h3>
+    <ol>
+
+        <code>true</code> if finished, <code>false</code> if returned early, or nil+err (under
+ the hood it will return: "<code>not ngx.worker.exiting()</code>").
+    </ol>
+
+
+
+    <h3>Usage:</h3>
+    <ul>
+        <pre class="example"><span class="keyword">if</span> <span class="keyword">not</span> sleep(<span class="number">5</span>) <span class="keyword">then</span>
+  <span class="comment">-- sleep was interrupted, exit now
+</span>  <span class="keyword">return</span> <span class="keyword">nil</span>, <span class="string">"exiting"</span>
+<span class="keyword">end</span>
+
+<span class="comment">-- do stuff</span></pre>
+    </ul>
+
 </dd>
 </dl>
 
@@ -219,7 +263,7 @@ <h3>Usage:</h3>
 </div> <!-- id="main" -->
 <div id="about">
 <i>generated by <a href="http://github.com/stevedonovan/LDoc">LDoc 1.4.6</a></i>
-<i style="float:right;">Last updated 2020-11-07 00:01:15 </i>
+<i style="float:right;">Last updated 2021-08-12 14:42:50 </i>
 </div> <!-- id="about" -->
 </div> <!-- id="container" -->
 </body>

docs/topics/README.md.html

@@ -32,6 +32,7 @@ <h2>Contents</h2>
 <li><a href="#Status">Status </a></li>
 <li><a href="#Synopsis">Synopsis </a></li>
 <li><a href="#Description">Description </a></li>
+<li><a href="#Performance_and_optimizations">Performance and optimizations </a></li>
 <li><a href="#History">History </a></li>
 <li><a href="#Copyright_and_License">Copyright and License </a></li>
 </ul>
@@ -53,7 +54,7 @@ <h2>Modules</h2>
 
 <h1>lua-resty-timer</h1>
 
-<p><a href="https://travis-ci.org/Kong/lua-resty-timer/branches"><img src="https://travis-ci.org/Kong/lua-resty-timer.svg?branch=master" alt="Build Status"/></a></p>
+<p><a href="https://travis-ci.com/Kong/lua-resty-timer/branches"><img src="https://travis-ci.com/Kong/lua-resty-timer.svg?branch=master" alt="Build Status"/></a></p>
 
 <p>Extended timers for OpenResty. Provided recurring, cancellable, node-wide timers,
 beyond what the basic OpenResty timers do.</p>
@@ -86,6 +87,7 @@ <h2>Synopsis</h2>
             shm_name = <span class="string">"timer_shm"</span>,   <span class="comment">-- shm to use for node-wide timers
 </span>            key_name = <span class="string">"my_key"</span>,      <span class="comment">-- key-name to use for node-wide timers
 </span>            sub_interval = <span class="number">0.1</span>,       <span class="comment">-- max cross worker extra delay
+</span>            max_use = <span class="number">1000</span>,           <span class="comment">-- maximum re-use of timer context
 </span>        }
 
         <span class="keyword">local</span> object
@@ -118,7 +120,8 @@ <h2>Description</h2>
 project.</p>
 
 <ul>
-    <li><p>recurring timers (supported by OR as well through <code>ngx.timer.every</code>)</p></li>
+    <li><p>recurring timers (supported by OR as well through <code>ngx.timer.every</code>, but this
+    implementation will not run overlapping timers)</p></li>
     <li><p>immediate first run for recurring timers</p></li>
     <li><p>cancellable timers</p></li>
     <li><p>cancel callback, called when the timer is cancelled</p></li>
@@ -132,6 +135,37 @@ <h2>Description</h2>
 <p>See the <a href="https://kong.github.io/lua-resty-timer/topics/README.md.html">online LDoc documentation</a>
 for the complete API.</p>
 
+<p><a name="Performance_and_optimizations"></a></p>
+<h2>Performance and optimizations</h2>
+
+<p>This timer implementation is based on "sleeping on a timer-context". This means
+that a single timer is created, and in between recurring invocations <code>ngx.sleep</code>
+is called as a delay to the next invocation. This as opposed to creating a new
+Nginx timer for each invocation. This is configurable however.</p>
+
+<p>Creating a new context is a rather expensive operation. Hence we keep the context
+alive and just sleep without the need to recreate it. The downside is that there
+is the possibility of a memory leak. Since a timer is implemented in OR as a
+request and requests are short-lived, some memory is not released until after the
+context is destroyed.</p>
+
+<p>The setting <code>max_use</code> controls the timer behaviour. The default value is <code>1000</code>,
+which means that after each <code>1000</code> invocations the timer context is destroyed
+and a new one is generated (this happens transparent to the user).</p>
+
+<p>Optimizing this setting (very opinionated/arbitrary!):</p>
+
+<ul>
+    <li><p>if the timer interval is more than <code>60</code> seconds, then keeping the context
+    around in idle state for that period is probably more expensive resource wise
+    than having to recreate the context. So use <code>max_use == 1</code> to drop the
+    context after each invocation.</p></li>
+    <li><p>if the timer interval is less than <code>5</code> seconds then reusing the context makes
+    sense. Assume recycling to be done once per minute, or for very high
+    frequency timers (and hence higher risk of memory leak), more than once per
+    minute.</p></li>
+</ul>
+
 <p><a name="History"></a></p>
 <h2>History</h2>
 
@@ -150,6 +184,25 @@ <h3>Releasing new versions:</h3>
     <li>upload rock to luarocks: <code>luarocks upload rockspecs/[name] --api-key=abc</code></li>
 </ul>
 
+<h3>unreleased</h3>
+
+<ul>
+    <li>Feat: provide a stacktrace upon errors in the timer callback</li>
+    <li>Feat: add a <code>max_use</code> option. This ensures timer-contexts are recycled to
+
+<pre>
+prevent memory leaks.
+</pre>
+</li>
+    <li>Feat: adds a new function <a href="../index.html#sleep">sleep</a> similar to <code>ngx.sleep</code> except that it is
+
+<pre>
+interrupted on worker exit.
+</pre>
+</li>
+    <li>Fix: now accounts for execution time of the handler, when rescheduling.</li>
+</ul>
+
 <h3>1.1.0 (6-Nov-2020)</h3>
 
 <ul>
@@ -213,7 +266,7 @@ <h2>Copyright and License</h2>
 </div> <!-- id="main" -->
 <div id="about">
 <i>generated by <a href="http://github.com/stevedonovan/LDoc">LDoc 1.4.6</a></i>
-<i style="float:right;">Last updated 2020-11-07 00:01:15 </i>
+<i style="float:right;">Last updated 2021-08-12 14:42:50 </i>
 </div> <!-- id="about" -->
 </div> <!-- id="container" -->
 </body>

lib/resty/timer.lua

@@ -14,7 +14,7 @@ local anchor_registry = {}
 local gc_registry = setmetatable({},{ __mode = "v" })
 local timer_id = 0
 local now = ngx.now
-local sleep = ngx.sleep
+local ngx_sleep = ngx.sleep
 local exiting = ngx.worker.exiting
 
 local KEY_PREFIX    = "[lua-resty-timer]"
@@ -23,6 +23,64 @@ local CANCEL_GC     = "GC"
 local CANCEL_SYSTEM = "SYSTEM"
 local CANCEL_USER   = "USER"
 
+local sleep do
+  -- create a 10yr timer. Will only be called when the worker exits with
+  -- `premature` set. The callback will release a global semaphore to wake up
+  -- sleeping threads
+  local sema = assert(require("ngx.semaphore").new())
+  assert(timer_at(10*365*24*60*60, function()
+    sema:post(math.huge)
+
+    -- TODO: remove the sleep(0), it's a hack around semaphores
+    -- not being released properly, an Openresty bug.
+    -- See https://github.com/openresty/lua-resty-core/issues/337
+    ngx_sleep(0)
+  end))
+
+  --- A `sleep` function that exits early on worker exit. The same as `ngx.sleep`
+  -- except that it will be interrupted when the current worker starts exiting.
+  -- Calling this function after the worker started exiting will immediately
+  -- return (after briefly yielding with a `ngx.sleep(0)`).
+  -- @param delay same as `ngx.sleep()`; delay in seconds.
+  -- @return `true` if finished, `false` if returned early, or nil+err (under
+  -- the hood it will return: "`not ngx.worker.exiting()`").
+  -- @usage if not sleep(5) then
+  --   -- sleep was interrupted, exit now
+  --   return nil, "exiting"
+  -- end
+  --
+  -- -- do stuff
+  function sleep(delay)
+    if type(delay) ~= "number" then
+      error("Bad argument #1, expected number, got " .. type(delay), 2)
+    end
+
+    if delay <= 0 then
+      -- no delay, just yield and return
+      ngx_sleep(delay)
+      return not exiting()
+    end
+
+    local ok, err = sema:wait(delay)
+    if err == "timeout" then
+      -- the sleep was finished
+      return not exiting()
+    end
+
+    -- each call to sleep should at a minimum at least yield, to prevent
+    -- dead-locks elsewhere, so forcefully yield here.
+    ngx_sleep(0)
+
+    if ok then
+      -- we're exiting early because resources were posted to the semaphore
+      return not exiting()
+    end
+
+    ngx.log(ngx.ERR, "waiting for semaphore failed: ", err)
+    return nil, err
+  end
+end
+
 
 
 --- Cancel the timer.
@@ -141,10 +199,7 @@ local function handler(premature, id)
     end
 
     -- existing timer recurring, so keep this thread alive and just sleep
-    if not exiting() then
-      sleep(next_interval)
-    end
-    premature = exiting()
+    premature = not sleep(next_interval)
   end  -- while
 end
 
@@ -160,12 +215,12 @@ end
 --
 -- * `jitter` : (optional, number) variable interval to add to the first interval, default 0.
 -- If set to 1 second then the first interval will be set between `interval` and `interval + 1`.
--- This makes sure if large numbers of timers are used, their execution gets randomly
+-- This makes sure if a large number of timers are used, their execution gets randomly
 -- distributed.
 --
 -- * `immediate` : (boolean) will do the first run immediately (the initial
 -- interval will be set to 0 seconds). This option requires the `recurring` option.
--- The first run will not include the `jitter` interval, it will be added to second run.
+-- The first run will not include the `jitter` interval, it will be added to the second run.
 --
 -- * `detached` : (boolean) if set to `true` the timer will keep running detached, if
 -- set to `false` the timer will be garbage collected unless anchored
@@ -340,6 +395,7 @@ end
 return setmetatable(
   {
     new = new,
+    sleep = sleep,
     CANCEL_GC = CANCEL_GC,
     CANCEL_SYSTEM = CANCEL_SYSTEM,
     CANCEL_USER = CANCEL_USER,