Skip to main content

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

  • From: Shawn Walker < >
  • To: Edward Pilatowicz < >
  • Cc:
  • Subject: [pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)
  • Date: Thu, 15 Aug 2013 15:45:19 -0700

On 08/15/13 13:35, Edward Pilatowicz wrote:
On Thu, Aug 15, 2013 at 01:08:23PM -0700, Shawn Walker wrote:
On 08/15/13 12:58, Edward Pilatowicz wrote:
On Sun, Aug 11, 2013 at 04:20:43PM -0700, Shawn Walker wrote:
Greetings,

The following webrev contains changes for the following:

   17230751 pkg execution should gracefully handle SIGTERM and SIGHUP

https://ips.java.net/webrev/srwalker/pkg-sig-1/


i hate signals and singal handling.  is there any reason that we don't
just write a "start" history entry before starting an operation, and
then write a "end" history entry when we've finished?  if we did this we
wouldn't need signal handling (which won't work for things like SIGKILL,
SIGSEGV, etc).  i'm guessing the answer is "this would require
re-writing how the history mechanism works", but i thought i'd ask
anyway.

For history, each operation is recorded within a single file (XML),
what you suggest would mean writing each history file twice (at
start of operation and at end) or writing two separate files.
That's a pretty substantial change in behaviour.

Although, I'd note that the History module needs to be rewritten at
some point anyway, so this should be considered then.

However, I think we still want to handle the signal if only to avoid
the spurious output that Python currently produces.  Bart also
suggested that we may want to consider ignoring SIGHUP/SIGTERM
during specific critical sections in which case we'd end up doing
some signal handling anyway.

With that in mind, do the proposed changes look right?


the changes seem pretty strait forward.

- i don't know much about how python does signal handling, so i was
   wondering what happens if the abort() call in the signal handler
   generates an exception.  that led me into history.py`abort() to see
   what kind of exceptions it might generate, and when i got to the
   highly magical history.py`__setattr__() i realized i didn't want to
   know the answer that badly.

That's why I have the bare try/except handler; if we can't record the history entry, I just drive on.

The abort() method is specifically designed to be called when a user interrupts the process (e.g. ^C) and this is a very similar case.

- in image.py, i'm annoyed by the fact that we keep repeating the names
   of the cache files everywhere instead of having a define for each
   file.  that said, your code matches the current conventions, so it's
   probably ok.

Yes, I saw the same thing and I almost did something but wanted to minimise the amount of change for this fix.

- do we want a test case for this functionality?  (the pkg client could
   check for some debug value and send itself a SIGHUP or SIGTERM in the
   middle of plan execution, then we could check if there is a history
   entry and if all the cache files have been deleted.)

I hadn't considered doing it that way; I'm not sure how I feel about adding a debug flag that intentionally causes the process to force itself to exit :-)

With that said, I'm also happy you had an idea on how to automatically test this. I can write this if everyone's ok with this sort of thing.


-Shawn



[pkg-discuss] [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Shawn Walker 08/11/2013

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Edward Pilatowicz 08/15/2013

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Shawn Walker 08/15/2013

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Edward Pilatowicz 08/15/2013

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Shawn Walker 08/15/2013

[pkg-discuss] Re: [review] graceful SIGTERM / SIGHUP handling for pkg(1)

Bart Smaalders 08/15/2013
 
 
Close
loading
Please Confirm
Close