[Merge] lp:~jamesodhunt/upstart/bug-1227212 into lp:upstart

[James Hunt]

James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1227212 into lp:upstart.

Requested reviews:
  Upstart Reviewers (upstart-reviewers)
Related bugs:
  Bug #1227212 in upstart : "Session logout takes too long"
  https://bugs.launchpad.net/upstart/+bug/1227212

For more details, see:
https://code.launchpad.net/~jamesodhunt/upstart/bug-1227212/+merge/187844

* init/event.c: event_pending_handle_jobs(): Force quiesce when all job
  instances have finished to speed session shutdown.
* init/job_process.c: job_process_jobs_running(): Only consider job
  instances with associated pids to avoid abstract jobs confusing the
  shutdown.
* init/quiesce.c:
  - quiesce(): Optimise session shutdown 
    - Skip wait phase if no jobs care about the 'session-end' event
      (LP: #1227212).
    - Stop already running instances if other jobs care about
      'session-end' to allow the already-running jobs to shut down in
       parallel with the newly-started session-end jobs.
  - quiesce_wait_callback():
    - Simplify logic.
    - Improve wait phase checks to detect earliest time to finalise.
  - quiesce_finalise(): Display time to shutdown.
  - quiesce_complete(): New function to force final shutdown phase.
  - quiesce_event_match(): New function to determine if any jobs
    'start on' contains a particular event.
  - quiesce_in_progress(): Determine if shutdown is being handled.
* test/test_util_common.c:
  - _start_upstart(): Call get_upstart_binary() rather than relying on
    UPSTART_BINARY define.
  - start_upstart_common(): Remove '--no-startup-event' as this is now
    needed by a test.
  - get_upstart_binary(): Assert that file exists.
  - file_exists(): New helper function.
* test/test_util_common.h: Typo and prototype.
* util/tests/test_initctl.c: test_quiesce():
  - New test "session shutdown: one long-running job which starts on
    startup".
  - Adjusted expected shutdown times.

-----------------------------

Expected behaviour:

= Common Case =

- If no jobs 'start on session-end', shutdown is "immediate". This is the common case.
  => Shutdown time: 1 second.

= Jobs Specifying 'session-end' =

- If any job specifies 'start on session-end' (no standard jobs do):
  - The shutdown will stop all running job instances.
  - The shutdown will allow session-end jobs to run as long as they complete within 5 seconds.
  => Shutdown time: 5 seconds.

= Jobs Specifying 'session-end' and blocking SIGTERM =

- If no jobs specify 'start on session-end' but some jobs block SIGTERM:
  => Shutdown time: 5 seconds.

- If any job specifies 'start on session-end' and blocks SIGTERM (but
  not other jobs block SIGTERM):
  - The shutdown will wait for up to 5 seconds for the jobs to run.
  - The shutdown will then signal them with SIGTERM and wait for up to a further 5 seconds before sending SIGKILL.
  => Shutdown time: 5+5 = 10 seconds.

- If any job specifies 'start on session-end' and blocks SIGTERM *and* other non-session-end jobs
  running at shutdown also block SIGTERM:
  => Shutdown time: 5+5 = 10 seconds (*).

= Jobs with Long-Running pre-start stanzas =

Job that have long-running pre-start stanzas could also slow down the session shutdown since changing their state will take the duration of the time to execute the pre-start stanza. One of the standard session jobs fell into this category ('logrotate') but has now been fixed.

Note that the SIGTERM case is pathological and should be surmountable using the 'kill signal' stanza (see init(5) and
http://upstart.ubuntu.com/cookbook/#kill-signal).

(*) - Not 15 seconds, since the shutdown will send SIGTERM to all non-session-end jobs such that their kill timer runs in parallel with the session-end jobs wait time.
  
-- 
https://code.launchpad.net/~jamesodhunt/upstart/bug-1227212/+merge/187844
Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1227212 into lp:upstart.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: review-diff.txt
Type: text/x-diff
Size: 15055 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/upstart-devel/attachments/20130926/0e8fcdc4/attachment.diff>